On Tue, Mar 27, 2018 at 10:23:54AM +0200, Daniel Vetter wrote:
> It's only used to protect our page list, and only when we know we have
> a full reference. This means none of these code paths can ever race
> with the final unref, and hence we do not need dev->struct_mutex
> serialization and can simply switch to our own locking.
>
> For more context the only magic the locked gem_free_object provides is
> that it prevents concurrent final unref (and destruction) of gem
> objects while anyone is holding dev->struct_mutex. This was used by
> i915 (and other drivers) to implement eviction handling with less
> headaches.
>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Reviewed-by: Sean Paul <seanp...@chromium.org>
> Cc: Dave Airlie <airl...@redhat.com>
> ---
> drivers/gpu/drm/udl/udl_dmabuf.c | 5 +++--
> drivers/gpu/drm/udl/udl_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.h | 2 ++
> drivers/gpu/drm/udl/udl_gem.c | 5 +++--
> drivers/gpu/drm/udl/udl_main.c | 2 ++
> 5 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c
> b/drivers/gpu/drm/udl/udl_dmabuf.c
> index 2867ed155ff6..0a20695eb120 100644
> --- a/drivers/gpu/drm/udl/udl_dmabuf.c
> +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> @@ -76,6 +76,7 @@ static struct sg_table *udl_map_dma_buf(struct
> dma_buf_attachment *attach,
> struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
> struct udl_gem_object *obj = to_udl_bo(attach->dmabuf->priv);
> struct drm_device *dev = obj->base.dev;
> + struct udl_device *udl = dev->dev_private;
> struct scatterlist *rd, *wr;
> struct sg_table *sgt = NULL;
> unsigned int i;
> @@ -112,7 +113,7 @@ static struct sg_table *udl_map_dma_buf(struct
> dma_buf_attachment *attach,
> return ERR_PTR(-ENOMEM);
> }
>
> - mutex_lock(&dev->struct_mutex);
> + mutex_lock(&udl->gem_lock);
>
> rd = obj->sg->sgl;
> wr = sgt->sgl;
> @@ -137,7 +138,7 @@ static struct sg_table *udl_map_dma_buf(struct
> dma_buf_attachment *attach,
> attach->priv = udl_attach;
>
> err_unlock:
> - mutex_unlock(&dev->struct_mutex);
> + mutex_unlock(&udl->gem_lock);
> return sgt;
> }
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 3c45a3064726..9ef515df724b 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -53,7 +53,7 @@ static struct drm_driver driver = {
> .unload = udl_driver_unload,
>
> /* gem hooks */
> - .gem_free_object = udl_gem_free_object,
> + .gem_free_object_unlocked = udl_gem_free_object,
> .gem_vm_ops = &udl_gem_vm_ops,
>
> .dumb_create = udl_dumb_create,
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 2a75ab80527a..55c0cc309198 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -54,6 +54,8 @@ struct udl_device {
> struct usb_device *udev;
> struct drm_crtc *crtc;
>
> + struct mutex gem_lock;
> +
> int sku_pixel_limit;
>
> struct urb_list urbs;
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index dee6bd9a3dd1..9a15cce22cce 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -214,9 +214,10 @@ int udl_gem_mmap(struct drm_file *file, struct
> drm_device *dev,
> {
> struct udl_gem_object *gobj;
> struct drm_gem_object *obj;
> + struct udl_device *udl = dev->dev_private;
> int ret = 0;
>
> - mutex_lock(&dev->struct_mutex);
> + mutex_lock(&udl->gem_lock);
> obj = drm_gem_object_lookup(file, handle);
> if (obj == NULL) {
> ret = -ENOENT;
> @@ -236,6 +237,6 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device
> *dev,
> out:
> drm_gem_object_put(&gobj->base);
> unlock:
> - mutex_unlock(&dev->struct_mutex);
> + mutex_unlock(&udl->gem_lock);
> return ret;
> }
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index f1ec4528a73e..d518de8f496b 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -324,6 +324,8 @@ int udl_driver_load(struct drm_device *dev, unsigned long
> flags)
> udl->ddev = dev;
> dev->dev_private = udl;
>
> + mutex_init(&udl->gem_lock);
> +
> if (!udl_parse_vendor_descriptor(dev, udl->udev)) {
> ret = -ENODEV;
> DRM_ERROR("firmware not recognized. Assume incompatible
> device\n");
> --
> 2.16.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
