search for: ttm_bo_valid

A kernel test bot reported a problem with the locktorture testcase that was triggered by the GEM VRAM helpers. ... [ 10.004734] RIP: 0010:ttm_bo_validate+0x41/0x141 [ttm] ... [ 10.015669] ? kvm_sched_clock_read+0x5/0xd [ 10.016157] ? get_lock_stats+0x11/0x3f [ 10.016607] drm_gem_vram_pin+0x77/0xa2 [drm_vram_helper] [ 10.017229] drm_gem_vram_driver_gem_prime_vmap+0xe/0x39 [drm_vram_helper] [ 10.018015] drm_gem_vmap+0x36/...

A kernel test bot reported a problem with the locktorture testcase that was triggered by the GEM VRAM helpers. ... [ 10.004734] RIP: 0010:ttm_bo_validate+0x41/0x141 [ttm] ... [ 10.015669] ? kvm_sched_clock_read+0x5/0xd [ 10.016157] ? get_lock_stats+0x11/0x3f [ 10.016607] drm_gem_vram_pin+0x77/0xa2 [drm_vram_helper] [ 10.017229] drm_gem_vram_driver_gem_prime_vmap+0xe/0x39 [drm_vram_helper] [ 10.018015] drm_gem_vmap+0x36/...

...0x106/0x3c0 [ttm] [ 1313.811066] ttm_dma_pool_get_pages+0x10a/0x1e0 [ttm] [ 1313.811070] ttm_dma_populate+0x21f/0x2f0 [ttm] [ 1313.811075] ttm_tt_bind+0x2f/0x60 [ttm] [ 1313.811079] ttm_bo_handle_move_mem+0x51f/0x580 [ttm] [ 1313.811084] ? ttm_bo_handle_move_mem+0x5/0x580 [ttm] [ 1313.811088] ttm_bo_validate+0x10c/0x120 [ttm] [ 1313.811092] ? ttm_bo_validate+0x5/0x120 [ttm] [ 1313.811106] ? drm_mode_setcrtc+0x20e/0x540 [drm] [ 1313.811109] ttm_bo_init_reserved+0x290/0x490 [ttm] [ 1313.811114] ttm_bo_init+0x52/0xb0 [ttm] [ 1313.811141] ? nv10_bo_put_tile_region+0x60/0x60 [nouveau] [ 1313.811163]...

...lse, NULL); > > + r = reservation_object_lock_interruptible(bo->gem_base.resv, NULL); > Can you elaborate a bit about how TTM keeps the BOs alive in, for > example, virtio_gpu_transfer_from_host_ioctl? In that function, only > three TTM functions are called: ttm_bo_reserve, ttm_bo_validate, and > ttm_bo_unreserve. I am curious how they keep the BO alive. It can't go away between reserve and unreserve, and I think it also can't be evicted then. Havn't checked how ttm implements that. cheers, Gerd

...lse, NULL); > > + r = reservation_object_lock_interruptible(bo->gem_base.resv, NULL); > Can you elaborate a bit about how TTM keeps the BOs alive in, for > example, virtio_gpu_transfer_from_host_ioctl? In that function, only > three TTM functions are called: ttm_bo_reserve, ttm_bo_validate, and > ttm_bo_unreserve. I am curious how they keep the BO alive. It can't go away between reserve and unreserve, and I think it also can't be evicted then. Havn't checked how ttm implements that. cheers, Gerd

...00003] Call Trace: [ +0.000009] dump_stack+0x9f/0xe1 [ +0.000008] swiotlb_alloc_coherent+0xdf/0x150 [ +0.000010] ttm_dma_pool_get_pages+0x1ec/0x4b0 [ +0.000015] ttm_dma_populate+0x24c/0x340 [ +0.000011] ttm_tt_bind+0x23/0x50 [ +0.000006] ttm_bo_handle_move_mem+0x58c/0x5c0 [ +0.000015] ttm_bo_validate+0x152/0x190 [ +0.000004] ? ttm_bo_init_reserved+0x3d8/0x490 [ +0.000012] ? mutex_trylock+0xcd/0xe0 [ +0.000004] ? ttm_bo_handle_move_mem+0x58/0x5c0 [ +0.000007] ttm_bo_init_reserved+0x3f4/0x490 [ +0.000010] ttm_bo_init+0x2f/0xa0 [ +0.000009] ? nouveau_bo_invalidate_caches+0x10/0x10 [...

...vbo->bo.ttm && nvbo->bo.ttm->caching_state == tt_cached) You don't want to do it also for tt_wc ? > + ttm_dma_tt_cache_sync_for_device((struct ttm_dma_tt *)nvbo->bo.ttm, > + &nouveau_bdev(nvbo->bo.ttm->bdev)->dev->pdev->dev); > + > ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement, > interruptible, no_wait_gpu); > if (ret) > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c > index 830cb7b..f632b92 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > +++ b/dr...

...811066] ttm_dma_pool_get_pages+0x10a/0x1e0 [ttm] > [ 1313.811070] ttm_dma_populate+0x21f/0x2f0 [ttm] > [ 1313.811075] ttm_tt_bind+0x2f/0x60 [ttm] > [ 1313.811079] ttm_bo_handle_move_mem+0x51f/0x580 [ttm] > [ 1313.811084] ? ttm_bo_handle_move_mem+0x5/0x580 [ttm] > [ 1313.811088] ttm_bo_validate+0x10c/0x120 [ttm] > [ 1313.811092] ? ttm_bo_validate+0x5/0x120 [ttm] > [ 1313.811106] ? drm_mode_setcrtc+0x20e/0x540 [drm] > [ 1313.811109] ttm_bo_init_reserved+0x290/0x490 [ttm] > [ 1313.811114] ttm_bo_init+0x52/0xb0 [ttm] > [ 1313.811141] ? nv10_bo_put_tile_region+0x60/0x60...

