| Message ID | 20230903170736.513347-10-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 Sun, 3 Sep 2023 20:07:25 +0300 Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > Everything that uses the mapped buffer should be agnostic to is_iomem. > The only reason for the is_iomem test is that we're setting shmem->vaddr > to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove > the obsoleted is_iomem test to clean up the code. > > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 2b50d1a7f718..25e99468ced2 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -317,12 +317,6 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, > > if (obj->import_attach) { > ret = dma_buf_vmap(obj->import_attach->dmabuf, map); > - if (!ret) { > - if (drm_WARN_ON(obj->dev, map->is_iomem)) { > - dma_buf_vunmap(obj->import_attach->dmabuf, map); > - return -EIO; > - } > - } Given there's nothing to unroll for the dmabuf case, I think it'd be good to return directly and skip all the error paths. It would also allow you to get rid of one indentation level for the !dmabuf path. if (obj->import_attach) return dma_buf_vmap(obj->import_attach->dmabuf, map); // non-dmabuf vmap logic here... > } else { > pgprot_t prot = PAGE_KERNEL; >
On 9/5/23 09:46, Boris Brezillon wrote: > On Sun, 3 Sep 2023 20:07:25 +0300 > Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > >> Everything that uses the mapped buffer should be agnostic to is_iomem. >> The only reason for the is_iomem test is that we're setting shmem->vaddr >> to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove >> the obsoleted is_iomem test to clean up the code. >> >> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> >> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> --- >> drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >> index 2b50d1a7f718..25e99468ced2 100644 >> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >> @@ -317,12 +317,6 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, >> >> if (obj->import_attach) { >> ret = dma_buf_vmap(obj->import_attach->dmabuf, map); >> - if (!ret) { >> - if (drm_WARN_ON(obj->dev, map->is_iomem)) { >> - dma_buf_vunmap(obj->import_attach->dmabuf, map); >> - return -EIO; >> - } >> - } > > Given there's nothing to unroll for the dmabuf case, I think it'd be > good to return directly and skip all the error paths. It would also > allow you to get rid of one indentation level for the !dmabuf path. > > if (obj->import_attach) > return dma_buf_vmap(obj->import_attach->dmabuf, map); > > // non-dmabuf vmap logic here... There is a common error message there that uses the common ret. The error unwinding could be improved, but then it should be a separate patch as it's unrelated to the change made here.
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 2b50d1a7f718..25e99468ced2 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -317,12 +317,6 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem, if (obj->import_attach) { ret = dma_buf_vmap(obj->import_attach->dmabuf, map); - if (!ret) { - if (drm_WARN_ON(obj->dev, map->is_iomem)) { - dma_buf_vunmap(obj->import_attach->dmabuf, map); - return -EIO; - } - } } else { pgprot_t prot = PAGE_KERNEL;
Everything that uses the mapped buffer should be agnostic to is_iomem. The only reason for the is_iomem test is that we're setting shmem->vaddr to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove the obsoleted is_iomem test to clean up the code. Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ------ 1 file changed, 6 deletions(-)