Re: [PATCH v2 0/3] input: Dynamic Minor Numbers

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 6c58bff..0bc4507 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c

@@ -23,11 +23,11 @@
 #include <linux/input/mt.h>
 #include <linux/major.h>
 #include <linux/device.h>
+#include <linux/cdev.h>
 #include "input-compat.h"

 struct evdev {
        int open;
-       int minor;
        struct input_handle handle;
        wait_queue_head_t wait;
        struct evdev_client __rcu *grab;

@@ -35,6 +35,7 @@ struct evdev {
        spinlock_t client_lock; /* protects client_list */
        struct mutex mutex;
        struct device dev;
+       struct cdev cdev;
        bool exist;
 };

@@ -51,9 +52,6 @@ struct evdev_client {
        struct input_event buffer[];
 };

-static struct evdev *evdev_table[EVDEV_MINORS];
-static DEFINE_MUTEX(evdev_table_mutex);
-
 static void evdev_pass_event(struct evdev_client *client,
                             struct input_event *event,
                             ktime_t mono, ktime_t real)

@@ -285,35 +283,16 @@ static unsigned int evdev_compute_buffer_size(struct input_dev *dev)

 static int evdev_open(struct inode *inode, struct file *file)
 {
-       struct evdev *evdev;
+       struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev);
+       unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev);
        struct evdev_client *client;
-       int i = iminor(inode) - EVDEV_MINOR_BASE;
-       unsigned int bufsize;
        int error;

-       if (i >= EVDEV_MINORS)
-               return -ENODEV;
-
-       error = mutex_lock_interruptible(&evdev_table_mutex);
-       if (error)
-               return error;
-       evdev = evdev_table[i];
-       if (evdev)
-               get_device(&evdev->dev);
-       mutex_unlock(&evdev_table_mutex);
-
-       if (!evdev)
-               return -ENODEV;
-
-       bufsize = evdev_compute_buffer_size(evdev->handle.dev);
-
        client = kzalloc(sizeof(struct evdev_client) +
                                bufsize * sizeof(struct input_event),
                         GFP_KERNEL);
-       if (!client) {
-               error = -ENOMEM;
-               goto err_put_evdev;
-       }
+       if (!client)
+               return -ENOMEM;

        client->bufsize = bufsize;
        spin_lock_init(&client->buffer_lock);

@@ -327,13 +306,12 @@ static int evdev_open(struct inode *inode, struct file *file)
        file->private_data = client;
        nonseekable_open(inode, file);

+       get_device(&evdev->dev);
        return 0;

  err_free_client:
        evdev_detach_client(evdev, client);
        kfree(client);
- err_put_evdev:
-       put_device(&evdev->dev);
        return error;
 }

@@ -915,26 +893,6 @@ static const struct file_operations evdev_fops = {
        .llseek         = no_llseek,
 };

