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

[Alan Stern]

On Mon, 3 Jun 2013, Greg KH wrote:

> 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


> @@ -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;
>  }

The test for serial->disconnected got lost.  And the locking isn't 
right; the routine is documented to return with serial->disc_mutex held 
(in the case where the device hasn't been disconnected).

Also, the kref_get() needs to occur within the scope of the table_lock.

I didn't check the rest of the patch for similar errors.  Finding three 
in the first function seemed like enough.  :-)

Alan Stern

--
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/