| Message ID | 20231029230205.93277-15-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 Mon, 30 Oct 2023 02:01:53 +0300 Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > To simplify the drm-shmem refcnt handling, we're moving away from > the implicit get_pages() that is used by get_pages_sgt(). From now on > drivers will have to pin pages while they use sgt. Lima driver doesn't > have shrinker, hence pages are pinned and sgt is valid as long as pages' > use-count > 0. > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/lima/lima_gem.c | 18 ++++++++++++++++-- > drivers/gpu/drm/lima/lima_gem.h | 1 + > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c > index 988e74f67465..d255f5775dac 100644 > --- a/drivers/gpu/drm/lima/lima_gem.c > +++ b/drivers/gpu/drm/lima/lima_gem.c > @@ -46,6 +46,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm) > return -ENOMEM; > } > > + bo->put_pages = true; > bo->base.pages = pages; > refcount_set(&bo->base.pages_use_count, 1); > > @@ -115,6 +116,7 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, > return PTR_ERR(shmem); > > obj = &shmem->base; > + bo = to_lima_bo(obj); > > /* Mali Utgard GPU can only support 32bit address space */ > mask = mapping_gfp_mask(obj->filp->f_mapping); > @@ -123,13 +125,19 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, > mapping_set_gfp_mask(obj->filp->f_mapping, mask); > > if (is_heap) { > - bo = to_lima_bo(obj); > err = lima_heap_alloc(bo, NULL); > if (err) > goto out; > } else { > - struct sg_table *sgt = drm_gem_shmem_get_pages_sgt(shmem); > + struct sg_table *sgt; > + > + err = drm_gem_shmem_get_pages(shmem); > + if (err) > + goto out; > + > + bo->put_pages = true; > > + sgt = drm_gem_shmem_get_pages_sgt(shmem); > if (IS_ERR(sgt)) { > err = PTR_ERR(sgt); > goto out; Pretty sure we don't need this put_pages flag. We can either check ba->base.base.pages or refcount_read(&bo->base.pages_use_count). Or, even better, if it's just used in the error path of the same function, simply have a dedicated error path for that case: drm_gem_object_put(obj); return 0; err_put_pages: if (!is_heap) drm_gem_shmem_put_pages(shmem); err_put_bo: drm_gem_object_put(obj); return err; } > @@ -139,6 +147,9 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, > err = drm_gem_handle_create(file, obj, handle); > > out: > + if (err && bo->put_pages) > + drm_gem_shmem_put_pages(shmem); > + > /* drop reference from allocate - handle holds it now */ > drm_gem_object_put(obj); > > @@ -152,6 +163,9 @@ static void lima_gem_free_object(struct drm_gem_object *obj) > if (!list_empty(&bo->va)) > dev_err(obj->dev->dev, "lima gem free bo still has va\n"); > > + if (bo->put_pages) > + drm_gem_shmem_put_pages(&bo->base); This one can be replaced by if (!is_heap || bo->base.base.pages) drm_gem_shmem_put_pages(&bo->base); > + > drm_gem_shmem_free(&bo->base); > } > > diff --git a/drivers/gpu/drm/lima/lima_gem.h b/drivers/gpu/drm/lima/lima_gem.h > index ccea06142f4b..dc5a6d465c80 100644 > --- a/drivers/gpu/drm/lima/lima_gem.h > +++ b/drivers/gpu/drm/lima/lima_gem.h > @@ -16,6 +16,7 @@ struct lima_bo { > struct list_head va; > > size_t heap_size; > + bool put_pages; > }; > > static inline struct lima_bo *
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c index 988e74f67465..d255f5775dac 100644 --- a/drivers/gpu/drm/lima/lima_gem.c +++ b/drivers/gpu/drm/lima/lima_gem.c @@ -46,6 +46,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm) return -ENOMEM; } + bo->put_pages = true; bo->base.pages = pages; refcount_set(&bo->base.pages_use_count, 1); @@ -115,6 +116,7 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, return PTR_ERR(shmem); obj = &shmem->base; + bo = to_lima_bo(obj); /* Mali Utgard GPU can only support 32bit address space */ mask = mapping_gfp_mask(obj->filp->f_mapping); @@ -123,13 +125,19 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, mapping_set_gfp_mask(obj->filp->f_mapping, mask); if (is_heap) { - bo = to_lima_bo(obj); err = lima_heap_alloc(bo, NULL); if (err) goto out; } else { - struct sg_table *sgt = drm_gem_shmem_get_pages_sgt(shmem); + struct sg_table *sgt; + + err = drm_gem_shmem_get_pages(shmem); + if (err) + goto out; + + bo->put_pages = true; + sgt = drm_gem_shmem_get_pages_sgt(shmem); if (IS_ERR(sgt)) { err = PTR_ERR(sgt); goto out; @@ -139,6 +147,9 @@ int lima_gem_create_handle(struct drm_device *dev, struct drm_file *file, err = drm_gem_handle_create(file, obj, handle); out: + if (err && bo->put_pages) + drm_gem_shmem_put_pages(shmem); + /* drop reference from allocate - handle holds it now */ drm_gem_object_put(obj); @@ -152,6 +163,9 @@ static void lima_gem_free_object(struct drm_gem_object *obj) if (!list_empty(&bo->va)) dev_err(obj->dev->dev, "lima gem free bo still has va\n"); + if (bo->put_pages) + drm_gem_shmem_put_pages(&bo->base); + drm_gem_shmem_free(&bo->base); } diff --git a/drivers/gpu/drm/lima/lima_gem.h b/drivers/gpu/drm/lima/lima_gem.h index ccea06142f4b..dc5a6d465c80 100644 --- a/drivers/gpu/drm/lima/lima_gem.h +++ b/drivers/gpu/drm/lima/lima_gem.h @@ -16,6 +16,7 @@ struct lima_bo { struct list_head va; size_t heap_size; + bool put_pages; }; static inline struct lima_bo *
To simplify the drm-shmem refcnt handling, we're moving away from the implicit get_pages() that is used by get_pages_sgt(). From now on drivers will have to pin pages while they use sgt. Lima driver doesn't have shrinker, hence pages are pinned and sgt is valid as long as pages' use-count > 0. Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- drivers/gpu/drm/lima/lima_gem.c | 18 ++++++++++++++++-- drivers/gpu/drm/lima/lima_gem.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-)