[PATCH] drm/panthor: Defer scheduler entitiy destruction to queue release

[PATCH] drm/panthor: Defer scheduler entitiy destruction to queue release

[Adrián Larumbe]

Commit de8548813824 ("drm/panthor: Add the scheduler logical block")
handled destruction of a group's queues' drm scheduler entities early
into the group destruction procedure.

However, that races with the group submit ioctl, because by the time
entities are destroyed (through the group destroy ioctl), the submission
procedure might've already obtained a group handle, and therefore the
ability to push jobs into entities. This is met with a DRM error message
within the drm scheduler core as a situation that should never occur.

Fix by deferring drm scheduler entity destruction to queue release time.

Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_sched.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 0cc9055f4ee5..f5e01cb16cfc 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -898,8 +898,7 @@ static void group_free_queue(struct panthor_group *group, struct panthor_queue *
 	if (IS_ERR_OR_NULL(queue))
 		return;
 
-	if (queue->entity.fence_context)
-		drm_sched_entity_destroy(&queue->entity);
+	drm_sched_entity_destroy(&queue->entity);
 
 	if (queue->scheduler.ops)
 		drm_sched_fini(&queue->scheduler);
@@ -3609,11 +3608,6 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
 	if (!group)
 		return -EINVAL;
 
-	for (u32 i = 0; i < group->queue_count; i++) {
-		if (group->queues[i])
-			drm_sched_entity_destroy(&group->queues[i]->entity);
-	}
-
 	mutex_lock(&sched->reset.lock);
 	mutex_lock(&sched->lock);
 	group->destroyed = true;
-- 
2.51.0

Re: [PATCH] drm/panthor: Defer scheduler entitiy destruction to queue release

[Steven Price]

On 19/09/2025 17:43, Adrián Larumbe wrote:
> Commit de8548813824 ("drm/panthor: Add the scheduler logical block")
> handled destruction of a group's queues' drm scheduler entities early
> into the group destruction procedure.
> 
> However, that races with the group submit ioctl, because by the time
> entities are destroyed (through the group destroy ioctl), the submission
> procedure might've already obtained a group handle, and therefore the
> ability to push jobs into entities. This is met with a DRM error message
> within the drm scheduler core as a situation that should never occur.
> 
> Fix by deferring drm scheduler entity destruction to queue release time.
> 
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

Looks good - I can't figure out why we had the early destruction, this
is more simple and safer.

Reviewed-by: Steven Price <steven.price@arm.com>

Thanks,
Steve

> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0cc9055f4ee5..f5e01cb16cfc 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -898,8 +898,7 @@ static void group_free_queue(struct panthor_group *group, struct panthor_queue *
>  	if (IS_ERR_OR_NULL(queue))
>  		return;
>  
> -	if (queue->entity.fence_context)
> -		drm_sched_entity_destroy(&queue->entity);
> +	drm_sched_entity_destroy(&queue->entity);
>  
>  	if (queue->scheduler.ops)
>  		drm_sched_fini(&queue->scheduler);
> @@ -3609,11 +3608,6 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  	if (!group)
>  		return -EINVAL;
>  
> -	for (u32 i = 0; i < group->queue_count; i++) {
> -		if (group->queues[i])
> -			drm_sched_entity_destroy(&group->queues[i]->entity);
> -	}
> -
>  	mutex_lock(&sched->reset.lock);
>  	mutex_lock(&sched->lock);
>  	group->destroyed = true;

Re: [PATCH] drm/panthor: Defer scheduler entitiy destruction to queue release

[Boris Brezillon]

On Fri, 19 Sep 2025 17:43:48 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> Commit de8548813824 ("drm/panthor: Add the scheduler logical block")
> handled destruction of a group's queues' drm scheduler entities early
> into the group destruction procedure.
> 
> However, that races with the group submit ioctl, because by the time
> entities are destroyed (through the group destroy ioctl), the submission
> procedure might've already obtained a group handle, and therefore the
> ability to push jobs into entities. This is met with a DRM error message
> within the drm scheduler core as a situation that should never occur.
> 
> Fix by deferring drm scheduler entity destruction to queue release time.
> 
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

Discussed with you before you posted it, so here's the official

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

Thanks for fixing that.

Boris


> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0cc9055f4ee5..f5e01cb16cfc 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -898,8 +898,7 @@ static void group_free_queue(struct panthor_group *group, struct panthor_queue *
>  	if (IS_ERR_OR_NULL(queue))
>  		return;
>  
> -	if (queue->entity.fence_context)
> -		drm_sched_entity_destroy(&queue->entity);
> +	drm_sched_entity_destroy(&queue->entity);
>  
>  	if (queue->scheduler.ops)
>  		drm_sched_fini(&queue->scheduler);
> @@ -3609,11 +3608,6 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  	if (!group)
>  		return -EINVAL;
>  
> -	for (u32 i = 0; i < group->queue_count; i++) {
> -		if (group->queues[i])
> -			drm_sched_entity_destroy(&group->queues[i]->entity);
> -	}
> -
>  	mutex_lock(&sched->reset.lock);
>  	mutex_lock(&sched->lock);
>  	group->destroyed = true;

Re: [PATCH] drm/panthor: Defer scheduler entitiy destruction to queue release

[Steven Price]

On 19/09/2025 17:43, Adrián Larumbe wrote:
> Commit de8548813824 ("drm/panthor: Add the scheduler logical block")
> handled destruction of a group's queues' drm scheduler entities early
> into the group destruction procedure.
> 
> However, that races with the group submit ioctl, because by the time
> entities are destroyed (through the group destroy ioctl), the submission
> procedure might've already obtained a group handle, and therefore the
> ability to push jobs into entities. This is met with a DRM error message
> within the drm scheduler core as a situation that should never occur.
> 
> Fix by deferring drm scheduler entity destruction to queue release time.
> 
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

Applied to drm-misc-fixes.

Thanks,
Steve

> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0cc9055f4ee5..f5e01cb16cfc 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -898,8 +898,7 @@ static void group_free_queue(struct panthor_group *group, struct panthor_queue *
>  	if (IS_ERR_OR_NULL(queue))
>  		return;
>  
> -	if (queue->entity.fence_context)
> -		drm_sched_entity_destroy(&queue->entity);
> +	drm_sched_entity_destroy(&queue->entity);
>  
>  	if (queue->scheduler.ops)
>  		drm_sched_fini(&queue->scheduler);
> @@ -3609,11 +3608,6 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  	if (!group)
>  		return -EINVAL;
>  
> -	for (u32 i = 0; i < group->queue_count; i++) {
> -		if (group->queues[i])
> -			drm_sched_entity_destroy(&group->queues[i]->entity);
> -	}
> -
>  	mutex_lock(&sched->reset.lock);
>  	mutex_lock(&sched->lock);
>  	group->destroyed = true;