[PATCH v3 6/9] drm: convert drm_device.master_mutex into a rwsem
Desmond Cheong Zhi Xi
desmondcheongzx at gmail.com
Wed Aug 18 07:38:21 UTC 2021
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 at 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
More information about the Linux-kernel-mentees
mailing list