[PATCH v7 2/7] drm: convert drm_device.master_mutex into a rwsem
From: Desmond Cheong Zhi Xi
Date: Wed Aug 25 2021 - 10:23:33 EST
drm_device.master_mutex currently protects the following:
- drm_device.master
- drm_file.master
- drm_file.was_master
- drm_file.is_master
- drm_master.unique
- drm_master.unique_len
- drm_master.magic_map
There is a clear separation between functions that read or change
these attributes. Hence, convert master_mutex into a rwsem to enable
concurrent readers.
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@xxxxxxxxx>
---
drivers/gpu/drm/drm_auth.c | 35 ++++++++++++++++++-----------------
drivers/gpu/drm/drm_debugfs.c | 4 ++--
drivers/gpu/drm/drm_drv.c | 3 +--
drivers/gpu/drm/drm_ioctl.c | 10 +++++-----
include/drm/drm_auth.h | 6 +++---
include/drm/drm_device.h | 10 ++++++----
include/drm/drm_file.h | 12 ++++++------
7 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 60a6b21474b1..73ade0513ccb 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->master_lookup_lock) ||
- lockdep_is_held(&fpriv->minor->dev->master_mutex));
+ lockdep_is_held(&fpriv->minor->dev->master_rwsem));
return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;
}
@@ -96,7 +96,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
struct drm_auth *auth = data;
int ret = 0;
- mutex_lock(&dev->master_mutex);
+ down_write(&dev->master_rwsem);
if (!file_priv->magic) {
ret = idr_alloc(&file_priv->master->magic_map, file_priv,
1, 0, GFP_KERNEL);
@@ -104,7 +104,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
file_priv->magic = ret;
}
auth->magic = file_priv->magic;
- mutex_unlock(&dev->master_mutex);
+ up_write(&dev->master_rwsem);
DRM_DEBUG("%u\n", auth->magic);
@@ -119,13 +119,13 @@ int drm_authmagic(struct drm_device *dev, void *data,
DRM_DEBUG("%u\n", auth->magic);
- mutex_lock(&dev->master_mutex);
+ down_write(&dev->master_rwsem);
file = idr_find(&file_priv->master->magic_map, auth->magic);
if (file) {
file->authenticated = 1;
idr_replace(&file_priv->master->magic_map, NULL, auth->magic);
}
- mutex_unlock(&dev->master_mutex);
+ up_write(&dev->master_rwsem);
return file ? 0 : -EINVAL;
}
@@ -167,7 +167,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;
@@ -249,7 +249,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)
@@ -281,7 +281,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;
}
@@ -298,7 +298,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)
@@ -321,8 +321,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;
}
@@ -334,7 +335,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 {
@@ -342,7 +343,7 @@ int drm_master_open(struct drm_file *file_priv)
file_priv->master = drm_master_get(dev->master);
spin_unlock(&file_priv->master_lookup_lock);
}
- mutex_unlock(&dev->master_mutex);
+ up_write(&dev->master_rwsem);
return ret;
}
@@ -352,7 +353,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 (file_priv->magic)
idr_remove(&file_priv->master->magic_map, file_priv->magic);
@@ -375,7 +376,7 @@ void drm_master_release(struct drm_file *file_priv)
/* drop the master reference held by the file priv */
if (file_priv->master)
drm_master_put(&file_priv->master);
- mutex_unlock(&dev->master_mutex);
+ up_write(&dev->master_rwsem);
}
/**
@@ -450,9 +451,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;
}
@@ -463,6 +464,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 7a5097467ba5..4556bf42954c 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);
ret = drmm_add_action(dev, drm_dev_init_release, NULL);
if (ret)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 26f3a9ede8fe..d25713b09b80 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -119,16 +119,16 @@ int drm_getunique(struct drm_device *dev, void *data,
struct drm_unique *u = data;
struct drm_master *master;
- mutex_lock(&dev->master_mutex);
+ down_read(&dev->master_rwsem);
master = file_priv->master;
if (u->unique_len >= master->unique_len) {
if (copy_to_user(u->unique, master->unique, master->unique_len)) {
- mutex_unlock(&dev->master_mutex);
+ up_read(&dev->master_rwsem);
return -EFAULT;
}
}
u->unique_len = master->unique_len;
- mutex_unlock(&dev->master_mutex);
+ up_read(&dev->master_rwsem);
return 0;
}
@@ -385,7 +385,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_write(&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) {
@@ -420,7 +420,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_write(&dev->master_rwsem);
return retcode;
}
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index ba248ca8866f..f0a89e5fcaad 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -67,17 +67,17 @@ struct drm_master {
struct drm_device *dev;
/**
* @unique: Unique identifier: e.g. busid. Protected by
- * &drm_device.master_mutex.
+ * &drm_device.master_rwsem.
*/
char *unique;
/**
* @unique_len: Length of unique field. Protected by
- * &drm_device.master_mutex.
+ * &drm_device.master_rwsem.
*/
int unique_len;
/**
* @magic_map: Map of used authentication tokens. Protected by
- * &drm_device.master_mutex.
+ * &drm_device.master_rwsem.
*/
struct idr magic_map;
void *driver_priv;
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 604b1d1b2d72..142fb2f6e74d 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,13 @@ struct drm_device {
struct mutex struct_mutex;
/**
- * @master_mutex:
+ * @master_rwsem:
*
- * Lock for &drm_minor.master and &drm_file.is_master
+ * Lock for &drm_device.master, &drm_file.was_master,
+ * &drm_file.is_master, &drm_file.master, &drm_master.unique,
+ * &drm_master.unique_len, and &drm_master.magic_map.
*/
- struct mutex master_mutex;
+ struct rw_semaphore master_rwsem;
/**
* @open_count:
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index a3acb7ac3550..d12bb2ba7814 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,19 +227,19 @@ struct drm_file {
* @master:
*
* Master this node is currently associated with. Protected by struct
- * &drm_device.master_mutex, and serialized by @master_lookup_lock.
+ * &drm_device.master_rwsem, and serialized by @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
* @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