* [PATCH 1/3] drm: Don't reserve minors for control nodes
2022-08-17 23:05 [PATCH 0/3] drm: Use full allocated minor range for DRM Michał Winiarski
@ 2022-08-17 23:05 ` Michał Winiarski
2022-08-17 23:05 ` [PATCH 2/3] drm: Expand max DRM device number to full MINORBITS Michał Winiarski
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Michał Winiarski @ 2022-08-17 23:05 UTC (permalink / raw)
To: dri-devel; +Cc: Michał Winiarski, David Airlie, Thomas Zimmermann
Control nodes are no longer with us.
While we still need to preserve render nodes numbering, there's no need
to reserve the range formerly used for control. Let's repurpose it to be
used by primary and remove control remains from the code entirely.
References: commit 0d49f303e8a7 ("drm: remove all control node code")
References: commit c9ac371d4b59 ("drm: Fix render node numbering regression from control node removal.")
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/drm_drv.c | 4 ++--
include/drm/drm_file.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8214a0b1ab7f..d81783f43452 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -126,8 +126,8 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
spin_lock_irqsave(&drm_minor_lock, flags);
r = idr_alloc(&drm_minors_idr,
NULL,
- 64 * type,
- 64 * (type + 1),
+ 128 * type,
+ 128 * (type + 1),
GFP_NOWAIT);
spin_unlock_irqrestore(&drm_minor_lock, flags);
idr_preload_end();
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index d780fd151789..a3be533e99e0 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -54,7 +54,6 @@ struct file;
*/
enum drm_minor_type {
DRM_MINOR_PRIMARY,
- DRM_MINOR_CONTROL,
DRM_MINOR_RENDER,
};
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drm: Expand max DRM device number to full MINORBITS
2022-08-17 23:05 [PATCH 0/3] drm: Use full allocated minor range for DRM Michał Winiarski
2022-08-17 23:05 ` [PATCH 1/3] drm: Don't reserve minors for control nodes Michał Winiarski
@ 2022-08-17 23:05 ` Michał Winiarski
2022-08-17 23:06 ` [PATCH 3/3] drm: Use mutex for minors Michał Winiarski
[not found] ` <wWV4E0y1qoweGL7vMn0IgY903SnA0rN5qqy_58NIvSX9j0XkFb1JKTmea4HhNCPsxCUe88Ni7HUa2Bu1UMIxtZZh1nrIR3aVaoooDiAaUvU=@emersion.fr>
3 siblings, 0 replies; 6+ messages in thread
From: Michał Winiarski @ 2022-08-17 23:05 UTC (permalink / raw)
To: dri-devel; +Cc: Michał Winiarski, David Airlie, Thomas Zimmermann
Even after getting rid of control nodes, we've only bumped max DRM
device number from 64 to 128.
That's not good enough for modern world where we have multi-GPU servers,
SR-IOV virtual functions and virtual devices used for testing.
Let's utilize full minor range for DRM devices.
To avoid regressing the existing userspace, we're still maintaining the
0-127 for primary, 128-255 for render. This numbering scheme is
continued for minors > 256 (256-383 for primary, 384-511 for render, and
so on).
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/drm_drv.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d81783f43452..0dab1ef8a98d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -113,7 +113,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
unsigned long flags;
- int r;
+ int r, start, end;
minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
if (!minor)
@@ -122,15 +122,19 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
minor->type = type;
minor->dev = dev;
- idr_preload(GFP_KERNEL);
- spin_lock_irqsave(&drm_minor_lock, flags);
- r = idr_alloc(&drm_minors_idr,
- NULL,
- 128 * type,
- 128 * (type + 1),
- GFP_NOWAIT);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
- idr_preload_end();
+ start = 128 * type;
+ end = 128 * (type + 1);
+
+ do {
+ idr_preload(GFP_KERNEL);
+ spin_lock_irqsave(&drm_minor_lock, flags);
+ r = idr_alloc(&drm_minors_idr, NULL, start, end, GFP_NOWAIT);
+ spin_unlock_irqrestore(&drm_minor_lock, flags);
+ idr_preload_end();
+
+ start += 256;
+ end += 256;
+ } while ((r == -ENOSPC) && end <= (1 << MINORBITS));
if (r < 0)
return r;
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm: Use mutex for minors
2022-08-17 23:05 [PATCH 0/3] drm: Use full allocated minor range for DRM Michał Winiarski
2022-08-17 23:05 ` [PATCH 1/3] drm: Don't reserve minors for control nodes Michał Winiarski
2022-08-17 23:05 ` [PATCH 2/3] drm: Expand max DRM device number to full MINORBITS Michał Winiarski
@ 2022-08-17 23:06 ` Michał Winiarski
[not found] ` <wWV4E0y1qoweGL7vMn0IgY903SnA0rN5qqy_58NIvSX9j0XkFb1JKTmea4HhNCPsxCUe88Ni7HUa2Bu1UMIxtZZh1nrIR3aVaoooDiAaUvU=@emersion.fr>
3 siblings, 0 replies; 6+ messages in thread
From: Michał Winiarski @ 2022-08-17 23:06 UTC (permalink / raw)
To: dri-devel; +Cc: Michał Winiarski, David Airlie, Thomas Zimmermann
Operating on drm minor is not done in IRQ context, which means that we
could safely downgrade to regular non-irq spinlock.
But we can also go further and drop the idr_preload tricks by just using
a mutex.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/drm_drv.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 0dab1ef8a98d..b31497e28e6a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -53,7 +53,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 DEFINE_MUTEX(drm_minor_lock);
static struct idr drm_minors_idr;
/*
@@ -98,21 +98,19 @@ 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);
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
idr_remove(&drm_minors_idr, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
}
static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
int r, start, end;
minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
@@ -126,11 +124,9 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
end = 128 * (type + 1);
do {
- idr_preload(GFP_KERNEL);
- spin_lock_irqsave(&drm_minor_lock, flags);
- r = idr_alloc(&drm_minors_idr, NULL, start, end, GFP_NOWAIT);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
- idr_preload_end();
+ mutex_lock(&drm_minor_lock);
+ r = idr_alloc(&drm_minors_idr, NULL, start, end, GFP_KERNEL);
+ mutex_unlock(&drm_minor_lock);
start += 256;
end += 256;
@@ -156,7 +152,6 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
static int drm_minor_register(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
int ret;
DRM_DEBUG("\n");
@@ -176,9 +171,9 @@ 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);
+ mutex_lock(&drm_minor_lock);
idr_replace(&drm_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
DRM_DEBUG("new minor registered %d\n", minor->index);
return 0;
@@ -191,16 +186,15 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
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 || !device_is_registered(minor->kdev))
return;
/* replace @minor with NULL so lookups will fail from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
+ mutex_lock(&drm_minor_lock);
idr_replace(&drm_minors_idr, NULL, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
@@ -219,13 +213,12 @@ 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);
+ mutex_lock(&drm_minor_lock);
minor = idr_find(&drm_minors_idr, minor_id);
if (minor)
drm_dev_get(minor->dev);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ mutex_unlock(&drm_minor_lock);
if (!minor) {
return ERR_PTR(-ENODEV);
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread