Skip to content

Commit

Permalink
drm: make minors independent of global lock
Browse files Browse the repository at this point in the history
We used to protect minor-lookup and setup by the global drm lock. To
continue our attempts of dropping drm_global_mutex, this patch makes the
minor management independent of it. Furthermore, we make it all atomic and
switch to spin-locks instead of a mutex.

Now that minor-lookup is independent, we also move the
"drm_is_unplugged()" test into the minor-lookup path. There is no reason
to ever return a minor for unplugged objects, so keep that logic internal.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
  • Loading branch information
David Herrmann committed Mar 16, 2014
1 parent 7d86cf1 commit 0d63988
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 16 deletions.
11 changes: 1 addition & 10 deletions drivers/gpu/drm/drm_fops.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include <linux/slab.h>
#include <linux/module.h>

/* from BKL pushdown: note that nothing else serializes idr_find() */
/* from BKL pushdown */
DEFINE_MUTEX(drm_global_mutex);
EXPORT_SYMBOL(drm_global_mutex);

Expand Down Expand Up @@ -91,11 +91,6 @@ int drm_open(struct inode *inode, struct file *filp)
return PTR_ERR(minor);

dev = minor->dev;
if (drm_device_is_unplugged(dev)) {
retcode = -ENODEV;
goto err_release;
}

if (!dev->open_count++)
need_setup = 1;
mutex_lock(&dev->struct_mutex);
Expand Down Expand Up @@ -127,7 +122,6 @@ int drm_open(struct inode *inode, struct file *filp)
dev->dev_mapping = old_mapping;
mutex_unlock(&dev->struct_mutex);
dev->open_count--;
err_release:
drm_minor_release(minor);
return retcode;
}
Expand Down Expand Up @@ -157,9 +151,6 @@ int drm_stub_open(struct inode *inode, struct file *filp)
goto out_unlock;

dev = minor->dev;
if (drm_device_is_unplugged(dev))
goto out_release;

new_fops = fops_get(dev->driver->fops);
if (!new_fops)
goto out_release;
Expand Down
51 changes: 45 additions & 6 deletions drivers/gpu/drm/drm_stub.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600);

static DEFINE_SPINLOCK(drm_minor_lock);
struct idr drm_minors_idr;

struct class *drm_class;
Expand Down Expand Up @@ -240,6 +241,19 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
return 0;
}

/*
* DRM Minors
* A DRM device can provide several char-dev interfaces on the DRM-Major. Each
* of them is represented by a drm_minor object. Depending on the capabilities
* of the device-driver, different interfaces are registered.
*
* Minors can be accessed via dev->$minor_name. This pointer is either
* NULL or a valid drm_minor pointer and stays valid as long as the device is
* valid. This means, DRM minors have the same life-time as the underlying
* device. However, this doesn't mean that the minor is active. Minors are
* registered and unregistered dynamically according to device-state.
*/

static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
unsigned int type)
{
Expand Down Expand Up @@ -285,6 +299,7 @@ static void drm_minor_free(struct drm_device *dev, unsigned int type)
static int drm_minor_register(struct drm_device *dev, unsigned int type)
{
struct drm_minor *new_minor;
unsigned long flags;
int ret;
int minor_id;

Expand All @@ -294,19 +309,21 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
if (!new_minor)
return 0;

idr_preload(GFP_KERNEL);
spin_lock_irqsave(&drm_minor_lock, flags);
minor_id = idr_alloc(&drm_minors_idr,
NULL,
64 * type,
64 * (type + 1),
GFP_KERNEL);
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
idr_preload_end();

if (minor_id < 0)
return minor_id;

new_minor->index = minor_id;

idr_replace(&drm_minors_idr, new_minor, minor_id);

ret = drm_debugfs_init(new_minor, minor_id, drm_debugfs_root);
if (ret) {
DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
Expand All @@ -319,27 +336,40 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
goto err_debugfs;
}

/* replace NULL with @minor so lookups will succeed from now on */
spin_lock_irqsave(&drm_minor_lock, flags);
idr_replace(&drm_minors_idr, new_minor, new_minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);

DRM_DEBUG("new minor assigned %d\n", minor_id);
return 0;

err_debugfs:
drm_debugfs_cleanup(new_minor);
err_id:
spin_lock_irqsave(&drm_minor_lock, flags);
idr_remove(&drm_minors_idr, minor_id);
spin_unlock_irqrestore(&drm_minor_lock, flags);
new_minor->index = 0;
return ret;
}

static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
unsigned long flags;

minor = *drm_minor_get_slot(dev, type);
if (!minor || !minor->kdev)
return;

spin_lock_irqsave(&drm_minor_lock, flags);
idr_remove(&drm_minors_idr, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
minor->index = 0;

drm_debugfs_cleanup(minor);
drm_sysfs_device_remove(minor);
idr_remove(&drm_minors_idr, minor->index);
}

/**
Expand All @@ -361,12 +391,21 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
struct drm_minor *drm_minor_acquire(unsigned int minor_id)
{
struct drm_minor *minor;
unsigned long flags;

spin_lock_irqsave(&drm_minor_lock, flags);
minor = idr_find(&drm_minors_idr, minor_id);
if (!minor)
if (minor)
drm_dev_ref(minor->dev);
spin_unlock_irqrestore(&drm_minor_lock, flags);

if (!minor) {
return ERR_PTR(-ENODEV);
} else if (drm_device_is_unplugged(minor->dev)) {
drm_dev_unref(minor->dev);
return ERR_PTR(-ENODEV);
}

drm_dev_ref(minor->dev);
return minor;
}

Expand Down

0 comments on commit 0d63988

Please sign in to comment.