On Mon, 23 Sep 2024 11:34:06 +0100
Steven Price <steven.price@xxxxxxx> wrote:
> XArray provides it's own internal lock which protects the internal array
> when entries are being simultaneously added and removed. However there
> is still a race between retrieving the pointer from the XArray and
> incrementing the reference count.
>
> To avoid this race simply hold the internal XArray lock when
> incrementing the reference count, this ensures there cannot be a racing
> call to xa_erase().
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Steven Price <steven.price@xxxxxxx>
Uh, nice catch!
Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 42afdf0ddb7e..0dbeebcf23b4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3242,6 +3242,18 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
> return 0;
> }
>
> +static struct panthor_group *group_from_handle(struct panthor_group_pool *pool,
> + u32 group_handle)
> +{
> + struct panthor_group *group;
> +
> + xa_lock(&pool->xa);
> + group = group_get(xa_load(&pool->xa, group_handle));
> + xa_unlock(&pool->xa);
> +
> + return group;
> +}
> +
> int panthor_group_get_state(struct panthor_file *pfile,
> struct drm_panthor_group_get_state *get_state)
> {
> @@ -3253,7 +3265,7 @@ int panthor_group_get_state(struct panthor_file *pfile,
> if (get_state->pad)
> return -EINVAL;
>
> - group = group_get(xa_load(&gpool->xa, get_state->group_handle));
> + group = group_from_handle(gpool, get_state->group_handle);
> if (!group)
> return -EINVAL;
>
> @@ -3384,7 +3396,7 @@ panthor_job_create(struct panthor_file *pfile,
> job->call_info.latest_flush = qsubmit->latest_flush;
> INIT_LIST_HEAD(&job->node);
>
> - job->group = group_get(xa_load(&gpool->xa, group_handle));
> + job->group = group_from_handle(gpool, group_handle);
> if (!job->group) {
> ret = -EINVAL;
> goto err_put_job;
