- Rename it to 'objects', as requested in todo.rst
- Also convert leases IDR to XArray as the two are occasionally used by
the same code (see drm_mode_get_lease_ioctl())
- Refactor drm_mode_create_lease_ioctl() to create the new drm_master
early to avoid creating an XArray on the stack and reparenting it
afterwards.
Signed-off-by: Matthew Wilcox <wi...@infradead.org>
---
Documentation/gpu/todo.rst | 3 -
drivers/gpu/drm/drm_auth.c | 4 +-
drivers/gpu/drm/drm_lease.c | 136 ++++++++++++++----------------
drivers/gpu/drm/drm_mode_config.c | 3 +-
drivers/gpu/drm/drm_mode_object.c | 47 +++++------
include/drm/drm_auth.h | 2 +-
include/drm/drm_mode_config.h | 6 +-
7 files changed, 90 insertions(+), 111 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 14191b64446d..41da7b06195c 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -354,9 +354,6 @@ KMS cleanups
Some of these date from the very introduction of KMS in 2008 ...
-- drm_mode_config.crtc_idr is misnamed, since it contains all KMS object.
Should
- be renamed to drm_mode_config.object_idr.
-
- drm_display_mode doesn't need to be derived from drm_mode_object. That's
leftovers from older (never merged into upstream) KMS designs where modes
where set using their ID, including support to add/remove modes.
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1813507f9b9c..c6967f0b095d 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -110,7 +110,7 @@ struct drm_master *drm_master_create(struct drm_device *dev)
/* initialize the tree of output resource lessees */
master->lessor = NULL;
master->lessee_id = 0;
- idr_init(&master->leases);
+ xa_init(&master->leases);
xa_init_flags(&master->lessees, XA_FLAGS_ALLOC1);
return master;
@@ -345,7 +345,7 @@ static void drm_master_destroy(struct kref *kref)
drm_legacy_master_rmmaps(dev, master);
- idr_destroy(&master->leases);
+ xa_destroy(&master->leases);
kfree(master->unique);
kfree(master);
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 47830f9ec616..1e88f406c738 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -20,8 +20,6 @@
#include <drm/drm_auth.h>
#include <drm/drm_crtc_helper.h>
-static uint64_t drm_lease_idr_object;
-
/**
* drm_lease_owner - return ancestor owner drm_master
* @master: drm_master somewhere within tree of lessees and lessors
@@ -69,7 +67,7 @@ static int _drm_lease_held_master(struct drm_master *master,
int id)
{
lockdep_assert_held(&master->dev->mode_config.idr_mutex);
if (master->lessor)
- return idr_find(&master->leases, id) != NULL;
+ return xa_load(&master->leases, id) != NULL;
return true;
}
@@ -183,7 +181,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv,
uint32_t crtcs_in)
/*
* drm_lease_create - create a new drm_master with leased objects (idr_mutex
not held)
* @lessor: lease holder (or owner) of objects
- * @leases: objects to lease to the new drm_master
+ * @lessee: leaser of objects
*
* Uses drm_master_create to allocate a new drm_master, then checks to
* make sure all of the desired objects can be leased, atomically
@@ -195,35 +193,30 @@ uint32_t drm_lease_filter_crtcs(struct drm_file
*file_priv, uint32_t crtcs_in)
* ERR_PTR(-EEXIST) same object specified more than once in the
provided list
* ERR_PTR(-ENOMEM) allocation failed
*/
-static struct drm_master *drm_lease_create(struct drm_master *lessor, struct
idr *leases)
+static struct drm_master *drm_lease_create(struct drm_master *lessor,
+ struct drm_master *lessee)
{
struct drm_device *dev = lessor->dev;
int error;
- struct drm_master *lessee;
- int object;
void *entry;
+ unsigned long index;
DRM_DEBUG_LEASE("lessor %d\n", lessor->lessee_id);
- lessee = drm_master_create(lessor->dev);
- if (!lessee) {
- DRM_DEBUG_LEASE("drm_master_create failed\n");
- return ERR_PTR(-ENOMEM);
- }
-
mutex_lock(&dev->mode_config.idr_mutex);
- idr_for_each_entry(leases, entry, object) {
+ xa_for_each(&lessee->leases, index, entry) {
error = 0;
- if (!idr_find(&dev->mode_config.crtc_idr, object))
+ if (!xa_load(&dev->mode_config.objects, index))
error = -ENOENT;
- else if (!_drm_lease_held_master(lessor, object))
+ else if (!_drm_lease_held_master(lessor, index))
error = -EACCES;
- else if (_drm_has_leased(lessor, object))
+ else if (_drm_has_leased(lessor, index))
error = -EBUSY;
if (error != 0) {
- DRM_DEBUG_LEASE("object %d failed %d\n", object, error);
+ DRM_DEBUG_LEASE("object %d failed %d\n", (u32)index,
+ error);
goto out_lessee;
}
}
@@ -236,8 +229,6 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
lessee->lessor = drm_master_get(lessor);
- /* Move the leases over */
- lessee->leases = *leases;
DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id,
lessee, lessor->lessee_id, lessor);
mutex_unlock(&dev->mode_config.idr_mutex);
@@ -246,8 +237,6 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
out_lessee:
mutex_unlock(&dev->mode_config.idr_mutex);
- drm_master_put(&lessee);
-
return ERR_PTR(error);
}
@@ -292,8 +281,6 @@ void drm_lease_destroy(struct drm_master *master)
*/
static void _drm_lease_revoke(struct drm_master *top)
{
- int object;
- void *entry;
struct drm_master *master = top;
lockdep_assert_held(&top->dev->mode_config.idr_mutex);
@@ -309,8 +296,7 @@ static void _drm_lease_revoke(struct drm_master *top)
DRM_DEBUG_LEASE("revoke leases for %p %d\n", master,
master->lessee_id);
/* Evacuate the lease */
- idr_for_each_entry(&master->leases, entry, object)
- idr_remove(&master->leases, object);
+ xa_destroy(&master->leases);
/* Depth-first tree walk */
tmp = xa_find(&master->lessees, &index, ULONG_MAX, XA_PRESENT);
@@ -378,11 +364,9 @@ static int validate_lease(struct drm_device *dev,
return 0;
}
-static int fill_object_idr(struct drm_device *dev,
- struct drm_file *lessor_priv,
- struct idr *leases,
- int object_count,
- u32 *object_ids)
+static int fill_object_array(struct drm_device *dev,
+ struct drm_file *lessor_priv, struct xarray *leases,
+ int object_count, u32 *object_ids)
{
struct drm_mode_object **objects;
u32 o;
@@ -431,14 +415,14 @@ static int fill_object_idr(struct drm_device *dev,
DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
/*
- * We're using an IDR to hold the set of leased
+ * We're using an array to hold the set of leased
* objects, but we don't need to point at the object's
- * data structure from the lease as the main crtc_idr
+ * data structure from the lease as the main object array
* will be used to actually find that. Instead, all we
* really want is a 'leased/not-leased' result, for
* which any non-NULL pointer will work fine.
*/
- ret = idr_alloc(leases, &drm_lease_idr_object , object_id,
object_id + 1, GFP_KERNEL);
+ ret = xa_insert(leases, object_id, xa_mk_value(0), GFP_KERNEL);
if (ret < 0) {
DRM_DEBUG_LEASE("Object %d cannot be inserted into
leases (%d)\n",
object_id, ret);
@@ -446,19 +430,21 @@ static int fill_object_idr(struct drm_device *dev,
}
if (obj->type == DRM_MODE_OBJECT_CRTC && !universal_planes) {
struct drm_crtc *crtc = obj_to_crtc(obj);
- ret = idr_alloc(leases, &drm_lease_idr_object,
crtc->primary->base.id, crtc->primary->base.id + 1, GFP_KERNEL);
+ ret = xa_insert(leases, crtc->primary->base.id,
+ xa_mk_value(0), GFP_KERNEL);
if (ret < 0) {
DRM_DEBUG_LEASE("Object primary plane %d cannot
be inserted into leases (%d)\n",
object_id, ret);
goto out_free_objects;
}
- if (crtc->cursor) {
- ret = idr_alloc(leases, &drm_lease_idr_object,
crtc->cursor->base.id, crtc->cursor->base.id + 1, GFP_KERNEL);
- if (ret < 0) {
- DRM_DEBUG_LEASE("Object cursor plane %d
cannot be inserted into leases (%d)\n",
- object_id, ret);
- goto out_free_objects;
- }
+ if (!crtc->cursor)
+ continue;
+ ret = xa_insert(leases, crtc->cursor->base.id,
+ xa_mk_value(0), GFP_KERNEL);
+ if (ret < 0) {
+ DRM_DEBUG_LEASE("Object cursor plane %d cannot
be inserted into leases (%d)\n",
+ object_id, ret);
+ goto out_free_objects;
}
}
}
@@ -490,9 +476,8 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
struct drm_mode_create_lease *cl = data;
size_t object_count;
int ret = 0;
- struct idr leases;
struct drm_master *lessor = lessor_priv->master;
- struct drm_master *lessee = NULL;
+ struct drm_master *lessee;
struct file *lessee_file = NULL;
struct file *lessor_file = lessor_priv->filp;
struct drm_file *lessee_priv;
@@ -520,37 +505,42 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
return -EINVAL;
}
- object_count = cl->object_count;
+ lessee = drm_master_create(lessor->dev);
+ if (!lessee) {
+ DRM_DEBUG_LEASE("drm_master_create failed\n");
+ return -ENOMEM;
+ }
- object_ids = memdup_user(u64_to_user_ptr(cl->object_ids), object_count
* sizeof(__u32));
- if (IS_ERR(object_ids))
- return PTR_ERR(object_ids);
+ object_count = cl->object_count;
- idr_init(&leases);
+ object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
+ array_size(object_count, sizeof(__u32)));
+ if (IS_ERR(object_ids)) {
+ ret = PTR_ERR(object_ids);
+ goto out_lessee;
+ }
- /* fill and validate the object idr */
- ret = fill_object_idr(dev, lessor_priv, &leases,
+ /* fill and validate the object array */
+ ret = fill_object_array(dev, lessor_priv, &lessee->leases,
object_count, object_ids);
kfree(object_ids);
if (ret) {
DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
- idr_destroy(&leases);
- return ret;
+ goto out_lessee;
}
/* Allocate a file descriptor for the lease */
- fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
- if (fd < 0) {
- idr_destroy(&leases);
- return fd;
- }
+ ret = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
+ if (ret < 0)
+ goto out_lessee;
+ fd = ret;
DRM_DEBUG_LEASE("Creating lease\n");
- lessee = drm_lease_create(lessor, &leases);
+ lessee = drm_lease_create(lessor, lessee);
if (IS_ERR(lessee)) {
ret = PTR_ERR(lessee);
- goto out_leases;
+ goto out_fd;
}
/* Clone the lessor file to create a new file for us */
@@ -558,7 +548,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
lessee_file = file_clone_open(lessor_file);
if (IS_ERR(lessee_file)) {
ret = PTR_ERR(lessee_file);
- goto out_lessee;
+ goto out_fd;
}
lessee_priv = lessee_file->private_data;
@@ -579,13 +569,11 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
return 0;
+out_fd:
+ put_unused_fd(fd);
out_lessee:
drm_master_put(&lessee);
-out_leases:
- put_unused_fd(fd);
- idr_destroy(&leases);
-
DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
return ret;
}
@@ -627,7 +615,7 @@ int drm_mode_list_lessees_ioctl(struct drm_device *dev,
count = 0;
xa_for_each(&lessor->lessees, index, lessee) {
/* Only list un-revoked leases */
- if (!idr_is_empty(&lessee->leases)) {
+ if (!xa_empty(&lessee->leases)) {
if (count_lessees > count) {
DRM_DEBUG_LEASE("Add lessee %d\n",
lessee->lessee_id);
ret = put_user(lessee->lessee_id, lessee_ids +
count);
@@ -663,10 +651,10 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev,
__u32 __user *object_ids = (__u32 __user *) (uintptr_t)
(arg->objects_ptr);
__u32 count_objects = arg->count_objects;
struct drm_master *lessee = lessee_priv->master;
- struct idr *object_idr;
+ struct xarray *objects;
int count;
void *entry;
- int object;
+ unsigned long index;
int ret = 0;
if (arg->pad)
@@ -682,16 +670,16 @@ int drm_mode_get_lease_ioctl(struct drm_device *dev,
if (lessee->lessor == NULL)
/* owner can use all objects */
- object_idr = &lessee->dev->mode_config.crtc_idr;
+ objects = &lessee->dev->mode_config.objects;
else
- /* lessee can only use allowed object */
- object_idr = &lessee->leases;
+ /* lessee can only use allowed objects */
+ objects = &lessee->leases;
count = 0;
- idr_for_each_entry(object_idr, entry, object) {
+ xa_for_each(objects, index, entry) {
if (count_objects > count) {
- DRM_DEBUG_LEASE("adding object %d\n", object);
- ret = put_user(object, object_ids + count);
+ DRM_DEBUG_LEASE("adding object %d\n", (u32)index);
+ ret = put_user((u32)index, object_ids + count);
if (ret)
break;
}
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index 609b30d7dcb1..10c616e0f591 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -393,7 +393,7 @@ void drm_mode_config_init(struct drm_device *dev)
INIT_LIST_HEAD(&dev->mode_config.property_list);
INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
INIT_LIST_HEAD(&dev->mode_config.plane_list);
- idr_init(&dev->mode_config.crtc_idr);
+ xa_init_flags(&dev->mode_config.objects, XA_FLAGS_ALLOC1);
xa_init_flags(&dev->mode_config.tiles, XA_FLAGS_ALLOC1);
ida_init(&dev->mode_config.connector_ida);
spin_lock_init(&dev->mode_config.connector_list_lock);
@@ -495,7 +495,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
}
ida_destroy(&dev->mode_config.connector_ida);
- idr_destroy(&dev->mode_config.crtc_idr);
drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
diff --git a/drivers/gpu/drm/drm_mode_object.c
b/drivers/gpu/drm/drm_mode_object.c
index 004191d01772..686fba472abf 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -37,24 +37,23 @@ int __drm_mode_object_add(struct drm_device *dev, struct
drm_mode_object *obj,
{
int ret;
- mutex_lock(&dev->mode_config.idr_mutex);
- ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL,
- 1, 0, GFP_KERNEL);
- if (ret >= 0) {
- /*
- * Set up the object linking under the protection of the idr
- * lock so that other users can't see inconsistent state.
- */
- obj->id = ret;
- obj->type = obj_type;
- if (obj_free_cb) {
- obj->free_cb = obj_free_cb;
- kref_init(&obj->refcount);
- }
+ /*
+ * Initialise the object before putting the object in the array
+ * so that other users can't see inconsistent state. The ID is
+ * initialised before the object is inserted into the array with
+ * a write barrier so even RCU-protected walkers can't see an
+ * uninitialised ID.
+ */
+ obj->type = obj_type;
+ if (obj_free_cb) {
+ obj->free_cb = obj_free_cb;
+ kref_init(&obj->refcount);
}
- mutex_unlock(&dev->mode_config.idr_mutex);
- return ret < 0 ? ret : 0;
+ ret = xa_alloc(&dev->mode_config.objects, &obj->id,
+ register_obj ? obj : NULL, xa_limit_31b, GFP_KERNEL);
+
+ return ret;
}
/**
@@ -78,9 +77,7 @@ int drm_mode_object_add(struct drm_device *dev,
void drm_mode_object_register(struct drm_device *dev,
struct drm_mode_object *obj)
{
- mutex_lock(&dev->mode_config.idr_mutex);
- idr_replace(&dev->mode_config.crtc_idr, obj, obj->id);
- mutex_unlock(&dev->mode_config.idr_mutex);
+ xa_store(&dev->mode_config.objects, obj->id, obj, 0);
}
/**
@@ -97,12 +94,10 @@ void drm_mode_object_register(struct drm_device *dev,
void drm_mode_object_unregister(struct drm_device *dev,
struct drm_mode_object *object)
{
- mutex_lock(&dev->mode_config.idr_mutex);
if (object->id) {
- idr_remove(&dev->mode_config.crtc_idr, object->id);
+ xa_erase(&dev->mode_config.objects, object->id);
object->id = 0;
}
- mutex_unlock(&dev->mode_config.idr_mutex);
}
/**
@@ -128,10 +123,10 @@ struct drm_mode_object *__drm_mode_object_find(struct
drm_device *dev,
struct drm_file *file_priv,
uint32_t id, uint32_t type)
{
- struct drm_mode_object *obj = NULL;
+ struct drm_mode_object *obj;
- mutex_lock(&dev->mode_config.idr_mutex);
- obj = idr_find(&dev->mode_config.crtc_idr, id);
+ xa_lock(&dev->mode_config.objects);
+ obj = xa_load(&dev->mode_config.objects, id);
if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
obj = NULL;
if (obj && obj->id != id)
@@ -145,7 +140,7 @@ struct drm_mode_object *__drm_mode_object_find(struct
drm_device *dev,
if (!kref_get_unless_zero(&obj->refcount))
obj = NULL;
}
- mutex_unlock(&dev->mode_config.idr_mutex);
+ xa_unlock(&dev->mode_config.objects);
return obj;
}
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index fbb58264538b..88c8bbf14916 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -90,7 +90,7 @@ struct drm_master {
struct drm_master *lessor;
int lessee_id;
- struct idr leases;
+ struct xarray leases;
struct xarray lessees;
};
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index fea334d99201..64bdf66d878c 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -397,12 +397,12 @@ struct drm_mode_config {
struct mutex idr_mutex;
/**
- * @crtc_idr:
+ * @objects:
*
- * Main KMS ID tracking object. Use this idr for all IDs, fb, crtc,
+ * Main KMS ID tracking object. Use this array for all IDs, fb, crtc,
* connector, modes - just makes life easier to have only one.
*/
- struct idr crtc_idr;
+ struct xarray objects;
/**
* @tiles:
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
