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

[Tvrtko Ursulin]

On 01/07/2026 09:59, Philipp Stanner wrote:
> drm_sched_entity_is_idle() contains a badly documented memory barrier
> and an invalid lockless access to entity->stopped.
>
> This function is in no way performance critical, so it is safer, more
> readable and more maintainable to take the spinlock. This also enables
> future cleanup work where the entity can be fully synchronized via its
> spinlock.
>
> Add locking to drm_sched_entity_is_idle().
>
> Signed-off-by: Philipp Stanner <phasta@xxxxxxxxxx>
> ---
>   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 0fc1213a0d3f..cb03d6a36578 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -178,14 +178,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;
>   }
>   
>   /**

I think this is fine and indeed not performance critical in any way so:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>

Regards,

Tvrtko