.....0886f47e5244 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -407,6 +407,8 @@ nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible, > { > int ret; > > + nouveau_bo_sync_for_device(nvbo); > + > ret = ttm_bo_validate(&nvbo->bo, &nvbo->placement, > interruptible, no_wait_gpu); > if (ret) > @@ -487,6 +489,36 @@ nouveau_bo_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags) > return 0; > } > > +#ifdef NOUVEAU_NEED_CACHE_SYNC I don't like this if...

Call reservation_object_* directly instead of using ttm_bo_{reserve,unreserve}. v4: check for EINTR only. v3: check for EINTR too. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git

Call reservation_object_* directly instead of using ttm_bo_{reserve,unreserve}. v4: check for EINTR only. v3: check for EINTR too. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git

...return 0; >>> + } >>> + >>> + drm_gem_vram_placement(gbo, pl_flag); >>> + for (i = 0; i < gbo->placement.num_placement; ++i) >>> + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; >>> + >>> + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); >>> + if (ret < 0) >>> + return ret; >>> + >>> + gbo->pin_count = 1; >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL(drm_gem_vram_pin_reserved); >&...

...Daniel > + > + if (gbo->pin_count) { > + ++gbo->pin_count; > + return 0; > + } > + > + drm_gem_vram_placement(gbo, pl_flag); > + for (i = 0; i < gbo->placement.num_placement; ++i) > + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; > + > + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); > + if (ret < 0) > + return ret; > + > + gbo->pin_count = 1; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_gem_vram_pin_reserved); > + > /** > * drm_gem_vram_unpin() - Unpins a GEM VRAM object > * @...

...Daniel > + > + if (gbo->pin_count) { > + ++gbo->pin_count; > + return 0; > + } > + > + drm_gem_vram_placement(gbo, pl_flag); > + for (i = 0; i < gbo->placement.num_placement; ++i) > + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; > + > + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); > + if (ret < 0) > + return ret; > + > + gbo->pin_count = 1; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_gem_vram_pin_reserved); > + > /** > * drm_gem_vram_unpin() - Unpins a GEM VRAM object > * @...

...eturn 0; > + > + if (gbo->kmap.virtual) > + ttm_bo_kunmap(&gbo->kmap); > + > + drm_gem_vram_placement(gbo, TTM_PL_FLAG_SYSTEM); > + for (i = 0; i < gbo->placement.num_placement ; ++i) > + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; > + > + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); > + if (ret) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_gem_vram_push_to_system); Very simliar to drm_gem_vram_unpin, can't we just call that function? Something like this: drm_gem_vram_push_to_system...

...eturn 0; > + > + if (gbo->kmap.virtual) > + ttm_bo_kunmap(&gbo->kmap); > + > + drm_gem_vram_placement(gbo, TTM_PL_FLAG_SYSTEM); > + for (i = 0; i < gbo->placement.num_placement ; ++i) > + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; > + > + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); > + if (ret) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_gem_vram_push_to_system); Very simliar to drm_gem_vram_unpin, can't we just call that function? Something like this: drm_gem_vram_push_to_system...

...r = reservation_object_lock_interruptible(bo->gem_base.resv, NULL); > > > Can you elaborate a bit about how TTM keeps the BOs alive in, for > > > example, virtio_gpu_transfer_from_host_ioctl? In that function, only > > > three TTM functions are called: ttm_bo_reserve, ttm_bo_validate, and > > > ttm_bo_unreserve. I am curious how they keep the BO alive. > > > > It can't go away between reserve and unreserve, and I think it also > > can't be evicted then. Havn't checked how ttm implements that. > Hm, but the vbuf using the BO outlive...

On Thu, 2018-05-10 at 12:28 +0200, Mike Galbraith wrote: > On Thu, 2018-05-10 at 11:10 +0200, Mike Galbraith wrote: > > Greetings, > > > > When box is earning its keep, nouveau/swiotlb grumble.. a LOT. The > > below is from master.today. > > > > [12594.640959] nouveau 0000:01:00.0: swiotlb buffer is full (sz: 2097152 bytes) > > [12594.693000] nouveau

...int i, ret; + struct ttm_operation_ctx ctx = { false, false }; + + if (gbo->pin_count) { + ++gbo->pin_count; + return 0; + } + + drm_gem_vram_placement(gbo, pl_flag); + for (i = 0; i < gbo->placement.num_placement; ++i) + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; + + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); + if (ret < 0) + return ret; + + gbo->pin_count = 1; + + return 0; +} +EXPORT_SYMBOL(drm_gem_vram_pin_reserved); + /** * drm_gem_vram_unpin() - Unpins a GEM VRAM object * @gbo: the GEM VRAM object @@ -285,6 +326,40 @@ int drm_gem_vr...

...>> [ +0.000008] swiotlb_alloc_coherent+0xdf/0x150 >> [ +0.000010] ttm_dma_pool_get_pages+0x1ec/0x4b0 >> [ +0.000015] ttm_dma_populate+0x24c/0x340 >> [ +0.000011] ttm_tt_bind+0x23/0x50 >> [ +0.000006] ttm_bo_handle_move_mem+0x58c/0x5c0 >> [ +0.000015] ttm_bo_validate+0x152/0x190 >> [ +0.000004] ? ttm_bo_init_reserved+0x3d8/0x490 >> [ +0.000012] ? mutex_trylock+0xcd/0xe0 >> [ +0.000004] ? ttm_bo_handle_move_mem+0x58/0x5c0 >> [ +0.000007] ttm_bo_init_reserved+0x3f4/0x490 >> [ +0.000010] ttm_bo_init+0x2f/0xa0 >> [...