Hi Brajesh, On 12/05/2026 07:47, Brajesh Gupta wrote: > Fix context ID value for the FW common context by moving the context > allocation earlier.
"Fix" is pretty vague here – can you describe the current behaviour, and why that's wrong? I also assume this fixes a previous change, so can you please add a Fixes: tag referencing the commit which introduced the faulty behaviour (and possibly a Cc: stable tag, if needed)? > Also an old incorrect comment was removed. This detail probably doesn't need to be mentioned. Cheers, Matt > > Signed-off-by: Brajesh Gupta <[email protected]> > --- > drivers/gpu/drm/imagination/pvr_context.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/imagination/pvr_context.c > b/drivers/gpu/drm/imagination/pvr_context.c > index 8de70c30b9de..eba4694400b5 100644 > --- a/drivers/gpu/drm/imagination/pvr_context.c > +++ b/drivers/gpu/drm/imagination/pvr_context.c > @@ -318,10 +318,14 @@ int pvr_context_create(struct pvr_file *pvr_file, > struct drm_pvr_ioctl_create_co > goto err_put_vm; > } > > - err = pvr_context_create_queues(ctx, args, ctx->data); > + err = xa_alloc(&pvr_dev->ctx_ids, &ctx->ctx_id, ctx, xa_limit_32b, > GFP_KERNEL); > if (err) > goto err_free_ctx_data; > > + err = pvr_context_create_queues(ctx, args, ctx->data); > + if (err) > + goto err_free_ctx_id; > + > err = init_fw_objs(ctx, args, ctx->data); > if (err) > goto err_destroy_queues; > @@ -329,23 +333,12 @@ int pvr_context_create(struct pvr_file *pvr_file, > struct drm_pvr_ioctl_create_co > err = pvr_fw_object_create(pvr_dev, ctx_size, > PVR_BO_FW_FLAGS_DEVICE_UNCACHED, > ctx_fw_data_init, ctx, &ctx->fw_obj); > if (err) > - goto err_free_ctx_data; > + goto err_destroy_queues; > > - err = xa_alloc(&pvr_dev->ctx_ids, &ctx->ctx_id, ctx, xa_limit_32b, > GFP_KERNEL); > + err = xa_alloc(&pvr_file->ctx_handles, &args->handle, ctx, > xa_limit_32b, GFP_KERNEL); > if (err) > goto err_destroy_fw_obj; > > - err = xa_alloc(&pvr_file->ctx_handles, &args->handle, ctx, > xa_limit_32b, GFP_KERNEL); > - if (err) { > - /* > - * It's possible that another thread could have taken a > reference on the context at > - * this point as it is in the ctx_ids xarray. Therefore > instead of directly > - * destroying the context, drop a reference instead. > - */ > - pvr_context_put(ctx); > - return err; > - } > - > spin_lock(&pvr_dev->ctx_list_lock); > list_add_tail(&ctx->file_link, &pvr_file->contexts); > spin_unlock(&pvr_dev->ctx_list_lock); > @@ -358,6 +351,15 @@ int pvr_context_create(struct pvr_file *pvr_file, struct > drm_pvr_ioctl_create_co > err_destroy_queues: > pvr_context_destroy_queues(ctx); > > +err_free_ctx_id: > + /* > + * Ctx_id is not exposed to userspace and not visible yet within > + * the kernel/FW, plus a matching context handle (exposed to > userspace) > + * hasn't been allocated yet, so it is safe to remove ctx_id > + * from the ctx_ids xarray. > + */ > + xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id); > + > err_free_ctx_data: > kfree(ctx->data); > > > -- > 2.43.0 > -- Matt Coster E: [email protected]
OpenPGP_signature.asc
Description: OpenPGP digital signature
