Re: [RFC] raise the maximum number of usb-serial devices to 512

[Greg KH]

On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> But, IMHO, a nicer approach would be to make the allocation completely
> dynamic, using e.g. the idr subsystem. Static tables are always feel
> like straight jackets to me, no matter how big they are :)

You are right, I didn't change the code to use idr (it predates idr by
about a decade or so), because I thought we needed the "rage" logic that
the usb-serial minor reservation does.

But I'm not so sure anymore, so here's a patch to change to use the idr
code, and should remove all minor number limitations (well 65k is the
limit the tty core should be setting I think.)

Tobias, can you test this patch out?  Note, I only compiled it, did not
get the chance to actually run it, so it might not work at all.

thanks,

greg k-h

Subject: [PATCH] usb: serial: remove minor number limitation of 255


Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

 drivers/usb/serial/usb-serial.c |   86 +++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 4753c00..74b6f08 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -37,6 +37,7 @@
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 #include <linux/kfifo.h>
+#include <linux/idr.h>
 #include "pl2303.h"
 
 #define DRIVER_AUTHOR "Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>"
@@ -49,7 +50,7 @@
    drivers depend on it.
 */
 
-static struct usb_serial *serial_table[SERIAL_TTY_MINORS];
+static DEFINE_IDR(serial_minors);
 static DEFINE_MUTEX(table_lock);
 static LIST_HEAD(usb_serial_driver_list);
 
@@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
 struct usb_serial *usb_serial_get_by_index(unsigned index)
 {
 	struct usb_serial *serial;
+	struct usb_serial_port *port;
 
 	mutex_lock(&table_lock);
-	serial = serial_table[index];
-
-	if (serial) {
-		mutex_lock(&serial->disc_mutex);
-		if (serial->disconnected) {
-			mutex_unlock(&serial->disc_mutex);
-			serial = NULL;
-		} else {
-			kref_get(&serial->kref);
-		}
-	}
+	port = idr_find(&serial_minors, index);
 	mutex_unlock(&table_lock);
+	if (!port)
+		return NULL;
+
+	serial = port->serial;
+	kref_get(&serial->kref);
 	return serial;
 }
 
-static struct usb_serial *get_free_serial(struct usb_serial *serial,
-					int num_ports, unsigned int *minor)
+static int get_free_port(struct usb_serial_port *port)
 {
-	unsigned int i, j;
-	int good_spot;
-
-	dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
+	int i;
 
-	*minor = 0;
 	mutex_lock(&table_lock);
-	for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
-		if (serial_table[i])
-			continue;
+	i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
+	if (i < 0)
+		return -EEXIST;
+	port->number = i;
+	mutex_unlock(&table_lock);
+	return i;
+}
 
-		good_spot = 1;
-		for (j = 1; j <= num_ports-1; ++j)
-			if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
-				good_spot = 0;
-				i += j;
-				break;
-			}
-		if (good_spot == 0)
-			continue;
+static int get_free_serial(struct usb_serial *serial, int num_ports,
+			   unsigned int *minor)
+{
+	unsigned int i;
+	unsigned int x;
 
-		*minor = i;
-		j = 0;
-		dev_dbg(&serial->interface->dev, "%s - minor base = %d\n", __func__, *minor);
-		for (i = *minor; (i < (*minor + num_ports)) && (i < SERIAL_TTY_MINORS); ++i) {
-			serial_table[i] = serial;
-			serial->port[j++]->number = i;
-		}
-		mutex_unlock(&table_lock);
-		return serial;
+	dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
+
+	*minor = 0xffffffff;
+	for (i = 0; i < num_ports; ++i) {
+		x = get_free_port(serial->port[i]);
+		if (x < 0)
+			goto error;
+		if (*minor == 0xffffffff)
+			*minor = x;
 	}
-	mutex_unlock(&table_lock);
-	return NULL;
+	return 0;
+error:
+	// FIXME unwind the already allocated minors
+	return -ENODEV;
 }
 
 static void return_serial(struct usb_serial *serial)
@@ -122,7 +116,7 @@ static void return_serial(struct usb_serial *serial)
 
 	mutex_lock(&table_lock);
 	for (i = 0; i < serial->num_ports; ++i)
-		serial_table[serial->minor + i] = NULL;
+		idr_remove(&serial_minors, serial->port[i]->number);
 	mutex_unlock(&table_lock);
 }
 
@@ -1041,7 +1035,7 @@ static int usb_serial_probe(struct usb_interface *interface,
 	 */
 	serial->disconnected = 1;
 
-	if (get_free_serial(serial, num_ports, &minor) == NULL) {
+	if (get_free_serial(serial, num_ports, &minor)) {
 		dev_err(ddev, "No more free serial devices\n");
 		goto probe_error;
 	}
@@ -1225,7 +1219,6 @@ static struct usb_driver usb_serial_driver = {
 
 static int __init usb_serial_init(void)
 {
-	int i;
 	int result;
 
 	usb_serial_tty_driver = alloc_tty_driver(SERIAL_TTY_MINORS);
@@ -1233,9 +1226,6 @@ static int __init usb_serial_init(void)
 		return -ENOMEM;
 
 	/* Initialize our global data */
-	for (i = 0; i < SERIAL_TTY_MINORS; ++i)
-		serial_table[i] = NULL;
-
 	result = bus_register(&usb_serial_bus_type);
 	if (result) {
 		pr_err("%s - registering bus driver failed\n", __func__);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/