public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Maíra Canal <mcanal@igalia.com>
To: Melissa Wen <mwen@igalia.com>,
	Tvrtko Ursulin <tvrtko.ursulin@igalia.com>,
	Maxime Ripard <mripard@kernel.org>
Cc: kernel-dev@igalia.com, dri-devel@lists.freedesktop.org,
	Maíra Canal <mcanal@igalia.com>,
	Iago Toral Quiroga <itoral@igalia.com>
Subject: [PATCH v2 5/6] drm/v3d: Attach per-fd reset counters to v3d_stats
Date: Thu, 05 Mar 2026 20:25:54 -0300	[thread overview]
Message-ID: <20260305-v3d-reset-locking-improv-v2-5-fd53c91f7726@igalia.com> (raw)
In-Reply-To: <20260305-v3d-reset-locking-improv-v2-0-fd53c91f7726@igalia.com>

From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

To remove the file_priv NULL-ing dance needed to check if the file
descriptor is open, move the per-fd reset counter into v3d_stats, which
is heap-allocated and refcounted, outliving the fd as long as jobs
reference it.

This change allows the removal of the last `queue_lock` usage to protect
`job->file_priv` and avoids possible NULL ptr dereference issues due to
lifetime mismatches.

Also, to simplify locking, replace both the global and per-fd locked
reset counters with atomics.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Co-developed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c   | 24 ++++++++----------------
 drivers/gpu/drm/v3d/v3d_drv.h   | 14 ++++----------
 drivers/gpu/drm/v3d/v3d_sched.c |  9 ++-------
 3 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 0f5e29f57fa59bfa2890de51b9ca78cda33d1edc..e5708b71738debaf3dd16010d50fb9e910a3918d 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -110,13 +110,17 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 		args->value = !!drm_gem_get_huge_mnt(dev);
 		return 0;
 	case DRM_V3D_PARAM_GLOBAL_RESET_COUNTER:
-		mutex_lock(&v3d->reset_lock);
-		args->value = v3d->reset_counter;
-		mutex_unlock(&v3d->reset_lock);
+		args->value = atomic_read(&v3d->reset_counter);
 		return 0;
 	case DRM_V3D_PARAM_CONTEXT_RESET_COUNTER:
+		args->value = 0;
+		/* The mutex ensures the user gets a reliable snapshot from all
+		 * queues. Without it, a queue could trigger a reset while the
+		 * loop is iterating.
+		 */
 		mutex_lock(&v3d->reset_lock);
-		args->value = v3d_priv->reset_counter;
+		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);
 		return 0;
 	default:
