On 8/26/20 8:24 AM, Christoph Hellwig wrote:
> None of the complicated overlapping regions bits of the kobj_map are
> required for the character device lookup, so just a trivial xarray
> instead.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/char_dev.c | 94 +++++++++++++++++++++++++--------------------------
> fs/dcache.c | 1 -
> fs/internal.h | 5 ---
> 3 files changed, 46 insertions(+), 54 deletions(-)
>
> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index ba0ded7842a779..6c4d6c4938f14b 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -17,7 +17,6 @@
> #include <linux/seq_file.h>
>
> #include <linux/kobject.h>
> -#include <linux/kobj_map.h>
> #include <linux/cdev.h>
> #include <linux/mutex.h>
> #include <linux/backing-dev.h>
> @@ -25,8 +24,7 @@
>
> #include "internal.h"
>
> -static struct kobj_map *cdev_map;
> -
> +static DEFINE_XARRAY(cdev_map);
> static DEFINE_MUTEX(chrdevs_lock);
>
> #define CHRDEV_MAJOR_HASH_SIZE 255
> @@ -367,6 +365,29 @@ void cdev_put(struct cdev *p)
> }
> }
>
> +static struct cdev *cdev_lookup(dev_t dev)
> +{
> + struct cdev *cdev;
> +
> +retry:
> + mutex_lock(&chrdevs_lock);
> + cdev = xa_load(&cdev_map, dev);
> + if (!cdev) {
> + mutex_unlock(&chrdevs_lock);
> +
> + if (request_module("char-major-%d-%d",
> + MAJOR(dev), MINOR(dev)) > 0)
> + /* Make old-style 2.4 aliases work */
> + request_module("char-major-%d", MAJOR(dev));
> + goto retry;
> + }
> +
> + if (!cdev_get(cdev))
> + cdev = NULL;
> + mutex_unlock(&chrdevs_lock);
> + return cdev;
> +}
> +
> /*
> * Called every time a character special file is opened
> */
> @@ -380,13 +401,10 @@ static int chrdev_open(struct inode *inode, struct file *filp)
> spin_lock(&cdev_lock);
> p = inode->i_cdev;
> if (!p) {
> - struct kobject *kobj;
> - int idx;
> spin_unlock(&cdev_lock);
> - kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx);
> - if (!kobj)
> + new = cdev_lookup(inode->i_rdev);
> + if (!new)
> return -ENXIO;
> - new = container_of(kobj, struct cdev, kobj);
> spin_lock(&cdev_lock);
> /* Check i_cdev again in case somebody beat us to it while
> we dropped the lock. */
> @@ -454,18 +472,6 @@ const struct file_operations def_chr_fops = {
> .llseek = noop_llseek,
> };
>
> -static struct kobject *exact_match(dev_t dev, int *part, void *data)
> -{
> - struct cdev *p = data;
> - return &p->kobj;
> -}
> -
> -static int exact_lock(dev_t dev, void *data)
> -{
> - struct cdev *p = data;
> - return cdev_get(p) ? 0 : -1;
> -}
> -
> /**
> * cdev_add() - add a char device to the system
> * @p: the cdev structure for the device
> @@ -478,7 +484,7 @@ static int exact_lock(dev_t dev, void *data)
> */
> int cdev_add(struct cdev *p, dev_t dev, unsigned count)
> {
> - int error;
> + int error, i;
>
> p->dev = dev;
> p->count = count;
> @@ -486,14 +492,22 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count)
> if (WARN_ON(dev == WHITEOUT_DEV))
> return -EBUSY;
>
> - error = kobj_map(cdev_map, dev, count, NULL,
> - exact_match, exact_lock, p);
> - if (error)
> - return error;
> + mutex_lock(&chrdevs_lock);
> + for (i = 0; i < count; i++) {
> + error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> + if (error)
> + goto out_unwind;
> + }
> + mutex_unlock(&chrdevs_lock);
>
> kobject_get(p->kobj.parent);
> -
> return 0;
> +
> +out_unwind:
> + while (--i >= 0)
> + xa_erase(&cdev_map, dev + i);
> + mutex_unlock(&chrdevs_lock);
> + return error;
> }
>
> /**
Do you really need the mutex?
Wouldn't xa_store_range() be better and avoid the mutex?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
