Re: [PATCH v1] drm/gem-vram: Fix deadlock in drm_gem_vram_vmap()
On 11/14/22 02:38, Dmitry Osipenko wrote: > Recently DRM framebuffer core and all drivers were moved to unlocked > vmapping functions that take the reservation lock. The drm_gem_vram_vmap() > was missed out by accident and now deadlocks drivers that use drm_gem_vram > helpers when framebuffer is updated, like Bochs driver. Remove the locking > from drm_gem_vram_v[un]map() functions to fix the deadlock. > > Reported-by: Dmitry Vyukov > Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap > operations") > Signed-off-by: Dmitry Osipenko > --- > drivers/gpu/drm/drm_gem_vram_helper.c | 18 +++--- > 1 file changed, 3 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c > b/drivers/gpu/drm/drm_gem_vram_helper.c > index 125160b534be..b6c7e3803bb3 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -433,25 +433,19 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, > struct iosys_map *map) > { > int ret; > > - ret = ttm_bo_reserve(>bo, true, false, NULL); > - if (ret) > - return ret; > + dma_resv_assert_held(gbo->bo.base.resv); > > ret = drm_gem_vram_pin_locked(gbo, 0); > if (ret) > - goto err_ttm_bo_unreserve; > + return ret; > ret = drm_gem_vram_kmap_locked(gbo, map); > if (ret) > goto err_drm_gem_vram_unpin_locked; > > - ttm_bo_unreserve(>bo); > - > return 0; > > err_drm_gem_vram_unpin_locked: > drm_gem_vram_unpin_locked(gbo); > -err_ttm_bo_unreserve: > - ttm_bo_unreserve(>bo); > return ret; > } > EXPORT_SYMBOL(drm_gem_vram_vmap); > @@ -467,16 +461,10 @@ EXPORT_SYMBOL(drm_gem_vram_vmap); > void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, >struct iosys_map *map) > { > - int ret; > - > - ret = ttm_bo_reserve(>bo, false, false, NULL); > - if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret)) > - return; > + dma_resv_assert_held(gbo->bo.base.resv); > > drm_gem_vram_kunmap_locked(gbo, map); > drm_gem_vram_unpin_locked(gbo); > - > - ttm_bo_unreserve(>bo); > } > EXPORT_SYMBOL(drm_gem_vram_vunmap); > Applied to drm-misc-next -- Best regards, Dmitry
Re: [PATCH v1] drm/gem-vram: Fix deadlock in drm_gem_vram_vmap()
Am 14.11.22 um 00:38 schrieb Dmitry Osipenko: Recently DRM framebuffer core and all drivers were moved to unlocked vmapping functions that take the reservation lock. The drm_gem_vram_vmap() was missed out by accident and now deadlocks drivers that use drm_gem_vram helpers when framebuffer is updated, like Bochs driver. Remove the locking from drm_gem_vram_v[un]map() functions to fix the deadlock. Reported-by: Dmitry Vyukov Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations") Signed-off-by: Dmitry Osipenko Reviewed-by: Christian König --- drivers/gpu/drm/drm_gem_vram_helper.c | 18 +++--- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 125160b534be..b6c7e3803bb3 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -433,25 +433,19 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map) { int ret; - ret = ttm_bo_reserve(>bo, true, false, NULL); - if (ret) - return ret; + dma_resv_assert_held(gbo->bo.base.resv); ret = drm_gem_vram_pin_locked(gbo, 0); if (ret) - goto err_ttm_bo_unreserve; + return ret; ret = drm_gem_vram_kmap_locked(gbo, map); if (ret) goto err_drm_gem_vram_unpin_locked; - ttm_bo_unreserve(>bo); - return 0; err_drm_gem_vram_unpin_locked: drm_gem_vram_unpin_locked(gbo); -err_ttm_bo_unreserve: - ttm_bo_unreserve(>bo); return ret; } EXPORT_SYMBOL(drm_gem_vram_vmap); @@ -467,16 +461,10 @@ EXPORT_SYMBOL(drm_gem_vram_vmap); void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, struct iosys_map *map) { - int ret; - - ret = ttm_bo_reserve(>bo, false, false, NULL); - if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret)) - return; + dma_resv_assert_held(gbo->bo.base.resv); drm_gem_vram_kunmap_locked(gbo, map); drm_gem_vram_unpin_locked(gbo); - - ttm_bo_unreserve(>bo); } EXPORT_SYMBOL(drm_gem_vram_vunmap);
[PATCH v1] drm/gem-vram: Fix deadlock in drm_gem_vram_vmap()
Recently DRM framebuffer core and all drivers were moved to unlocked vmapping functions that take the reservation lock. The drm_gem_vram_vmap() was missed out by accident and now deadlocks drivers that use drm_gem_vram helpers when framebuffer is updated, like Bochs driver. Remove the locking from drm_gem_vram_v[un]map() functions to fix the deadlock. Reported-by: Dmitry Vyukov Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations") Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/drm_gem_vram_helper.c | 18 +++--- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 125160b534be..b6c7e3803bb3 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -433,25 +433,19 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map) { int ret; - ret = ttm_bo_reserve(>bo, true, false, NULL); - if (ret) - return ret; + dma_resv_assert_held(gbo->bo.base.resv); ret = drm_gem_vram_pin_locked(gbo, 0); if (ret) - goto err_ttm_bo_unreserve; + return ret; ret = drm_gem_vram_kmap_locked(gbo, map); if (ret) goto err_drm_gem_vram_unpin_locked; - ttm_bo_unreserve(>bo); - return 0; err_drm_gem_vram_unpin_locked: drm_gem_vram_unpin_locked(gbo); -err_ttm_bo_unreserve: - ttm_bo_unreserve(>bo); return ret; } EXPORT_SYMBOL(drm_gem_vram_vmap); @@ -467,16 +461,10 @@ EXPORT_SYMBOL(drm_gem_vram_vmap); void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, struct iosys_map *map) { - int ret; - - ret = ttm_bo_reserve(>bo, false, false, NULL); - if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret)) - return; + dma_resv_assert_held(gbo->bo.base.resv); drm_gem_vram_kunmap_locked(gbo, map); drm_gem_vram_unpin_locked(gbo); - - ttm_bo_unreserve(>bo); } EXPORT_SYMBOL(drm_gem_vram_vunmap); -- 2.37.3