On Wed, Jun 05, 2013 at 10:54:55AM -0700, Greg KH wrote:
> From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> This moves the allocation of minor device numbers from a static array to
> be dynamic, using the idr interface. This means that you could
> potentially get "gaps" in a minor number range for a single USB serial
> device with multiple ports, but all should still work properly.
>
> Note, we still have the limitation of 255 USB to serial devices in the
> system, as that is all we are registering with the TTY layer at this
> point in time.
>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[...]
> -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)
> + goto exit;
> + port->minor = i;
> +exit:
> + 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 j;
> + 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, ++j) {
> - serial_table[i] = serial;
> - serial->port[j]->minor = i;
> - serial->port[j]->port_number = i - *minor;
> - }
> - mutex_unlock(&table_lock);
> - return serial;
> + dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> +
> + *minor = 0xffffffff;
You could use SERIAL_TTY_NO_MINOR here -- if needed at all, as it has
already been set in create_serial.
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> + goto error;
> + if (*minor == 0xffffffff)
> + *minor = x;
We must not update *minor until all port minors have been allocated, or
idr_remove might get called for unallocated minors or even minor numbers
of other ports in return_serial when the serial struct is destroyed.
> + serial->port[i]->port_number = i;
> }
> - mutex_unlock(&table_lock);
> - return NULL;
> + return 0;
> +error:
> + /* unwind the already allocated minors */
> + for (j = 0; j < i; ++j)
> + idr_remove(&serial_minors, serial->port[j]->minor);
> + return x;
table_lock?
> }
>
> static void return_serial(struct usb_serial *serial)
> @@ -123,7 +128,7 @@ static void return_serial(struct usb_ser
>
> 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]->minor);
> mutex_unlock(&table_lock);
> }
>
[...]
Looks good otherwise.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