-static int evdev_install_chrdev(struct evdev *evdev)
-{
-       /*
-        * No need to do any locking here as calls to connect and
-        * disconnect are serialized by the input core
-        */
-       evdev_table[evdev->minor] = evdev;
-       return 0;
-}
-
-static void evdev_remove_chrdev(struct evdev *evdev)
-{
-       /*
-        * Lock evdev table to prevent race with evdev_open()
-        */
-       mutex_lock(&evdev_table_mutex);
-       evdev_table[evdev->minor] = NULL;
-       mutex_unlock(&evdev_table_mutex);
-}
-
 /*
  * Mark device non-existent. This disables writes, ioctls and
  * prevents new users from opening the device. Already posted

@@ -953,7 +911,8 @@ static void evdev_cleanup(struct evdev *evdev)

        evdev_mark_dead(evdev);
        evdev_hangup(evdev);
-       evdev_remove_chrdev(evdev);
+
+       cdev_del(&evdev->cdev);

        /* evdev is marked dead so no one else accesses evdev->open */
        if (evdev->open) {

@@ -964,43 +923,47 @@ static void evdev_cleanup(struct evdev *evdev)

 /*
  * Create new evdev device. Note that input core serializes calls
- * to connect and disconnect so we don't need to lock evdev_table here.
+ * to connect and disconnect.
  */
 static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
                         const struct input_device_id *id)
 {
        struct evdev *evdev;
        int minor;
+       int dev_no;
        int error;

-       for (minor = 0; minor < EVDEV_MINORS; minor++)
-               if (!evdev_table[minor])
-                       break;
-
-       if (minor == EVDEV_MINORS) {
-               pr_err("no more free evdev devices\n");
-               return -ENFILE;
+       minor = input_get_new_minor(EVDEV_MINOR_BASE, 8 /*EVDEV_MINORS*/, true);

+       if (minor < 0) {
+               error = minor;
+               pr_err("failed to reserve new minor: %d\n", error);
+               return error;
        }

        evdev = kzalloc(sizeof(struct evdev), GFP_KERNEL);
-       if (!evdev)
-               return -ENOMEM;
+       if (!evdev) {
+               error = -ENOMEM;
+               goto err_free_minor;
+       }

        INIT_LIST_HEAD(&evdev->client_list);
        spin_lock_init(&evdev->client_lock);
        mutex_init(&evdev->mutex);
        init_waitqueue_head(&evdev->wait);
-
-       dev_set_name(&evdev->dev, "event%d", minor);
        evdev->exist = true;
-       evdev->minor = minor;
+
+       dev_no = minor;
+       /* Normalize device number if it falls into legacy range */
+       if (dev_no < EVDEV_MINOR_BASE + EVDEV_MINORS)
+               dev_no -= EVDEV_MINOR_BASE;
+       dev_set_name(&evdev->dev, "event%d", dev_no);

        evdev->handle.dev = input_get_device(dev);
        evdev->handle.name = dev_name(&evdev->dev);
        evdev->handle.handler = handler;
        evdev->handle.private = evdev;

-       evdev->dev.devt = MKDEV(INPUT_MAJOR, EVDEV_MINOR_BASE + minor);
+       evdev->dev.devt = MKDEV(INPUT_MAJOR, minor);
        evdev->dev.class = &input_class;
        evdev->dev.parent = &dev->dev;
        evdev->dev.release = evdev_free;

@@ -1010,7 +973,8 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
        if (error)
                goto err_free_evdev;

-       error = evdev_install_chrdev(evdev);
+       cdev_init(&evdev->cdev, &evdev_fops);
+       error = cdev_add(&evdev->cdev, evdev->dev.devt, 1);
        if (error)
                goto err_unregister_handle;

@@ -1026,6 +990,8 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
        input_unregister_handle(&evdev->handle);
  err_free_evdev:
        put_device(&evdev->dev);
+ err_free_minor:
+       input_free_minor(minor);
        return error;
 }

@@ -1035,6 +1001,7 @@ static void evdev_disconnect(struct input_handle *handle)

        device_del(&evdev->dev);
        evdev_cleanup(evdev);
+       input_free_minor(MINOR(evdev->dev.devt));
        input_unregister_handle(handle);
        put_device(&evdev->dev);
 }

@@ -1050,8 +1017,6 @@ static struct input_handler evdev_handler = {
        .event          = evdev_event,
        .connect        = evdev_connect,
        .disconnect     = evdev_disconnect,
-       .fops           = &evdev_fops,
-       .minor          = EVDEV_MINOR_BASE,
        .name           = "evdev",
        .id_table       = evdev_ids,
 };

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 768e46b..1869cf1 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c

@@ -14,6 +14,7 @@

 #include <linux/init.h>
 #include <linux/types.h>
+#include <linux/idr.h>
 #include <linux/input/mt.h>
 #include <linux/module.h>
 #include <linux/slab.h>

@@ -32,7 +33,9 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
 MODULE_LICENSE("GPL");

-#define INPUT_DEVICES  256
+#define INPUT_MAX_CHAR_DEVICES         1024
+#define INPUT_FIRST_DYNAMIC_DEV                256

+static DEFINE_IDA(input_ida);

 static LIST_HEAD(input_dev_list);
 static LIST_HEAD(input_handler_list);

@@ -45,8 +48,6 @@ static LIST_HEAD(input_handler_list);
  */
 static DEFINE_MUTEX(input_mutex);

-static struct input_handler *input_table[8];
-
 static inline int is_event_supported(unsigned int code,
                                     unsigned long *bm, unsigned int max)
 {

@@ -1144,8 +1145,6 @@ static int input_handlers_seq_show(struct seq_file *seq, void *v)
        seq_printf(seq, "N: Number=%u Name=%s", state->pos, handler->name);
        if (handler->filter)
                seq_puts(seq, " (filter)");
-       if (handler->fops)
-               seq_printf(seq, " Minor=%d", handler->minor);

        seq_putc(seq, '\n');

        return 0;

@@ -1932,22 +1931,14 @@ EXPORT_SYMBOL(input_unregister_device);
 int input_register_handler(struct input_handler *handler)
 {
        struct input_dev *dev;
-       int retval;
+       int error;

-       retval = mutex_lock_interruptible(&input_mutex);
-       if (retval)
-               return retval;
+       error = mutex_lock_interruptible(&input_mutex);
+       if (error)
+               return error;

        INIT_LIST_HEAD(&handler->h_list);

-       if (handler->fops != NULL) {
-               if (input_table[handler->minor >> 5]) {
-                       retval = -EBUSY;
-                       goto out;
-               }
-               input_table[handler->minor >> 5] = handler;
-       }
-
        list_add_tail(&handler->node, &input_handler_list);

        list_for_each_entry(dev, &input_dev_list, node)

@@ -1955,9 +1946,8 @@ int input_register_handler(struct input_handler *handler)

        input_wakeup_procfs_readers();

- out:
        mutex_unlock(&input_mutex);
-       return retval;
+       return 0;
 }
 EXPORT_SYMBOL(input_register_handler);

@@ -1980,9 +1970,6 @@ void input_unregister_handler(struct input_handler *handler)

        list_del_init(&handler->node);

-       if (handler->fops != NULL)
-               input_table[handler->minor >> 5] = NULL;
-
        input_wakeup_procfs_readers();

        mutex_unlock(&input_mutex);

@@ -2099,51 +2086,52 @@ void input_unregister_handle(struct input_handle *handle)
 }
 EXPORT_SYMBOL(input_unregister_handle);

