No, that spinlock is indeed incorrect.
See even when we protect the spsc queue with a spinlock that doesn't
make it correct. It can happen that the jobs pushed to the queue are
reversed in their sequence order and that can cause severe problems in
the memory management.
Christian.
Am 16.05.2018 um 05:33 schrieb Grodzovsky, Andrey:
Yeah, that what I am not sure about... It's lockless in a sense of single producer single consumer but not for multiple concurrent producers... So now I think this spinlock should stay there... It just looked useless to me at first sight...
Andrey
________________________________________
From: Zhou, David(ChunMing)
Sent: 15 May 2018 23:04:44
To: Grodzovsky, Andrey; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Koenig, Christian
Subject: Re: [PATCH 2/2] drm/scheduler: Remove obsolete spinlock.
On 2018年05月16日 03:31, Andrey Grodzovsky wrote:
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
---
drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ----
include/drm/gpu_scheduler.h | 1 -
2 files changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 1f1dd70..2569a63 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -140,7 +140,6 @@ int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
entity->last_scheduled = NULL;
spin_lock_init(&entity->rq_lock);
- spin_lock_init(&entity->queue_lock);
spsc_queue_init(&entity->job_queue);
atomic_set(&entity->fence_seq, 0);
@@ -424,11 +423,8 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
trace_drm_sched_job(sched_job, entity);
- spin_lock(&entity->queue_lock);
first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
- spin_unlock(&entity->queue_lock);
Is your spsc safely to be added simultaneously?
Regards,
David Zhou
-
/* first job wakes up scheduler */
if (first) {
/* Add the entity to the run queue */
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 350a62c..683eb65 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -56,7 +56,6 @@ struct drm_sched_entity {
spinlock_t rq_lock;
struct drm_gpu_scheduler *sched;
- spinlock_t queue_lock;
struct spsc_queue job_queue;
atomic_t fence_seq;
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
