| Message ID | 20201201133210.109309-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: stop destroying pinned ghost object | expand |
On Tue, Dec 1, 2020 at 2:32 PM Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > Daniel added a warning for this, but we were abusing that behavior here. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Fixes: 57fcd550eb15 ("drm/ttm: Warn on pinning without holding a reference") > --- > drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 7ccb2295cac1..5bbc1339d28e 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -310,7 +310,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, > kref_init(&fbo->base.kref); > fbo->base.destroy = &ttm_transfered_destroy; > fbo->base.acc_size = 0; > - fbo->base.pin_count = 1; > + fbo->base.pin_count = 0; Was this just to prevent lru reaping, and let the buffer deletion code clean up everything when it's all done? Just kinda freaking out that there's no unpin anywhere ... Anyway tracking ghost objects with the lru like anything else instead of tricks with pin count sounds like a good idea. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> But maybe ask Dave or Thomas for a second check. -Daniel > if (bo->type != ttm_bo_type_sg) > fbo->base.base.resv = &fbo->base.base._resv; > > @@ -319,6 +319,8 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, > ret = dma_resv_trylock(&fbo->base.base._resv); > WARN_ON(!ret); > > + ttm_bo_move_to_lru_tail_unlocked(&fbo->base); > + > *new_obj = &fbo->base; > return 0; > } > -- > 2.25.1 >
Am 01.12.20 um 17:58 schrieb Daniel Vetter: > On Tue, Dec 1, 2020 at 2:32 PM Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> Daniel added a warning for this, but we were abusing that behavior here. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Fixes: 57fcd550eb15 ("drm/ttm: Warn on pinning without holding a reference") >> --- >> drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c >> index 7ccb2295cac1..5bbc1339d28e 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c >> @@ -310,7 +310,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, >> kref_init(&fbo->base.kref); >> fbo->base.destroy = &ttm_transfered_destroy; >> fbo->base.acc_size = 0; >> - fbo->base.pin_count = 1; >> + fbo->base.pin_count = 0; > Was this just to prevent lru reaping, and let the buffer deletion code > clean up everything when it's all done? Just kinda freaking out that > there's no unpin anywhere ... Yeah, didn't realized how this works before either. > Anyway tracking ghost objects with the lru like anything else instead > of tricks with pin count sounds like a good idea. Well I really want to nuke ghost objects which would be an even better idea :) > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Thanks, Christian. > > But maybe ask Dave or Thomas for a second check. > -Daniel > >> if (bo->type != ttm_bo_type_sg) >> fbo->base.base.resv = &fbo->base.base._resv; >> >> @@ -319,6 +319,8 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, >> ret = dma_resv_trylock(&fbo->base.base._resv); >> WARN_ON(!ret); >> >> + ttm_bo_move_to_lru_tail_unlocked(&fbo->base); >> + >> *new_obj = &fbo->base; >> return 0; >> } >> -- >> 2.25.1 >> >
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 7ccb2295cac1..5bbc1339d28e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -310,7 +310,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, kref_init(&fbo->base.kref); fbo->base.destroy = &ttm_transfered_destroy; fbo->base.acc_size = 0; - fbo->base.pin_count = 1; + fbo->base.pin_count = 0; if (bo->type != ttm_bo_type_sg) fbo->base.base.resv = &fbo->base.base._resv; @@ -319,6 +319,8 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, ret = dma_resv_trylock(&fbo->base.base._resv); WARN_ON(!ret); + ttm_bo_move_to_lru_tail_unlocked(&fbo->base); + *new_obj = &fbo->base; return 0; }
Daniel added a warning for this, but we were abusing that behavior here. Signed-off-by: Christian König <christian.koenig@amd.com> Fixes: 57fcd550eb15 ("drm/ttm: Warn on pinning without holding a reference") --- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)