Skip to contentSkip to conversation

drivers: char: Replace bit operation functions with IDA allocator.

PATCH In review Varsha Rao 2 participants 4 messages Apr 10, 2017 1 message not in the archive Char and misc drivers

lore
Share & tools
lore

Varsha Rao

author
#1 lore raw

PATCH 1/2 drivers: char: Replace bit operation functions with IDA allocator.

In reply to a message not in the archive

Replace bit operation functions with IDA allocator functions. As IDA allocation is simpler, faster, more space efficient and it generates small integer IDs which can be used as minor device numbers.

---
 drivers/char/misc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/char/misc.c +10 -6
index c9cd1ea..5786281 100644
@@ -60,7 +60,7 @@ static DEFINE_MUTEX(misc_mtx);
  * Assigned numbers, used for dynamic minors
  */
 #define DYNAMIC_MINORS 64 /* like dynamic majors */
-static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
+static DEFINE_IDA(misc_minors_ida);
 
 #ifdef CONFIG_PROC_FS
 static void *misc_seq_start(struct seq_file *seq, loff_t *pos)
@@ -193,14 +193,18 @@ int misc_register(struct miscdevice *misc)
 	mutex_lock(&misc_mtx);
 
 	if (is_dynamic) {
-		int i = find_first_zero_bit(misc_minors, DYNAMIC_MINORS);
+		int i = ida_simple_get(&misc_minors_ida, 0,
+				       DYNAMIC_MINORS, GFP_KERNEL);
 
 		if (i >= DYNAMIC_MINORS) {
 			err = -EBUSY;
 			goto out;
-		}
+		} else if (i < 0) {
+			err = i;
+			goto out;
+		} else {
 		misc->minor = DYNAMIC_MINORS - i - 1;
-		set_bit(i, misc_minors);
+		}
 	} else {
 		struct miscdevice *c;
 
@@ -222,7 +226,7 @@ int misc_register(struct miscdevice *misc)
 			int i = DYNAMIC_MINORS - misc->minor - 1;
 
 			if (i < DYNAMIC_MINORS && i >= 0)
-				clear_bit(i, misc_minors);
+				ida_simple_remove(&misc_minors_ida, i);
 			misc->minor = MISC_DYNAMIC_MINOR;
 		}
 		err = PTR_ERR(misc->this_device);
@@ -258,7 +262,7 @@ void misc_deregister(struct miscdevice *misc)
 	list_del(&misc->list);
 	device_destroy(misc_class, MKDEV(MISC_MAJOR, misc->minor));
 	if (i < DYNAMIC_MINORS && i >= 0)
-		clear_bit(i, misc_minors);
+		ida_simple_remove(&misc_minors_ida, i);
 	mutex_unlock(&misc_mtx);
 }
 
Signature · 1 line
2.9.3
author
#3 lore raw

PATCH 2/2 drivers: char: Replace fail_printk with fail_print.

In reply to a message not in the archive

As printk is no longer used here, rename the label fail_printk as fail_print.

---
 drivers/char/misc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/char/misc.c +2 -2
index 5786281..0fed477 100644
@@ -293,11 +293,11 @@ static int __init misc_init(void)
 
 	err = -EIO;
 	if (register_chrdev(MISC_MAJOR, "misc", &misc_fops))
-		goto fail_printk;
+		goto fail_print;
 	misc_class->devnode = misc_devnode;
 	return 0;
 
-fail_printk:
+fail_print:
 	pr_err("unable to get major %d for misc devices\n", MISC_MAJOR);
 	class_destroy(misc_class);
 fail_remove:
Signature · 1 line
2.9.3
#4 lore raw

Re: [PATCH 2/2] drivers: char: Replace fail_printk with fail_print.

Quoted 15 lines · Varsha Rao

As printk is no longer used here, rename the label fail_printk as fail_print.

---
 drivers/char/misc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/char/misc.c +0 -0
index 5786281..0fed477 100644
@@ -293,11 +293,11 @@ static int __init misc_init(void)
 
 	err = -EIO;

drivers/char/misc.c static int __init misc_init(void) · line 302

Quoted 10 lines
drivers/char/misc.c +2 -2
 	if (register_chrdev(MISC_MAJOR, "misc", &misc_fops))
-		goto fail_printk;
+		goto fail_print;
 	misc_class->devnode = misc_devnode;
 	return 0;
 
-fail_printk:
+fail_print:
 	pr_err("unable to get major %d for misc devices\n", MISC_MAJOR);
 	class_destroy(misc_class);

That's really a pointless change, don't you think?

sorry,

greg k-h

#2 lore raw
In reply to Varsha Rao
Quoted 3 lines · Varsha Rao

Replace bit operation functions with IDA allocator functions. As IDA allocation is simpler, faster, more space efficient and it generates small integer IDs which can be used as minor device numbers.

The allocation is different than the bitfield searching is? How exactly? What is the old result vs. the new result of this change?

How did you test this?

You have to really sell this change, right now it looks pretty pointless...

thanks,

greg k-h