@@ -173,23 +177,11 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
 static void
 v3d_postclose(struct drm_device *dev, struct drm_file *file)
 {
-	struct v3d_dev *v3d = to_v3d_dev(dev);
 	struct v3d_file_priv *v3d_priv = file->driver_priv;
-	unsigned long irqflags;
 	enum v3d_queue q;
 
 	for (q = 0; q < V3D_MAX_QUEUES; q++) {
-		struct v3d_queue_state *queue = &v3d->queue[q];
-		struct v3d_job *job = queue->active_job;
-
 		drm_sched_entity_destroy(&v3d_priv->sched_entity[q]);
-
-		if (job && job->base.entity == &v3d_priv->sched_entity[q]) {
-			spin_lock_irqsave(&queue->queue_lock, irqflags);
-			job->file_priv = NULL;
-			spin_unlock_irqrestore(&queue->queue_lock, irqflags);
-		}
-
 		v3d_stats_put(v3d_priv->stats[q]);
 	}
 
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 72c3f40715dae6e86e0c8356cb997cdf1cf03fae..3de485abd8fc274b361cd17a00cab189d8e69643 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -55,6 +55,8 @@ struct v3d_stats {
 	 * job queues, even the write side never is.
 	 */
 	seqcount_t lock;
+
+	atomic_t reset_counter;
 };
 
 struct v3d_queue_state {
@@ -203,10 +205,8 @@ struct v3d_dev {
 	 */
 	struct v3d_perfmon *global_perfmon;
 
-	/* Global reset counter. The counter must be incremented when
-	 * a GPU reset happens. It must be protected by @reset_lock.
-	 */
-	unsigned int reset_counter;
+	/* Global reset counter incremented on each GPU reset. */
+	atomic_t reset_counter;
 };
 
 static inline struct v3d_dev *
@@ -233,12 +233,6 @@ struct v3d_file_priv {
 
 	/* Stores the GPU stats for a specific queue for this fd. */
 	struct v3d_stats *stats[V3D_MAX_QUEUES];
-
-	/* Per-fd reset counter, must be incremented when a job submitted
-	 * by this fd causes a GPU reset. It must be protected by
-	 * &struct v3d_dev->reset_lock.
-	 */
-	unsigned int reset_counter;
 };
 
 struct v3d_bo {
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 5c387a152e33f5ccbca6a9af97675f050a2a701f..1855ef5b3b5fe4e2de1cf0b77bced3735c23ab15 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -701,8 +701,6 @@ v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job *sched_job,
 			  enum v3d_queue q)
 {
 	struct v3d_job *job = to_v3d_job(sched_job);
-	struct v3d_file_priv *v3d_priv = job->file_priv;
-	unsigned long irqflags;
 	enum v3d_queue i;
 
 	mutex_lock(&v3d->reset_lock);
@@ -717,11 +715,8 @@ v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job *sched_job,
 	/* get the GPU back into the init state */
 	v3d_reset(v3d);
 
-	v3d->reset_counter++;
-	spin_lock_irqsave(&v3d->queue[q].queue_lock, irqflags);
-	if (v3d_priv)
-		v3d_priv->reset_counter++;
-	spin_unlock_irqrestore(&v3d->queue[q].queue_lock, irqflags);
+	atomic_inc(&v3d->reset_counter);
+	atomic_inc(&job->client_stats->reset_counter);
 
 	for (i = 0; i < V3D_MAX_QUEUES; i++)
 		drm_sched_resubmit_jobs(&v3d->queue[i].sched);

-- 
2.53.0


  parent reply	other threads:[~2026-03-05 23:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-05 23:25 [PATCH v2 0/6] drm/v3d: Improve v3d_stats lifetime and locking Maíra Canal
2026-03-05 23:25 ` [PATCH v2 1/6] drm/v3d: Handle error from drm_sched_entity_init() Maíra Canal
2026-03-08 23:20   ` Claude review: " Claude Code Review Bot
2026-03-05 23:25 ` [PATCH v2 2/6] drm/v3d: Use raw seqcount helpers instead of fighting with lockdep Maíra Canal
2026-03-08 23:20   ` Claude review: " Claude Code Review Bot
2026-03-05 23:25 ` [PATCH v2 3/6] drm/v3d: Refcount v3d_stats Maíra Canal
2026-03-08 23:20   ` Claude review: " Claude Code Review Bot
2026-03-05 23:25 ` [PATCH v2 4/6] drm/v3d: Hold v3d_stats references in each job Maíra Canal
2026-03-08 23:20   ` Claude review: " Claude Code Review Bot
2026-03-05 23:25 ` Maíra Canal [this message]
2026-03-08 23:20   ` Claude review: drm/v3d: Attach per-fd reset counters to v3d_stats Claude Code Review Bot
2026-03-05 23:25 ` [PATCH v2 6/6] drm/v3d: Remove dedicated fence_lock Maíra Canal
2026-03-08 23:20   ` Claude review: " Claude Code Review Bot
2026-03-08 23:20 ` Claude review: drm/v3d: Improve v3d_stats lifetime and locking Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260305-v3d-reset-locking-improv-v2-5-fd53c91f7726@igalia.com \
    --to=mcanal@igalia.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=itoral@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=mripard@kernel.org \
    --cc=mwen@igalia.com \
    --cc=tvrtko.ursulin@igalia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox