From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Hold v3d_stats references in each job Date: Mon, 09 Mar 2026 08:56:09 +1000 Message-ID: In-Reply-To: <20260306-v3d-reset-locking-improv-v3-4-49864fe00692@igalia.com> References: <20260306-v3d-reset-locking-improv-v3-0-49864fe00692@igalia.com> <20260306-v3d-reset-locking-improv-v3-4-49864fe00692@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This is the key patch that eliminates the race. By having each job hold its own kref'd references to both `client_stats` and `global_stats`, the stats pointers in the job are guaranteed valid for the job's entire lifetime, regardless of fd closure. The refactoring of `v3d_job_start_stats` and `v3d_job_update_stats` to no longer take a queue parameter is clean - the job already knows its stats pointers. The removal of the `queue_lock` / `file_priv` NULL check dance in `v3d_job_update_stats` is the main payoff: ```c - /* Don't update the local stats if the file context has already closed */ - spin_lock(&queue->queue_lock); - if (job->file_priv) - v3d_stats_update(job->file_priv->stats[q], now); - spin_unlock(&queue->queue_lock); ``` becomes simply: ```c v3d_stats_update(job->client_stats, now); ``` This is safe because the job's kref keeps the stats object alive. The stats refs are taken in `v3d_job_init()` after `kref_init(&job->refcount)` and released in `v3d_job_free()`. The placement looks correct. No issues. --- Generated by Claude Code Patch Reviewer