Skip to content

Commit

Permalink
drm: Use XArray instead of IDR for minors
Browse files Browse the repository at this point in the history
IDR is deprecated, and since XArray manages its own state with internal
locking, it simplifies the locking on DRM side.
Additionally, don't use the IRQ-safe variant, since operating on drm
minor is not done in IRQ context.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Acked-by: James Zhu <James.Zhu@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240823163048.2676257-2-michal.winiarski@intel.com
Signed-off-by: Christian König <christian.koenig@amd.com>
  • Loading branch information
Michał Winiarski authored and Christian König committed Aug 26, 2024
1 parent 22bc22c commit 5fbca8b
Showing 1 changed file with 25 additions and 38 deletions.
63 changes: 25 additions & 38 deletions drivers/gpu/drm/drm_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <linux/pseudo_fs.h>
#include <linux/slab.h>
#include <linux/srcu.h>
#include <linux/xarray.h>

#include <drm/drm_accel.h>
#include <drm/drm_cache.h>
Expand All @@ -54,8 +55,7 @@ MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl");
MODULE_DESCRIPTION("DRM shared core routines");
MODULE_LICENSE("GPL and additional rights");

static DEFINE_SPINLOCK(drm_minor_lock);
static struct idr drm_minors_idr;
static DEFINE_XARRAY_ALLOC(drm_minors_xa);

/*
* If the drm core fails to init for whatever reason,
Expand Down Expand Up @@ -101,26 +101,23 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
static void drm_minor_alloc_release(struct drm_device *dev, void *data)
{
struct drm_minor *minor = data;
unsigned long flags;

WARN_ON(dev != minor->dev);

put_device(minor->kdev);

if (minor->type == DRM_MINOR_ACCEL) {
if (minor->type == DRM_MINOR_ACCEL)
accel_minor_remove(minor->index);
} else {
spin_lock_irqsave(&drm_minor_lock, flags);
idr_remove(&drm_minors_idr, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
}
else
xa_erase(&drm_minors_xa, minor->index);
}

#define DRM_MINOR_LIMIT(t) ({ typeof(t) _t = (t); XA_LIMIT(64 * _t, 64 * _t + 63); })

static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type)
{
struct drm_minor *minor;
unsigned long flags;
int r;
int index, r;

minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
if (!minor)
Expand All @@ -129,24 +126,17 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type)
minor->type = type;
minor->dev = dev;

idr_preload(GFP_KERNEL);
if (type == DRM_MINOR_ACCEL) {
r = accel_minor_alloc();
index = r;
} else {
spin_lock_irqsave(&drm_minor_lock, flags);
r = idr_alloc(&drm_minors_idr,
NULL,
64 * type,
64 * (type + 1),
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
r = xa_alloc(&drm_minors_xa, &index, NULL, DRM_MINOR_LIMIT(type), GFP_KERNEL);
}
idr_preload_end();

if (r < 0)
return r;

minor->index = r;
minor->index = index;

r = drmm_add_action_or_reset(dev, drm_minor_alloc_release, minor);
if (r)
Expand All @@ -163,7 +153,7 @@ static int drm_minor_alloc(struct drm_device *dev, enum drm_minor_type type)
static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
{
struct drm_minor *minor;
unsigned long flags;
void *entry;
int ret;

DRM_DEBUG("\n");
Expand All @@ -189,9 +179,12 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
if (minor->type == DRM_MINOR_ACCEL) {
accel_minor_replace(minor, minor->index);
} else {
spin_lock_irqsave(&drm_minor_lock, flags);
idr_replace(&drm_minors_idr, minor, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
entry = xa_store(&drm_minors_xa, minor->index, minor, GFP_KERNEL);
if (xa_is_err(entry)) {
ret = xa_err(entry);
goto err_debugfs;
}
WARN_ON(entry);
}

DRM_DEBUG("new minor registered %d\n", minor->index);
Expand All @@ -205,20 +198,16 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type type)
{
struct drm_minor *minor;
unsigned long flags;

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

/* replace @minor with NULL so lookups will fail from now on */
if (minor->type == DRM_MINOR_ACCEL) {
if (minor->type == DRM_MINOR_ACCEL)
accel_minor_replace(NULL, minor->index);
} else {
spin_lock_irqsave(&drm_minor_lock, flags);
idr_replace(&drm_minors_idr, NULL, minor->index);
spin_unlock_irqrestore(&drm_minor_lock, flags);
}
else
xa_store(&drm_minors_xa, minor->index, NULL, GFP_KERNEL);

device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
Expand All @@ -237,13 +226,12 @@ static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type typ
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);
xa_lock(&drm_minors_xa);
minor = xa_load(&drm_minors_xa, minor_id);
if (minor)
drm_dev_get(minor->dev);
spin_unlock_irqrestore(&drm_minor_lock, flags);
xa_unlock(&drm_minors_xa);

if (!minor) {
return ERR_PTR(-ENODEV);
Expand Down Expand Up @@ -1072,7 +1060,7 @@ static void drm_core_exit(void)
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
drm_sysfs_destroy();
idr_destroy(&drm_minors_idr);
WARN_ON(!xa_empty(&drm_minors_xa));
drm_connector_ida_destroy();
}

Expand All @@ -1081,7 +1069,6 @@ static int __init drm_core_init(void)
int ret;

drm_connector_ida_init();
idr_init(&drm_minors_idr);
drm_memcpy_init_early();

ret = drm_sysfs_init();
Expand Down

0 comments on commit 5fbca8b

Please sign in to comment.