On 19/01/2023 22:44, Rob Clark wrote: > From: Rob Clark <robdclark@xxxxxxxxxxxx> > > Once we create the handle, the handle owns the reference. Currently > nothing was doing anything with the shmem ptr after the handle was > created, but let's change drm_gem_shmem_create_with_handle() to not > return the pointer, so-as to not encourage problematic use of this > function in the future. As a bonus, it makes the code a bit cleaner. > > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx> Nice cleanup, one comment below: > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index f21f47737817..fa6281e43954 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -415,7 +415,7 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, > } > EXPORT_SYMBOL(drm_gem_shmem_vunmap); > > -static struct drm_gem_shmem_object * > +static int > drm_gem_shmem_create_with_handle(struct drm_file *file_priv, > struct drm_device *dev, size_t size, > uint32_t *handle) > @@ -425,7 +425,7 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv, > > shmem = drm_gem_shmem_create(dev, size); > if (IS_ERR(shmem)) > - return shmem; > + return PTR_ERR(shmem); > > /* > * Allocate an id of idr table where the obj is registered > @@ -434,10 +434,8 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv, > ret = drm_gem_handle_create(file_priv, &shmem->base, handle); > /* drop reference from allocate - handle holds it now. */ > drm_gem_object_put(&shmem->base); > - if (ret) > - return ERR_PTR(ret); > > - return shmem; > + return ret; > } > > /* Update madvise status, returns true if not purged, else > @@ -533,9 +531,7 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev, > args->size = PAGE_ALIGN(args->pitch * args->height); > } > > - shmem = drm_gem_shmem_create_with_handle(file, dev, args->size, &args->handle); shmem is now unused in this function so the definition needs removing too. Steve > - > - return PTR_ERR_OR_ZERO(shmem); > + return drm_gem_shmem_create_with_handle(file, dev, args->size, &args->handle); > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create); >
