lkml.org 
[lkml]   [2021]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v3 6/9] drm: convert drm_device.master_mutex into a rwsem
    Date
    drm_device.master_mutex currently protects the following:
    - drm_device.master
    - drm_file.master
    - drm_file.was_master
    - drm_file.is_master

    These attributes determine modesetting permissions for drm devices,
    and there is a clear separation between functions that read or change
    the permissions. Hence, convert master_mutex into a rwsem to enable
    concurrent readers.

    Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
    ---
    drivers/gpu/drm/drm_auth.c | 27 ++++++++++++++-------------
    drivers/gpu/drm/drm_debugfs.c | 4 ++--
    drivers/gpu/drm/drm_drv.c | 3 +--
    drivers/gpu/drm/drm_ioctl.c | 4 ++--
    include/drm/drm_device.h | 11 +++++++----
    include/drm/drm_file.h | 12 ++++++------
    6 files changed, 32 insertions(+), 29 deletions(-)

    diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
    index 0acb444fbbac..b65681ff42fc 100644
    --- a/drivers/gpu/drm/drm_auth.c
    +++ b/drivers/gpu/drm/drm_auth.c
    @@ -64,7 +64,7 @@
    static bool drm_is_current_master_locked(struct drm_file *fpriv)
    {
    lockdep_assert_once(lockdep_is_held(&fpriv->minor->dev->master_lookup_lock) ||
    - lockdep_is_held(&fpriv->minor->dev->master_mutex));
    + lockdep_is_held(&fpriv->minor->dev->master_rwsem));

    return (fpriv->is_master && fpriv->master &&
    drm_lease_owner(fpriv->master) == fpriv->minor->dev->master);
    @@ -175,7 +175,7 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
    struct drm_master *old_master;
    struct drm_master *new_master;

    - lockdep_assert_held_once(&dev->master_mutex);
    + lockdep_assert_held_once(&dev->master_rwsem);

    WARN_ON(fpriv->is_master);
    old_master = fpriv->master;
    @@ -257,7 +257,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
    {
    int ret;

    - mutex_lock(&dev->master_mutex);
    + down_write(&dev->master_rwsem);

    ret = drm_master_check_perm(dev, file_priv);
    if (ret)
    @@ -289,7 +289,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,

    drm_set_master(dev, file_priv, false);
    out_unlock:
    - mutex_unlock(&dev->master_mutex);
    + up_write(&dev->master_rwsem);
    return ret;
    }

    @@ -306,7 +306,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
    {
    int ret;

    - mutex_lock(&dev->master_mutex);
    + down_write(&dev->master_rwsem);

    ret = drm_master_check_perm(dev, file_priv);
    if (ret)
    @@ -329,8 +329,9 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
    }

    drm_drop_master(dev, file_priv);
    +
    out_unlock:
    - mutex_unlock(&dev->master_mutex);
    + up_write(&dev->master_rwsem);
    return ret;
    }

    @@ -342,7 +343,7 @@ int drm_master_open(struct drm_file *file_priv)
    /* if there is no current master make this fd it, but do not create
    * any master object for render clients
    */
    - mutex_lock(&dev->master_mutex);
    + down_write(&dev->master_rwsem);
    if (!dev->master) {
    ret = drm_new_set_master(dev, file_priv);
    } else {
    @@ -350,7 +351,7 @@ int drm_master_open(struct drm_file *file_priv)
    file_priv->master = drm_master_get(dev->master);
    spin_unlock(&dev->master_lookup_lock);
    }
    - mutex_unlock(&dev->master_mutex);
    + up_write(&dev->master_rwsem);

    return ret;
    }
    @@ -360,7 +361,7 @@ void drm_master_release(struct drm_file *file_priv)
    struct drm_device *dev = file_priv->minor->dev;
    struct drm_master *master;

    - mutex_lock(&dev->master_mutex);
    + down_write(&dev->master_rwsem);
    master = file_priv->master;

    if (!master)
    @@ -393,7 +394,7 @@ void drm_master_release(struct drm_file *file_priv)
    spin_unlock(&dev->master_lookup_lock);
    }
    unlock:
    - mutex_unlock(&dev->master_mutex);
    + up_write(&dev->master_rwsem);
    }

    /**
    @@ -468,9 +469,9 @@ EXPORT_SYMBOL(drm_master_put);
    /* Used by drm_client and drm_fb_helper */
    bool drm_master_internal_acquire(struct drm_device *dev)
    {
    - mutex_lock(&dev->master_mutex);
    + down_read(&dev->master_rwsem);
    if (dev->master) {
    - mutex_unlock(&dev->master_mutex);
    + up_read(&dev->master_rwsem);
    return false;
    }

    @@ -481,6 +482,6 @@ EXPORT_SYMBOL(drm_master_internal_acquire);
    /* Used by drm_client and drm_fb_helper */
    void drm_master_internal_release(struct drm_device *dev)
    {
    - mutex_unlock(&dev->master_mutex);
    + up_read(&dev->master_rwsem);
    }
    EXPORT_SYMBOL(drm_master_internal_release);
    diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
    index b0a826489488..b34c9c263188 100644
    --- a/drivers/gpu/drm/drm_debugfs.c
    +++ b/drivers/gpu/drm/drm_debugfs.c
    @@ -55,7 +55,7 @@ static int drm_name_info(struct seq_file *m, void *data)
    struct drm_device *dev = minor->dev;
    struct drm_master *master;

    - mutex_lock(&dev->master_mutex);
    + down_read(&dev->master_rwsem);
    master = dev->master;
    seq_printf(m, "%s", dev->driver->name);
    if (dev->dev)
    @@ -65,7 +65,7 @@ static int drm_name_info(struct seq_file *m, void *data)
    if (dev->unique)
    seq_printf(m, " unique=%s", dev->unique);
    seq_printf(m, "\n");
    - mutex_unlock(&dev->master_mutex);
    + up_read(&dev->master_rwsem);

    return 0;
    }
    diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
    index 218c16f11c80..d61079ae6259 100644
    --- a/drivers/gpu/drm/drm_drv.c
    +++ b/drivers/gpu/drm/drm_drv.c
    @@ -570,7 +570,6 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
    /* Prevent use-after-free in drm_managed_release when debugging is
    * enabled. Slightly awkward, but can't really be helped. */
    dev->dev = NULL;
    - mutex_destroy(&dev->master_mutex);
    mutex_destroy(&dev->clientlist_mutex);
    mutex_destroy(&dev->filelist_mutex);
    mutex_destroy(&dev->struct_mutex);
    @@ -611,7 +610,7 @@ static int drm_dev_init(struct drm_device *dev,
    mutex_init(&dev->struct_mutex);
    mutex_init(&dev->filelist_mutex);
    mutex_init(&dev->clientlist_mutex);
    - mutex_init(&dev->master_mutex);
    + init_rwsem(&dev->master_rwsem);
    spin_lock_init(&dev->master_lookup_lock);

    ret = drmm_add_action(dev, drm_dev_init_release, NULL);
    diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
    index e5c3845b6e62..880fc565d599 100644
    --- a/drivers/gpu/drm/drm_ioctl.c
    +++ b/drivers/gpu/drm/drm_ioctl.c
    @@ -390,7 +390,7 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f
    struct drm_set_version *sv = data;
    int if_version, retcode = 0;

    - mutex_lock(&dev->master_mutex);
    + down_read(&dev->master_rwsem);
    if (sv->drm_di_major != -1) {
    if (sv->drm_di_major != DRM_IF_MAJOR ||
    sv->drm_di_minor < 0 || sv->drm_di_minor > DRM_IF_MINOR) {
    @@ -427,7 +427,7 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f
    sv->drm_di_minor = DRM_IF_MINOR;
    sv->drm_dd_major = dev->driver->major;
    sv->drm_dd_minor = dev->driver->minor;
    - mutex_unlock(&dev->master_mutex);
    + up_read(&dev->master_rwsem);

    return retcode;
    }
    diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
    index cf5d15aeb25f..f1ae4570a20a 100644
    --- a/include/drm/drm_device.h
    +++ b/include/drm/drm_device.h
    @@ -107,7 +107,7 @@ struct drm_device {
    * @master:
    *
    * Currently active master for this device.
    - * Protected by &master_mutex
    + * Protected by &master_rwsem
    */
    struct drm_master *master;

    @@ -146,11 +146,14 @@ struct drm_device {
    struct mutex struct_mutex;

    /**
    - * @master_mutex:
    + * @master_rwsem:
    *
    - * Lock for &drm_minor.master and &drm_file.is_master
    + * Synchronizes modesetting rights between multiple users. Users that
    + * can change the modeset or display state must hold a read lock on
    + * @master_rwsem, and users that change modesetting rights should hold
    + * a write lock.
    */
    - struct mutex master_mutex;
    + struct rw_semaphore master_rwsem;

    /**
    * @master_lookup_lock:
    diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
    index 0536e9612a46..ce213ac23c38 100644
    --- a/include/drm/drm_file.h
    +++ b/include/drm/drm_file.h
    @@ -205,7 +205,7 @@ struct drm_file {
    * @was_master:
    *
    * This client has or had, master capability. Protected by struct
    - * &drm_device.master_mutex.
    + * &drm_device.master_rwsem.
    *
    * This is used to ensure that CAP_SYS_ADMIN is not enforced, if the
    * client is or was master in the past.
    @@ -216,7 +216,7 @@ struct drm_file {
    * @is_master:
    *
    * This client is the creator of @master. Protected by struct
    - * &drm_device.master_mutex.
    + * &drm_device.master_rwsem.
    *
    * See also the :ref:`section on primary nodes and authentication
    * <drm_primary_node>`.
    @@ -227,20 +227,20 @@ struct drm_file {
    * @master:
    *
    * Master this node is currently associated with. Protected by struct
    - * &drm_device.master_mutex, and serialized by
    + * &drm_device.master_rwsem, and serialized by
    * &drm_device.master_lookup_lock.
    *
    * Only relevant if drm_is_primary_client() returns true. Note that
    * this only matches &drm_device.master if the master is the currently
    * active one.
    *
    - * To update @master, both &drm_device.master_mutex and
    + * To update @master, both &drm_device.master_rwsem and
    * &drm_device.master_lookup_lock need to be held, therefore holding
    * either of them is safe and enough for the read side.
    *
    * When dereferencing this pointer, either hold struct
    - * &drm_device.master_mutex for the duration of the pointer's use, or
    - * use drm_file_get_master() if struct &drm_device.master_mutex is not
    + * &drm_device.master_rwsem for the duration of the pointer's use, or
    + * use drm_file_get_master() if struct &drm_device.master_rwsem is not
    * currently held and there is no other need to hold it. This prevents
    * @master from being freed during use.
    *
    --
    2.25.1
    \
     
     \ /
      Last update: 2021-08-18 09:42    [W:4.263 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site