Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object

[Thomas Zimmermann]

Hi

Am 18.11.22 um 11:32 schrieb Chunyou Tang:
> Hi Thomas,
>     Can I discard the first two patchs, and pull the new code, then
>     modify and git send-email this patch?

Yes, of course. Just make sure that the next version is marked as v3, so 
it's obvious what it belongs to.

Best regards
Thomas

> 于 Thu, 17 Nov 2022 14:42:36 +0100
> Thomas Zimmermann <tzimmermann@xxxxxxx> 写道:
>
> > Hi
> >
> > Am 11.11.22 um 04:38 schrieb ChunyouTang:
> > > when goto err_free, the object had init, so it should be release
> > > when fail.
> > >
> > > Signed-off-by: ChunyouTang <tangchunyou@xxxxxxx>
> > > ---
> > >    drivers/gpu/drm/drm_gem.c              | 19 ++++++++++++++++---
> > >    drivers/gpu/drm/drm_gem_shmem_helper.c |  4 +++-
> > >    include/drm/drm_gem.h                  |  1 +
> > >    3 files changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index 8b68a3c1e6ab..cba32c46bb05 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -169,6 +169,21 @@ void drm_gem_private_object_init(struct
> > > drm_device *dev, }
> > >    EXPORT_SYMBOL(drm_gem_private_object_init);
> > >    
> > > +/**
> > > + * drm_gem_private_object_fini - Finalize a failed drm_gem_object
> > > + * @obj: drm_gem_object
> > > + *
> > > + * Uninitialize an already allocated GEM object when it
> > > initialized failed
> > > + */
> > > +void drm_gem_private_object_fini(struct drm_gem_object *obj)
> > > +{
> > > +	WARN_ON(obj->dma_buf);
> >
> > Rather lease this in its original place.
> >
> > > +
> > > +	dma_resv_fini(&obj->_resv);
> > > +	drm_gem_lru_remove(obj);
> >
> > AFAICT drm_gem_lru_remove() doesn't belong into this function.
> >
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_private_object_fini);
> > > +
> > >    /**
> > >     * drm_gem_object_handle_free - release resources bound to
> > > userspace handles
> > >     * @obj: GEM object to clean up.
> > > @@ -930,14 +945,12 @@ drm_gem_release(struct drm_device *dev,
> > > struct drm_file *file_private) void
> > >    drm_gem_object_release(struct drm_gem_object *obj)
> > >    {
> > > -	WARN_ON(obj->dma_buf);
> > > +	drm_gem_private_object_fini(obj);
> > >    
> > >    	if (obj->filp)
> > >    		fput(obj->filp);
> > >    
> > > -	dma_resv_fini(&obj->_resv);
> >
> > Please call drm_gem_private_object_fini() here.
> >
> > >    	drm_gem_free_mmap_offset(obj);
> > > -	drm_gem_lru_remove(obj);
> >
> > Please keep this line here.
> >
> > Best regards
> > Thomas
> >
> > >    }
> > >    EXPORT_SYMBOL(drm_gem_object_release);
> > >    
> > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > b/drivers/gpu/drm/drm_gem_shmem_helper.c index
> > > 35138f8a375c..845e3d5d71eb 100644 ---
> > > a/drivers/gpu/drm/drm_gem_shmem_helper.c +++
> > > b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -79,8 +79,10 @@
> > > __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool
> > > private) } else { ret = drm_gem_object_init(dev, obj, size);
> > >    	}
> > > -	if (ret)
> > > +	if (ret) {
> > > +		drm_gem_private_object_fini(obj)
> > >    		goto err_free;
> > > +	}
> > >    
> > >    	ret = drm_gem_create_mmap_offset(obj);
> > >    	if (ret)
> > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > > index bd42f25e449c..9b1feb03069d 100644
> > > --- a/include/drm/drm_gem.h
> > > +++ b/include/drm/drm_gem.h
> > > @@ -405,6 +405,7 @@ int drm_gem_object_init(struct drm_device *dev,
> > >    			struct drm_gem_object *obj, size_t size);
> > >    void drm_gem_private_object_init(struct drm_device *dev,
> > >    				 struct drm_gem_object *obj,
> > > size_t size); +void drm_gem_private_object_fini(struct
> > > drm_gem_object *obj); void drm_gem_vm_open(struct vm_area_struct
> > > *vma); void drm_gem_vm_close(struct vm_area_struct *vma);
> > >    int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long
> > > obj_size,

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature