@@ -121,6 +121,7 @@ v3d_open(struct drm_device *dev, struct drm_file *file)
1, NULL);
memset(&v3d_priv->stats[i], 0, sizeof(v3d_priv->stats[i]));
+ seqcount_init(&v3d_priv->stats[i].lock);
}
v3d_perfmon_open_file(v3d_priv);
@@ -145,10 +146,15 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file)
void v3d_get_stats(const struct v3d_stats *stats, u64 timestamp,
u64 *active_runtime, u64 *jobs_completed)
{
- *active_runtime = stats->enabled_ns;
- if (stats->start_ns)
- *active_runtime += timestamp - stats->start_ns;
- *jobs_completed = stats->jobs_completed;
+ unsigned int seq;
+
+ do {
+ seq = read_seqcount_begin(&stats->lock);
+ *active_runtime = stats->enabled_ns;
+ if (stats->start_ns)
+ *active_runtime += timestamp - stats->start_ns;
+ *jobs_completed = stats->jobs_completed;
+ } while (read_seqcount_retry(&stats->lock, seq));
}
static void v3d_show_fdinfo(struct drm_printer *p, struct drm_file *file)
@@ -40,6 +40,13 @@ struct v3d_stats {
u64 start_ns;
u64 enabled_ns;
u64 jobs_completed;
+
+ /*
+ * This seqcount is used to protect the access to the GPU stats
+ * variables. It must be used as, while we are reading the stats,
+ * IRQs can happen and the stats can be updated.
+ */
+ seqcount_t lock;
};
struct v3d_queue_state {
@@ -251,6 +251,7 @@ v3d_gem_init(struct drm_device *dev)
queue->fence_context = dma_fence_context_alloc(1);
memset(&queue->stats, 0, sizeof(queue->stats));
+ seqcount_init(&queue->stats.lock);
}
spin_lock_init(&v3d->mm_lock);
@@ -114,16 +114,23 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue queue)
struct v3d_stats *local_stats = &file->stats[queue];
u64 now = local_clock();
+ write_seqcount_begin(&local_stats->lock);
local_stats->start_ns = now;
+ write_seqcount_end(&local_stats->lock);
+
+ write_seqcount_begin(&global_stats->lock);
global_stats->start_ns = now;
+ write_seqcount_end(&global_stats->lock);
}
static void
v3d_stats_update(struct v3d_stats *stats, u64 now)
{
+ write_seqcount_begin(&stats->lock);
stats->enabled_ns += now - stats->start_ns;
stats->jobs_completed++;
stats->start_ns = 0;
+ write_seqcount_end(&stats->lock);
}
void
In V3D, the conclusion of a job is indicated by a IRQ. When a job finishes, then we update the local and the global GPU stats of that queue. But, while the GPU stats are being updated, a user might be reading the stats from sysfs or fdinfo. For example, on `gpu_stats_show()`, we could think about a scenario where `v3d->queue[queue].start_ns != 0`, then an interrupt happens, we update the value of `v3d->queue[queue].start_ns` to 0, we come back to `gpu_stats_show()` to calculate `active_runtime` and now, `active_runtime = timestamp`. In this simple example, the user would see a spike in the queue usage, that didn't match reality. In order to address this issue properly, use a seqcount to protect read and write sections of the code. Fixes: 09a93cc4f7d1 ("drm/v3d: Implement show_fdinfo() callback for GPU usage stats") Reported-by: Tvrtko Ursulin <tursulin@igalia.com> Signed-off-by: Maíra Canal <mcanal@igalia.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> --- drivers/gpu/drm/v3d/v3d_drv.c | 14 ++++++++++---- drivers/gpu/drm/v3d/v3d_drv.h | 7 +++++++ drivers/gpu/drm/v3d/v3d_gem.c | 1 + drivers/gpu/drm/v3d/v3d_sched.c | 7 +++++++ 4 files changed, 25 insertions(+), 4 deletions(-)