| Message ID | 20230914232721.408581-12-dmitry.osipenko@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers | expand |
On Fri, 15 Sep 2023 02:27:14 +0300 Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > Remove error unwinding from drm_gem_shmem_vmap_locked() making error > paths consistent for both dmabuf and non-dmabuf cases and improving > readability of the code. Often it's preferred to do the explicit error > unwinding, but this multi-path function is an exception. > > Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> Actually, I suggested returning directly, but I hadn't noticed the debug message which you pointed in one of your replies. No longer sure it's a good idea to drop the error path, since it makes code addition more error-prone (pretty easy to forget the drm_gem_shmem_unpin_locked() in new error paths). > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index e1fcb5154209..8a8eab4d0332 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -334,7 +334,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, > struct iosys_map *map) > { > struct drm_gem_object *obj = &shmem->base; > - int ret = 0; > + int ret; > > if (obj->import_attach) { > ret = dma_buf_vmap(obj->import_attach->dmabuf, map); > @@ -357,6 +357,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, > shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, > VM_MAP, prot); > if (!shmem->vaddr) { > + drm_gem_shmem_unpin_locked(shmem); > ret = -ENOMEM; > } else { > iosys_map_set_vaddr(map, shmem->vaddr); > @@ -364,16 +365,8 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, > } > } > > - if (ret) { > + if (ret) > drm_dbg_kms(obj->dev, "Failed to vmap pages, error %d\n", ret); > - goto err_put_pages; > - } > - > - return 0; > - > -err_put_pages: > - if (!obj->import_attach) > - drm_gem_shmem_unpin_locked(shmem); > > return ret; > }
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index e1fcb5154209..8a8eab4d0332 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -334,7 +334,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, struct iosys_map *map) { struct drm_gem_object *obj = &shmem->base; - int ret = 0; + int ret; if (obj->import_attach) { ret = dma_buf_vmap(obj->import_attach->dmabuf, map); @@ -357,6 +357,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, VM_MAP, prot); if (!shmem->vaddr) { + drm_gem_shmem_unpin_locked(shmem); ret = -ENOMEM; } else { iosys_map_set_vaddr(map, shmem->vaddr); @@ -364,16 +365,8 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, } } - if (ret) { + if (ret) drm_dbg_kms(obj->dev, "Failed to vmap pages, error %d\n", ret); - goto err_put_pages; - } - - return 0; - -err_put_pages: - if (!obj->import_attach) - drm_gem_shmem_unpin_locked(shmem); return ret; }
Remove error unwinding from drm_gem_shmem_vmap_locked() making error paths consistent for both dmabuf and non-dmabuf cases and improving readability of the code. Often it's preferred to do the explicit error unwinding, but this multi-path function is an exception. Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- drivers/gpu/drm/drm_gem_shmem_helper.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)