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: Attach per-fd reset counters to v3d_stats Date: Mon, 09 Mar 2026 09:20:16 +1000 Message-ID: In-Reply-To: <20260305-v3d-reset-locking-improv-v2-5-fd53c91f7726@igalia.com> References: <20260305-v3d-reset-locking-improv-v2-0-fd53c91f7726@igalia.com> <20260305-v3d-reset-locking-improv-v2-5-fd53c91f7726@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 patch completes the refcounting story by moving the per-fd reset counter into `v3d_stats`. The use of `atomic_t` for both global and per-client reset counters is appropriate since they're now incremented from the reset path without the `reset_lock` held for the per-client counter: ```c atomic_inc(&v3d->reset_counter); atomic_inc(&job->client_stats->reset_counter); ``` The `DRM_V3D_PARAM_CONTEXT_RESET_COUNTER` implementation correctly sums across all queues under `reset_lock` to get a consistent snapshot: ```c mutex_lock(&v3d->reset_lock); for (enum v3d_queue q = 0; q < V3D_MAX_QUEUES; q++) args->value += atomic_read(&v3d_priv->stats[q]->reset_counter); mutex_unlock(&v3d->reset_lock); ``` The comment explaining why the mutex is still needed here is good. The removal of the `file_priv` NULL-ing dance in `v3d_postclose` is the culmination of the series and is safe because: (1) `drm_sched_entity_destroy` waits for all entity jobs to complete, so no active job can reference this fd's entity afterward; and (2) the timeout/reset path now uses `job->client_stats` instead of `job->file_priv`. Note: `DRM_V3D_PARAM_GLOBAL_RESET_COUNTER` no longer takes `reset_lock` since it's a single atomic read. This is correct. --- Generated by Claude Code Patch Reviewer