* [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object
@ 2022-11-11 3:38 ChunyouTang
2022-11-17 13:42 ` Thomas Zimmermann
0 siblings, 1 reply; 4+ messages in thread
From: ChunyouTang @ 2022-11-11 3:38 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
sumit.semwal, christian.koenig
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig, ChunyouTang
when goto err_free, the object had init, so it should be release when fail.
Signed-off-by: ChunyouTang <tangchunyou@163.com>
---
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);
+
+ dma_resv_fini(&obj->_resv);
+ drm_gem_lru_remove(obj);
+}
+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);
drm_gem_free_mmap_offset(obj);
- drm_gem_lru_remove(obj);
}
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,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object
2022-11-11 3:38 [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object ChunyouTang
@ 2022-11-17 13:42 ` Thomas Zimmermann
2022-11-18 10:32 ` Chunyou Tang
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Zimmermann @ 2022-11-17 13:42 UTC (permalink / raw)
To: ChunyouTang, maarten.lankhorst, mripard, airlied, daniel,
sumit.semwal, christian.koenig
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig
[-- Attachment #1.1: Type: text/plain, Size: 3393 bytes --]
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@163.com>
> ---
> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object
2022-11-17 13:42 ` Thomas Zimmermann
@ 2022-11-18 10:32 ` Chunyou Tang
2022-11-18 11:21 ` Thomas Zimmermann
0 siblings, 1 reply; 4+ messages in thread
From: Chunyou Tang @ 2022-11-18 10:32 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: maarten.lankhorst, mripard, airlied, daniel, sumit.semwal,
christian.koenig, dri-devel, linux-kernel, linux-media,
linaro-mm-sig
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB18030, Size: 3509 bytes --]
Hi Thomas,
Can I discard the first two patchs, and pull the new code, then
modify and git send-email this patch?
ÓÚ Thu, 17 Nov 2022 14:42:36 +0100
Thomas Zimmermann <tzimmermann@suse.de> дµÀ:
> 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@163.com>
> > ---
> > 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,
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object
2022-11-18 10:32 ` Chunyou Tang
@ 2022-11-18 11:21 ` Thomas Zimmermann
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Zimmermann @ 2022-11-18 11:21 UTC (permalink / raw)
To: Chunyou Tang
Cc: maarten.lankhorst, mripard, airlied, daniel, sumit.semwal,
christian.koenig, dri-devel, linux-kernel, linux-media,
linaro-mm-sig
[-- Attachment #1.1: Type: text/plain, Size: 4053 bytes --]
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@suse.de> 写道:
>
>> 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@163.com>
>>> ---
>>> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-18 11:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11 3:38 [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object ChunyouTang
2022-11-17 13:42 ` Thomas Zimmermann
2022-11-18 10:32 ` Chunyou Tang
2022-11-18 11:21 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).