-static int input_open_file(struct inode *inode, struct file *file)
+/**
+ * input_get_new_minor - allocates a new input minor number
+ * @legacy_base: beginning or the legacy range to be searched

+ * @legacy_num: size of legacy range
+ * @allow_dynamic: whether we can also take ID from the dynamic range
+ *
+ * This function allocates a new device minor for from input major namespace.
+ * Caller can request legacy minor by specifying @legacy_base and @legacy_num
+ * parameters and whether ID can be allocated from dynamic range if there are
+ * no free IDs in legacy range.
+ */
+int input_get_new_minor(int legacy_base, unsigned int legacy_num,
+                       bool allow_dynamic)
 {
-       struct input_handler *handler;
-       const struct file_operations *old_fops, *new_fops = NULL;
-       int err;
-
-       err = mutex_lock_interruptible(&input_mutex);
-       if (err)
-               return err;
-
-       /* No load-on-demand here? */
-       handler = input_table[iminor(inode) >> 5];
-       if (handler)
-               new_fops = fops_get(handler->fops);
-
-       mutex_unlock(&input_mutex);
-
        /*
-        * That's _really_ odd. Usually NULL ->open means "nothing special",
-        * not "no device". Oh, well...
+        * This function should be called from input handler's ->connect()
+        * methods, which are serialized with input_mutex, so no additional
+        * locking is needed here.
         */
-       if (!new_fops || !new_fops->open) {
-               fops_put(new_fops);
-               err = -ENODEV;
-               goto out;
+       if (legacy_base >= 0) {
+               int minor = ida_simple_get(&input_ida,
+                                          legacy_base,
+                                          legacy_base + legacy_num,
+                                          GFP_KERNEL);
+               if (minor >= 0 || !allow_dynamic)
+                       return minor;
        }

-       old_fops = file->f_op;
-       file->f_op = new_fops;
-
-       err = new_fops->open(inode, file);
-       if (err) {
-               fops_put(file->f_op);
-               file->f_op = fops_get(old_fops);
-       }
-       fops_put(old_fops);
-out:
-       return err;
+       return ida_simple_get(&input_ida,
+                             INPUT_FIRST_DYNAMIC_DEV, INPUT_MAX_CHAR_DEVICES,
+                             GFP_KERNEL);
 }
+EXPORT_SYMBOL(input_get_new_minor);

-static const struct file_operations input_fops = {
-       .owner = THIS_MODULE,
-       .open = input_open_file,
-       .llseek = noop_llseek,
-};
+/**
+ * input_free_minor - release previously allocated minor
+ * @minor: minor to be released
+ *
+ * This function releases previously allocated input minor so that it can be
+ * reused later.
+ */
+void input_free_minor(unsigned int minor)
+{
+       ida_simple_remove(&input_ida, minor);
+}
+EXPORT_SYMBOL(input_free_minor);

 static int __init input_init(void)
 {

@@ -2159,7 +2147,8 @@ static int __init input_init(void)
        if (err)
                goto fail1;

-       err = register_chrdev(INPUT_MAJOR, "input", &input_fops);
+       err = register_chrdev_region(MKDEV(INPUT_MAJOR, 0),
+                                    INPUT_MAX_CHAR_DEVICES, "input");
        if (err) {
                pr_err("unable to register char major %d", INPUT_MAJOR);
                goto fail2;

@@ -2175,7 +2164,8 @@ static int __init input_init(void)
 static void __exit input_exit(void)
 {
        input_proc_exit();
-       unregister_chrdev(INPUT_MAJOR, "input");
+       unregister_chrdev_region(MKDEV(INPUT_MAJOR, 0),
+                                INPUT_MAX_CHAR_DEVICES);
        class_unregister(&input_class);
 }