diff mbox series

staging/vboxvideo: Replace ttm_bo_unref with ttm_bo_put

Message ID 20180731063735.11899-1-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series staging/vboxvideo: Replace ttm_bo_unref with ttm_bo_put | expand

Commit Message

Thomas Zimmermann July 31, 2018, 6:37 a.m. UTC
The function ttm_bo_put releases a reference to a TTM buffer object. The
function's name is more aligned to the Linux kernel convention of naming
ref-counting function _get and _put.

A call to ttm_bo_unref takes the address of the TTM BO object's pointer and
clears the pointer's value to NULL. This is not necessary in most cases and
sometimes even worked around by the calling code. A call to ttm_bo_put only
releases the reference without clearing the pointer.

The current behaviour of cleaning the pointer is kept in the calling code,
but should be removed if not required in a later patch.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/staging/vboxvideo/vbox_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Greg KH Aug. 2, 2018, 7:29 a.m. UTC | #1
On Tue, Jul 31, 2018 at 08:37:35AM +0200, Thomas Zimmermann wrote:
> The function ttm_bo_put releases a reference to a TTM buffer object. The
> function's name is more aligned to the Linux kernel convention of naming
> ref-counting function _get and _put.
> 
> A call to ttm_bo_unref takes the address of the TTM BO object's pointer and
> clears the pointer's value to NULL. This is not necessary in most cases and
> sometimes even worked around by the calling code. A call to ttm_bo_put only
> releases the reference without clearing the pointer.
> 
> The current behaviour of cleaning the pointer is kept in the calling code,
> but should be removed if not required in a later patch.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/staging/vboxvideo/vbox_main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
> index 9d2018cd544e..fc7293b756b2 100644
> --- a/drivers/staging/vboxvideo/vbox_main.c
> +++ b/drivers/staging/vboxvideo/vbox_main.c
> @@ -490,9 +490,8 @@ static void vbox_bo_unref(struct vbox_bo **bo)
>  		return;
>  
>  	tbo = &((*bo)->bo);
> -	ttm_bo_unref(&tbo);
> -	if (!tbo)
> -		*bo = NULL;
> +	ttm_bo_put(tbo);

This function is not in my tree, so it causes a build error.  I guess
merge this through the drm tree?

thanks,

greg k-h
Thomas Zimmermann Aug. 2, 2018, 7:52 a.m. UTC | #2
Hi

Am 02.08.2018 um 09:29 schrieb Greg KH:
>>  	tbo = &((*bo)->bo);
>> -	ttm_bo_unref(&tbo);
>> -	if (!tbo)
>> -		*bo = NULL;
>> +	ttm_bo_put(tbo);
> 
> This function is not in my tree, so it causes a build error.  I guess
> merge this through the drm tree?

Yes. These functions have been accepted into TTM and are present in
drm-tip. Apparently the respective patches have not yet propagated
towards upstream.

Best regards
Thomas

> thanks,
> 
> greg k-h
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Thomas Zimmermann Sept. 10, 2018, 8:39 a.m. UTC | #3
Hi Greg

Am 02.08.2018 um 09:29 schrieb Greg KH:
> On Tue, Jul 31, 2018 at 08:37:35AM +0200, Thomas Zimmermann wrote:
>> The function ttm_bo_put releases a reference to a TTM buffer object. The
>> function's name is more aligned to the Linux kernel convention of naming
>> ref-counting function _get and _put.
>>
>> A call to ttm_bo_unref takes the address of the TTM BO object's pointer and
>> clears the pointer's value to NULL. This is not necessary in most cases and
>> sometimes even worked around by the calling code. A call to ttm_bo_put only
>> releases the reference without clearing the pointer.
>>
>> The current behaviour of cleaning the pointer is kept in the calling code,
>> but should be removed if not required in a later patch.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/staging/vboxvideo/vbox_main.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
>> index 9d2018cd544e..fc7293b756b2 100644
>> --- a/drivers/staging/vboxvideo/vbox_main.c
>> +++ b/drivers/staging/vboxvideo/vbox_main.c
>> @@ -490,9 +490,8 @@ static void vbox_bo_unref(struct vbox_bo **bo)
>>  		return;
>>  
>>  	tbo = &((*bo)->bo);
>> -	ttm_bo_unref(&tbo);
>> -	if (!tbo)
>> -		*bo = NULL;
>> +	ttm_bo_put(tbo);
> 
> This function is not in my tree, so it causes a build error.  I guess
> merge this through the drm tree?

The required function ttm_bo_put() is now available in mainline. [1] Can
you please try again merging the patch? Thanks!

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=89c815ef07a1db0ac2cc09d06cb2d5c3d86d6322

> thanks,
> 
> greg k-h
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Greg KH Sept. 10, 2018, 8:42 a.m. UTC | #4
On Mon, Sep 10, 2018 at 10:39:57AM +0200, Thomas Zimmermann wrote:
> Hi Greg
> 
> Am 02.08.2018 um 09:29 schrieb Greg KH:
> > On Tue, Jul 31, 2018 at 08:37:35AM +0200, Thomas Zimmermann wrote:
> >> The function ttm_bo_put releases a reference to a TTM buffer object. The
> >> function's name is more aligned to the Linux kernel convention of naming
> >> ref-counting function _get and _put.
> >>
> >> A call to ttm_bo_unref takes the address of the TTM BO object's pointer and
> >> clears the pointer's value to NULL. This is not necessary in most cases and
> >> sometimes even worked around by the calling code. A call to ttm_bo_put only
> >> releases the reference without clearing the pointer.
> >>
> >> The current behaviour of cleaning the pointer is kept in the calling code,
> >> but should be removed if not required in a later patch.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> ---
> >>  drivers/staging/vboxvideo/vbox_main.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
> >> index 9d2018cd544e..fc7293b756b2 100644
> >> --- a/drivers/staging/vboxvideo/vbox_main.c
> >> +++ b/drivers/staging/vboxvideo/vbox_main.c
> >> @@ -490,9 +490,8 @@ static void vbox_bo_unref(struct vbox_bo **bo)
> >>  		return;
> >>  
> >>  	tbo = &((*bo)->bo);
> >> -	ttm_bo_unref(&tbo);
> >> -	if (!tbo)
> >> -		*bo = NULL;
> >> +	ttm_bo_put(tbo);
> > 
> > This function is not in my tree, so it causes a build error.  I guess
> > merge this through the drm tree?
> 
> The required function ttm_bo_put() is now available in mainline. [1] Can
> you please try again merging the patch? Thanks!

Sure, can you resend it please?  This one is long gone from my patch
queue.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c
index 9d2018cd544e..fc7293b756b2 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -490,9 +490,8 @@  static void vbox_bo_unref(struct vbox_bo **bo)
 		return;
 
 	tbo = &((*bo)->bo);
-	ttm_bo_unref(&tbo);
-	if (!tbo)
-		*bo = NULL;
+	ttm_bo_put(tbo);
+	*bo = NULL;
 }
 
 void vbox_gem_free_object(struct drm_gem_object *obj)