[PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Alex Deucher]

From: Ahzo <Ahzo@tutanota.com>

Set the drm_device to NULL, so that the newly created buffer object
doesn't appear to use the embedded gem object.

This is necessary, because otherwise no corresponding dma_resv_fini for
the dma_resv_init is called, resulting in a memory leak.

The dma_resv_fini in ttm_bo_release_list is only called if the embedded
gem object is not used, which is determined by checking if the
drm_device is NULL.

Bug: https://gitlab.freedesktop.org/drm/amd/issues/958
Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
Signed-off-by: Ahzo <Ahzo@tutanota.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index c8e359ded1df..44c1e7adfb7c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
 		fbo->base.base.resv = &fbo->base.base._resv;
 
 	dma_resv_init(&fbo->base.base._resv);
+	fbo->base.base.dev = NULL;
 	ret = dma_resv_trylock(&fbo->base.base._resv);
 	WARN_ON(!ret);
 
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Christian König]

Am 25.02.20 um 19:56 schrieb Alex Deucher:
> From: Ahzo <Ahzo@tutanota.com>
>
> Set the drm_device to NULL, so that the newly created buffer object
> doesn't appear to use the embedded gem object.
>
> This is necessary, because otherwise no corresponding dma_resv_fini for
> the dma_resv_init is called, resulting in a memory leak.
>
> The dma_resv_fini in ttm_bo_release_list is only called if the embedded
> gem object is not used, which is determined by checking if the
> drm_device is NULL.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/958
> Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
> Signed-off-by: Ahzo <Ahzo@tutanota.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Good catch I was trying to hunt that one down as well.

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index c8e359ded1df..44c1e7adfb7c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>   		fbo->base.base.resv = &fbo->base.base._resv;
>   
>   	dma_resv_init(&fbo->base.base._resv);
> +	fbo->base.base.dev = NULL;
>   	ret = dma_resv_trylock(&fbo->base.base._resv);
>   	WARN_ON(!ret);
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Alex Deucher]

On Tue, Feb 25, 2020 at 2:09 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 25.02.20 um 19:56 schrieb Alex Deucher:
> > From: Ahzo <Ahzo@tutanota.com>
> >
> > Set the drm_device to NULL, so that the newly created buffer object
> > doesn't appear to use the embedded gem object.
> >
> > This is necessary, because otherwise no corresponding dma_resv_fini for
> > the dma_resv_init is called, resulting in a memory leak.
> >
> > The dma_resv_fini in ttm_bo_release_list is only called if the embedded
> > gem object is not used, which is determined by checking if the
> > drm_device is NULL.
> >
> > Bug: https://gitlab.freedesktop.org/drm/amd/issues/958
> > Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
> > Signed-off-by: Ahzo <Ahzo@tutanota.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Good catch I was trying to hunt that one down as well.
>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Can you apply it to drm-misc?

Thanks,

Alex

>
> > ---
> >   drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > index c8e359ded1df..44c1e7adfb7c 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
> >               fbo->base.base.resv = &fbo->base.base._resv;
> >
> >       dma_resv_init(&fbo->base.base._resv);
> > +     fbo->base.base.dev = NULL;
> >       ret = dma_resv_trylock(&fbo->base.base._resv);
> >       WARN_ON(!ret);
> >
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Christian König]

Am 25.02.20 um 20:11 schrieb Alex Deucher:
> On Tue, Feb 25, 2020 at 2:09 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 25.02.20 um 19:56 schrieb Alex Deucher:
>>> From: Ahzo <Ahzo@tutanota.com>
>>>
>>> Set the drm_device to NULL, so that the newly created buffer object
>>> doesn't appear to use the embedded gem object.
>>>
>>> This is necessary, because otherwise no corresponding dma_resv_fini for
>>> the dma_resv_init is called, resulting in a memory leak.
>>>
>>> The dma_resv_fini in ttm_bo_release_list is only called if the embedded
>>> gem object is not used, which is determined by checking if the
>>> drm_device is NULL.
>>>
>>> Bug: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F958&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&amp;sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&amp;reserved=0
>>> Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
>>> Signed-off-by: Ahzo <Ahzo@tutanota.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Good catch I was trying to hunt that one down as well.
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
> Can you apply it to drm-misc?

