Commit 95c081c1 authored by Daniel Vetter's avatar Daniel Vetter

drm: Move master pointer from drm_minor to drm_device

There can only be one current master, and it's for the overall device.
Render/control minors don't support master-based auth at all.

This simplifies the master logic a lot, at least in my eyes: All these
additional pointer chases are just confusing.

While doing the conversion I spotted some locking fail:
- drm_lock/drm_auth check dev->master without holding the
  master_mutex. This is fallout from

  commit c996fd0b
  Author: Thomas Hellstrom <thellstrom@vmware.com>
  Date:   Tue Feb 25 19:57:44 2014 +0100

      drm: Protect the master management with a drm_device::master_mutex v3

  but I honestly don't care one bit about those old legacy drivers
  using this.

- debugfs name info should just grab master_mutex.

- And the fbdev helper looked at it to figure out whether someone is
  using KMS. We just need a consistent value, so READ_ONCE. Aside: We
  should probably check if anyone has opened a control node too, but I
  guess current userspace doesn't really do that yet.

v2: Balance locking, reported by Julia.

v3: Rebase on top of Chris' oops fixes.

Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> (v2)
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1466499262-18717-1-git-send-email-daniel.vetter@ffwll.ch
parent 84601dbd
...@@ -128,13 +128,13 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) ...@@ -128,13 +128,13 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
lockdep_assert_held_once(&dev->master_mutex); lockdep_assert_held_once(&dev->master_mutex);
/* create a new master */ /* create a new master */
fpriv->minor->master = drm_master_create(fpriv->minor->dev); dev->master = drm_master_create(dev);
if (!fpriv->minor->master) if (!dev->master)
return -ENOMEM; return -ENOMEM;
/* take another reference for the copy in the local file priv */ /* take another reference for the copy in the local file priv */
old_master = fpriv->master; old_master = fpriv->master;
fpriv->master = drm_master_get(fpriv->minor->master); fpriv->master = drm_master_get(dev->master);
if (dev->driver->master_create) { if (dev->driver->master_create) {
ret = dev->driver->master_create(dev, fpriv->master); ret = dev->driver->master_create(dev, fpriv->master);
...@@ -157,7 +157,7 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) ...@@ -157,7 +157,7 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
out_err: out_err:
/* drop both references and restore old master on failure */ /* drop both references and restore old master on failure */
drm_master_put(&fpriv->minor->master); drm_master_put(&dev->master);
drm_master_put(&fpriv->master); drm_master_put(&fpriv->master);
fpriv->master = old_master; fpriv->master = old_master;
...@@ -173,7 +173,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, ...@@ -173,7 +173,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
if (file_priv->is_master) if (file_priv->is_master)
goto out_unlock; goto out_unlock;
if (file_priv->minor->master) { if (dev->master) {
ret = -EINVAL; ret = -EINVAL;
goto out_unlock; goto out_unlock;
} }
...@@ -188,13 +188,13 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, ...@@ -188,13 +188,13 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
goto out_unlock; goto out_unlock;
} }
file_priv->minor->master = drm_master_get(file_priv->master); dev->master = drm_master_get(file_priv->master);
file_priv->is_master = 1; file_priv->is_master = 1;
if (dev->driver->master_set) { if (dev->driver->master_set) {
ret = dev->driver->master_set(dev, file_priv, false); ret = dev->driver->master_set(dev, file_priv, false);
if (unlikely(ret != 0)) { if (unlikely(ret != 0)) {
file_priv->is_master = 0; file_priv->is_master = 0;
drm_master_put(&file_priv->minor->master); drm_master_put(&dev->master);
} }
} }
...@@ -212,13 +212,13 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, ...@@ -212,13 +212,13 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
if (!file_priv->is_master) if (!file_priv->is_master)
goto out_unlock; goto out_unlock;
if (!file_priv->minor->master) if (!dev->master)
goto out_unlock; goto out_unlock;
ret = 0; ret = 0;
if (dev->driver->master_drop) if (dev->driver->master_drop)
dev->driver->master_drop(dev, file_priv, false); dev->driver->master_drop(dev, file_priv, false);
drm_master_put(&file_priv->minor->master); drm_master_put(&dev->master);
file_priv->is_master = 0; file_priv->is_master = 0;
out_unlock: out_unlock:
...@@ -234,10 +234,10 @@ int drm_master_open(struct drm_file *file_priv) ...@@ -234,10 +234,10 @@ int drm_master_open(struct drm_file *file_priv)
/* if there is no current master make this fd it, but do not create /* if there is no current master make this fd it, but do not create
* any master object for render clients */ * any master object for render clients */
mutex_lock(&dev->master_mutex); mutex_lock(&dev->master_mutex);
if (!file_priv->minor->master) if (!dev->master)
ret = drm_new_set_master(dev, file_priv); ret = drm_new_set_master(dev, file_priv);
else else
file_priv->master = drm_master_get(file_priv->minor->master); file_priv->master = drm_master_get(dev->master);
mutex_unlock(&dev->master_mutex); mutex_unlock(&dev->master_mutex);
return ret; return ret;
...@@ -271,11 +271,11 @@ void drm_master_release(struct drm_file *file_priv) ...@@ -271,11 +271,11 @@ void drm_master_release(struct drm_file *file_priv)
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
} }
if (file_priv->minor->master == file_priv->master) { if (dev->master == file_priv->master) {
/* drop the reference held my the minor */ /* drop the reference held my the minor */
if (dev->driver->master_drop) if (dev->driver->master_drop)
dev->driver->master_drop(dev, file_priv, true); dev->driver->master_drop(dev, file_priv, true);
drm_master_put(&file_priv->minor->master); drm_master_put(&dev->master);
} }
out: out:
/* drop the master reference held by the file priv */ /* drop the master reference held by the file priv */
......
...@@ -51,7 +51,7 @@ static struct drm_map_list *drm_find_matching_map(struct drm_device *dev, ...@@ -51,7 +51,7 @@ static struct drm_map_list *drm_find_matching_map(struct drm_device *dev,
*/ */
if (!entry->map || if (!entry->map ||
map->type != entry->map->type || map->type != entry->map->type ||
entry->master != dev->primary->master) entry->master != dev->master)
continue; continue;
switch (map->type) { switch (map->type) {
case _DRM_SHM: case _DRM_SHM:
...@@ -245,12 +245,12 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset, ...@@ -245,12 +245,12 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
map->offset = (unsigned long)map->handle; map->offset = (unsigned long)map->handle;
if (map->flags & _DRM_CONTAINS_LOCK) { if (map->flags & _DRM_CONTAINS_LOCK) {
/* Prevent a 2nd X Server from creating a 2nd lock */ /* Prevent a 2nd X Server from creating a 2nd lock */
if (dev->primary->master->lock.hw_lock != NULL) { if (dev->master->lock.hw_lock != NULL) {
vfree(map->handle); vfree(map->handle);
kfree(map); kfree(map);
return -EBUSY; return -EBUSY;
} }
dev->sigdata.lock = dev->primary->master->lock.hw_lock = map->handle; /* Pointer to lock */ dev->sigdata.lock = dev->master->lock.hw_lock = map->handle; /* Pointer to lock */
} }
break; break;
case _DRM_AGP: { case _DRM_AGP: {
...@@ -356,7 +356,7 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset, ...@@ -356,7 +356,7 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
if (!(map->flags & _DRM_DRIVER)) if (!(map->flags & _DRM_DRIVER))
list->master = dev->primary->master; list->master = dev->master;
*maplist = list; *maplist = list;
return 0; return 0;
} }
......
...@@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) ...@@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
/* Sometimes user space wants everything disabled, so don't steal the /* Sometimes user space wants everything disabled, so don't steal the
* display if there's a master. */ * display if there's a master. */
if (dev->primary->master) if (READ_ONCE(dev->master))
return false; return false;
drm_for_each_crtc(crtc, dev) { drm_for_each_crtc(crtc, dev) {
......
...@@ -50,7 +50,12 @@ int drm_name_info(struct seq_file *m, void *data) ...@@ -50,7 +50,12 @@ int drm_name_info(struct seq_file *m, void *data)
struct drm_info_node *node = (struct drm_info_node *) m->private; struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_minor *minor = node->minor; struct drm_minor *minor = node->minor;
struct drm_device *dev = minor->dev; struct drm_device *dev = minor->dev;
struct drm_master *master = minor->master; struct drm_master *master;
mutex_lock(&dev->master_mutex);
master = dev->master;
if (!master)
goto out_unlock;
seq_printf(m, "%s", dev->driver->name); seq_printf(m, "%s", dev->driver->name);
if (dev->dev) if (dev->dev)
...@@ -60,6 +65,8 @@ int drm_name_info(struct seq_file *m, void *data) ...@@ -60,6 +65,8 @@ int drm_name_info(struct seq_file *m, void *data)
if (dev->unique) if (dev->unique)
seq_printf(m, " unique=%s", dev->unique); seq_printf(m, " unique=%s", dev->unique);
seq_printf(m, "\n"); seq_printf(m, "\n");
out_unlock:
mutex_unlock(&dev->master_mutex);
return 0; return 0;
} }
......
...@@ -334,7 +334,7 @@ void drm_legacy_lock_release(struct drm_device *dev, struct file *filp) ...@@ -334,7 +334,7 @@ void drm_legacy_lock_release(struct drm_device *dev, struct file *filp)
struct drm_file *file_priv = filp->private_data; struct drm_file *file_priv = filp->private_data;
/* if the master has gone away we can't do anything with the lock */ /* if the master has gone away we can't do anything with the lock */
if (!file_priv->minor->master) if (!dev->master)
return; return;
if (drm_legacy_i_have_hw_lock(dev, file_priv)) { if (drm_legacy_i_have_hw_lock(dev, file_priv)) {
......
...@@ -316,7 +316,7 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, ...@@ -316,7 +316,7 @@ void sis_reclaim_buffers_locked(struct drm_device *dev,
struct sis_file_private *file_priv = file->driver_priv; struct sis_file_private *file_priv = file->driver_priv;
struct sis_memblock *entry, *next; struct sis_memblock *entry, *next;
if (!(file->minor->master && file->master->lock.hw_lock)) if (!(dev->master && file->master->lock.hw_lock))
return; return;
drm_legacy_idlelock_take(&file->master->lock); drm_legacy_idlelock_take(&file->master->lock);
......
...@@ -208,7 +208,7 @@ void via_reclaim_buffers_locked(struct drm_device *dev, ...@@ -208,7 +208,7 @@ void via_reclaim_buffers_locked(struct drm_device *dev,
struct via_file_private *file_priv = file->driver_priv; struct via_file_private *file_priv = file->driver_priv;
struct via_memblock *entry, *next; struct via_memblock *entry, *next;
if (!(file->minor->master && file->master->lock.hw_lock)) if (!(dev->master && file->master->lock.hw_lock))
return; return;
drm_legacy_idlelock_take(&file->master->lock); drm_legacy_idlelock_take(&file->master->lock);
......
...@@ -336,7 +336,7 @@ struct drm_file { ...@@ -336,7 +336,7 @@ struct drm_file {
void *driver_priv; void *driver_priv;
struct drm_master *master; /* master this node is currently associated with struct drm_master *master; /* master this node is currently associated with
N.B. not always minor->master */ N.B. not always dev->master */
/** /**
* fbs - List of framebuffers associated with this file. * fbs - List of framebuffers associated with this file.
* *
...@@ -708,9 +708,6 @@ struct drm_minor { ...@@ -708,9 +708,6 @@ struct drm_minor {
struct list_head debugfs_list; struct list_head debugfs_list;
struct mutex debugfs_lock; /* Protects debugfs_list. */ struct mutex debugfs_lock; /* Protects debugfs_list. */
/* currently active master for this node. Protected by master_mutex */
struct drm_master *master;
}; };
...@@ -759,6 +756,10 @@ struct drm_device { ...@@ -759,6 +756,10 @@ struct drm_device {
struct drm_minor *control; /**< Control node */ struct drm_minor *control; /**< Control node */
struct drm_minor *primary; /**< Primary node */ struct drm_minor *primary; /**< Primary node */
struct drm_minor *render; /**< Render node */ struct drm_minor *render; /**< Render node */
/* currently active master for this device. Protected by master_mutex */
struct drm_master *master;
atomic_t unplugged; /**< Flag whether dev is dead */ atomic_t unplugged; /**< Flag whether dev is dead */
struct inode *anon_inode; /**< inode for private address-space */ struct inode *anon_inode; /**< inode for private address-space */
char *unique; /**< unique name of the device */ char *unique; /**< unique name of the device */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment