[PATCH] drm/sched: Lock drm_sched_entity_is_idle()

[PATCH] drm/sched: Lock drm_sched_entity_is_idle()

[Philipp Stanner]

drm_sched_entity_is_idle() contains a badly documented memory barrier
and an invalid lockless access to entity->stopped. Moreover, it also
accesses entity->list which is at all other places protected by
entity->lock.

This function is in no way performance critical, so it is safer, more
readable and more maintainable to take the spinlock.

Add locking to drm_sched_entity_is_idle().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/scheduler/sched_entity.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index c51101ec70c1..2d941b2b2861 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -179,14 +179,18 @@ EXPORT_SYMBOL(drm_sched_entity_modify_sched);
 
 static bool drm_sched_entity_is_idle(struct drm_sched_entity *entity)
 {
-	rmb(); /* for list_empty to work without lock */
+	bool idle = false;
+
+	spin_lock(&entity->lock);
 
 	if (list_empty(&entity->list) ||
 	    spsc_queue_count(&entity->job_queue) == 0 ||
 	    entity->stopped)
-		return true;
+		idle = true;
 
-	return false;
+	spin_unlock(&entity->lock);
+
+	return idle;
 }
 
 /**

base-commit: c9a2c8ddcd65e96e7f095a87c0cc0b3211f740ea
-- 
2.55.0

Re: [PATCH] drm/sched: Lock drm_sched_entity_is_idle()

[Tvrtko Ursulin]

On 13/08/2026 10:45, Philipp Stanner wrote:
> drm_sched_entity_is_idle() contains a badly documented memory barrier
> and an invalid lockless access to entity->stopped. Moreover, it also
> accesses entity->list which is at all other places protected by
> entity->lock.
> 
> This function is in no way performance critical, so it is safer, more
> readable and more maintainable to take the spinlock.
> 
> Add locking to drm_sched_entity_is_idle().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>   drivers/gpu/drm/scheduler/sched_entity.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index c51101ec70c1..2d941b2b2861 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -179,14 +179,18 @@ EXPORT_SYMBOL(drm_sched_entity_modify_sched);
>   
>   static bool drm_sched_entity_is_idle(struct drm_sched_entity *entity)
>   {
> -	rmb(); /* for list_empty to work without lock */
> +	bool idle = false;
> +
> +	spin_lock(&entity->lock);
>   
>   	if (list_empty(&entity->list) ||
>   	    spsc_queue_count(&entity->job_queue) == 0 ||
>   	    entity->stopped)
> -		return true;
> +		idle = true;
>   
> -	return false;
> +	spin_unlock(&entity->lock);
> +
> +	return idle;
>   }
>   
>   /**
> 
> base-commit: c9a2c8ddcd65e96e7f095a87c0cc0b3211f740ea

Spotted by chance on the mailing list but I believe I gave you r-b for 
this one already.

Regards,

Tvrtko