Sure, but not today. Need to get the kids to bed.

Christian.

>
> Thanks,
>
> Alex
>
>>> ---
>>>    drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> index c8e359ded1df..44c1e7adfb7c 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>> @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
>>>                fbo->base.base.resv = &fbo->base.base._resv;
>>>
>>>        dma_resv_init(&fbo->base.base._resv);
>>> +     fbo->base.base.dev = NULL;
>>>        ret = dma_resv_trylock(&fbo->base.base._resv);
>>>        WARN_ON(!ret);
>>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Christian König]

Am 25.02.20 um 20:12 schrieb Christian König:
> Am 25.02.20 um 20:11 schrieb Alex Deucher:
>> On Tue, Feb 25, 2020 at 2:09 PM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Am 25.02.20 um 19:56 schrieb Alex Deucher:
>>>> From: Ahzo <Ahzo@tutanota.com>
>>>>
>>>> Set the drm_device to NULL, so that the newly created buffer object
>>>> doesn't appear to use the embedded gem object.
>>>>
>>>> This is necessary, because otherwise no corresponding dma_resv_fini 
>>>> for
>>>> the dma_resv_init is called, resulting in a memory leak.
>>>>
>>>> The dma_resv_fini in ttm_bo_release_list is only called if the 
>>>> embedded
>>>> gem object is not used, which is determined by checking if the
>>>> drm_device is NULL.
>>>>
>>>> Bug: 
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F958&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&amp;sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&amp;reserved=0
>>>> Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
>>>> Signed-off-by: Ahzo <Ahzo@tutanota.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Good catch I was trying to hunt that one down as well.
>>>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> Can you apply it to drm-misc?
>
> Sure, but not today. Need to get the kids to bed.

So just pushed to drm-misc-fixes. Do we also need that on 
amd-staging-drm-next?

Christian.

>
> Christian.
>
>>
>> Thanks,
>>
>> Alex
>>
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
>>>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> index c8e359ded1df..44c1e7adfb7c 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>>>> @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct 
>>>> ttm_buffer_object *bo,
>>>>                fbo->base.base.resv = &fbo->base.base._resv;
>>>>
>>>>        dma_resv_init(&fbo->base.base._resv);
>>>> +     fbo->base.base.dev = NULL;
>>>>        ret = dma_resv_trylock(&fbo->base.base._resv);
>>>>        WARN_ON(!ret);
>>>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Daniel Vetter]

On Wed, Feb 26, 2020 at 2:30 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 25.02.20 um 20:12 schrieb Christian König:
> > Am 25.02.20 um 20:11 schrieb Alex Deucher:
> >> On Tue, Feb 25, 2020 at 2:09 PM Christian König
> >> <ckoenig.leichtzumerken@gmail.com> wrote:
> >>> Am 25.02.20 um 19:56 schrieb Alex Deucher:
> >>>> From: Ahzo <Ahzo@tutanota.com>
> >>>>
> >>>> Set the drm_device to NULL, so that the newly created buffer object
> >>>> doesn't appear to use the embedded gem object.
> >>>>
> >>>> This is necessary, because otherwise no corresponding dma_resv_fini
> >>>> for
> >>>> the dma_resv_init is called, resulting in a memory leak.
> >>>>
> >>>> The dma_resv_fini in ttm_bo_release_list is only called if the
> >>>> embedded
> >>>> gem object is not used, which is determined by checking if the
> >>>> drm_device is NULL.
> >>>>
> >>>> Bug:
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F958&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&amp;sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&amp;reserved=0
> >>>> Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
> >>>> Signed-off-by: Ahzo <Ahzo@tutanota.com>
> >>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> Good catch I was trying to hunt that one down as well.
> >>>
> >>> Reviewed-by: Christian König <christian.koenig@amd.com>
> >> Can you apply it to drm-misc?
> >
> > Sure, but not today. Need to get the kids to bed.
>
> So just pushed to drm-misc-fixes. Do we also need that on
> amd-staging-drm-next?

