| Message ID | 20200225185614.1058688-1-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: fix leaking fences via ttm_buffer_object_transfer | expand |
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); >
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); > > >
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&data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&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); >>>
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&data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&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
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&data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&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
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&data=02%7C01%7Cchristian.koenig%40amd.com%7Caa3f774da03e4dfcc09a08d7ba268305%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637182546879144030&sdata=N8WxYZZRj5obMU5SLv0giog0a1sCYi%2FouxSuWNom0gw%3D&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 >
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);