Backmerge -rc4 (once it's released) probably a good idea. Or
cherry-pick with a note why you needed it real quick.
-Daniel

>
> Christian.
>
> >
> > Christian.
> >
> >>
> >> Thanks,
> >>
> >> Alex
> >>
> >>>> ---
> >>>>    drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
> >>>>    1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> index c8e359ded1df..44c1e7adfb7c 100644
> >>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct
> >>>> ttm_buffer_object *bo,
> >>>>                fbo->base.base.resv = &fbo->base.base._resv;
> >>>>
> >>>>        dma_resv_init(&fbo->base.base._resv);
> >>>> +     fbo->base.base.dev = NULL;
> >>>>        ret = dma_resv_trylock(&fbo->base.base._resv);
> >>>>        WARN_ON(!ret);
> >>>>
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: fix leaking fences via ttm_buffer_object_transfer

[Alex Deucher]

On Wed, Feb 26, 2020 at 8:30 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 25.02.20 um 20:12 schrieb Christian König:
> > Am 25.02.20 um 20:11 schrieb Alex Deucher:
> >> On Tue, Feb 25, 2020 at 2:09 PM Christian König
> >> <ckoenig.leichtzumerken@gmail.com> wrote:
> >>> Am 25.02.20 um 19:56 schrieb Alex Deucher:
> >>>> From: Ahzo <Ahzo@tutanota.com>
> >>>>
> >>>> Set the drm_device to NULL, so that the newly created buffer object
> >>>> doesn't appear to use the embedded gem object.
> >>>>
> >>>> This is necessary, because otherwise no corresponding dma_resv_fini
> >>>> for
> >>>> the dma_resv_init is called, resulting in a memory leak.
> >>>>
> >>>> The dma_resv_fini in ttm_bo_release_list is only called if the
> >>>> embedded
> >>>> gem object is not used, which is determined by checking if the
> >>>> drm_device is NULL.
> >>>>
> >>>> Bug:
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F958&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&amp;sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&amp;reserved=0
> >>>> Fixes: 1e053b10ba60 ("drm/ttm: use gem reservation object")
> >>>> Signed-off-by: Ahzo <Ahzo@tutanota.com>
> >>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> Good catch I was trying to hunt that one down as well.
> >>>
> >>> Reviewed-by: Christian König <christian.koenig@amd.com>
> >> Can you apply it to drm-misc?
> >
> > Sure, but not today. Need to get the kids to bed.
>
> So just pushed to drm-misc-fixes. Do we also need that on
> amd-staging-drm-next?

Sure.

Alex

>
> Christian.
>
> >
> > Christian.
> >
> >>
> >> Thanks,
> >>
> >> Alex
> >>
> >>>> ---
> >>>>    drivers/gpu/drm/ttm/ttm_bo_util.c | 1 +
> >>>>    1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> index c8e359ded1df..44c1e7adfb7c 100644
> >>>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> >>>> @@ -514,6 +514,7 @@ static int ttm_buffer_object_transfer(struct
> >>>> ttm_buffer_object *bo,
> >>>>                fbo->base.base.resv = &fbo->base.base._resv;
> >>>>
> >>>>        dma_resv_init(&fbo->base.base._resv);
> >>>> +     fbo->base.base.dev = NULL;
> >>>>        ret = dma_resv_trylock(&fbo->base.base._resv);
> >>>>        WARN_ON(!ret);
> >>>>
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel