Commit f6470813 authored by Oliver Neukum's avatar Oliver Neukum Committed by Greg Kroah-Hartman

[PATCH] USB: sleeping problems in cyberjack driver

this driver has locking problems. Here's the first round of fixes
for the obvious cases.

- it makes clear differences between completion handlers and task context
- it fixes cases of sleeping in interrupt
parent 81377879
...@@ -295,7 +295,6 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs ) ...@@ -295,7 +295,6 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs )
{ {
struct usb_serial_port *port = (struct usb_serial_port *)urb->context; struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
struct cyberjack_private *priv = usb_get_serial_port_data(port); struct cyberjack_private *priv = usb_get_serial_port_data(port);
unsigned long flags;
struct usb_serial *serial; struct usb_serial *serial;
unsigned char *data = urb->transfer_buffer; unsigned char *data = urb->transfer_buffer;
int result; int result;
...@@ -323,13 +322,13 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs ) ...@@ -323,13 +322,13 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs )
/* This is a announcement of coming bulk_ins. */ /* This is a announcement of coming bulk_ins. */
unsigned short size = ((unsigned short)data[3]<<8)+data[2]+3; unsigned short size = ((unsigned short)data[3]<<8)+data[2]+3;
spin_lock_irqsave(&priv->lock, flags); spin_lock(&priv->lock);
old_rdtodo = priv->rdtodo; old_rdtodo = priv->rdtodo;
if( (old_rdtodo+size)<(old_rdtodo) ) { if( (old_rdtodo+size)<(old_rdtodo) ) {
dbg( "To many bulk_in urbs to do." ); dbg( "To many bulk_in urbs to do." );
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock(&priv->lock);
goto resubmit; goto resubmit;
} }
...@@ -338,11 +337,11 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs ) ...@@ -338,11 +337,11 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs )
dbg("%s - rdtodo: %d", __FUNCTION__, priv->rdtodo); dbg("%s - rdtodo: %d", __FUNCTION__, priv->rdtodo);
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock(&priv->lock);
if( !old_rdtodo ) { if( !old_rdtodo ) {
port->read_urb->dev = port->serial->dev; port->read_urb->dev = port->serial->dev;
result = usb_submit_urb(port->read_urb, GFP_KERNEL); result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
if( result ) if( result )
err("%s - failed resubmitting read urb, error %d", __FUNCTION__, result); err("%s - failed resubmitting read urb, error %d", __FUNCTION__, result);
dbg("%s - usb_submit_urb(read urb)", __FUNCTION__); dbg("%s - usb_submit_urb(read urb)", __FUNCTION__);
...@@ -351,7 +350,7 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs ) ...@@ -351,7 +350,7 @@ static void cyberjack_read_int_callback( struct urb *urb, struct pt_regs *regs )
resubmit: resubmit:
port->interrupt_in_urb->dev = port->serial->dev; port->interrupt_in_urb->dev = port->serial->dev;
result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); result = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC);
if (result) if (result)
err(" usb_submit_urb(read int) failed"); err(" usb_submit_urb(read int) failed");
dbg("%s - usb_submit_urb(int urb)", __FUNCTION__); dbg("%s - usb_submit_urb(int urb)", __FUNCTION__);
...@@ -361,7 +360,6 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs) ...@@ -361,7 +360,6 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs)
{ {
struct usb_serial_port *port = (struct usb_serial_port *)urb->context; struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
struct cyberjack_private *priv = usb_get_serial_port_data(port); struct cyberjack_private *priv = usb_get_serial_port_data(port);
unsigned long flags;
struct usb_serial *serial = get_usb_serial (port, __FUNCTION__); struct usb_serial *serial = get_usb_serial (port, __FUNCTION__);
struct tty_struct *tty; struct tty_struct *tty;
unsigned char *data = urb->transfer_buffer; unsigned char *data = urb->transfer_buffer;
...@@ -397,7 +395,7 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs) ...@@ -397,7 +395,7 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs)
tty_flip_buffer_push(tty); tty_flip_buffer_push(tty);
} }
spin_lock_irqsave(&priv->lock, flags); spin_lock(&priv->lock);
/* Reduce urbs to do by one. */ /* Reduce urbs to do by one. */
priv->rdtodo-=urb->actual_length; priv->rdtodo-=urb->actual_length;
...@@ -405,14 +403,14 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs) ...@@ -405,14 +403,14 @@ static void cyberjack_read_bulk_callback (struct urb *urb, struct pt_regs *regs)
if ( priv->rdtodo<0 ) priv->rdtodo = 0; if ( priv->rdtodo<0 ) priv->rdtodo = 0;
todo = priv->rdtodo; todo = priv->rdtodo;
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock(&priv->lock);
dbg("%s - rdtodo: %d", __FUNCTION__, todo); dbg("%s - rdtodo: %d", __FUNCTION__, todo);
/* Continue to read if we have still urbs to do. */ /* Continue to read if we have still urbs to do. */
if( todo /* || (urb->actual_length==port->bulk_in_endpointAddress)*/ ) { if( todo /* || (urb->actual_length==port->bulk_in_endpointAddress)*/ ) {
port->read_urb->dev = port->serial->dev; port->read_urb->dev = port->serial->dev;
result = usb_submit_urb(port->read_urb, GFP_KERNEL); result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
if (result) if (result)
err("%s - failed resubmitting read urb, error %d", __FUNCTION__, result); err("%s - failed resubmitting read urb, error %d", __FUNCTION__, result);
dbg("%s - usb_submit_urb(read urb)", __FUNCTION__); dbg("%s - usb_submit_urb(read urb)", __FUNCTION__);
...@@ -423,7 +421,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -423,7 +421,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
{ {
struct usb_serial_port *port = (struct usb_serial_port *)urb->context; struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
struct cyberjack_private *priv = usb_get_serial_port_data(port); struct cyberjack_private *priv = usb_get_serial_port_data(port);
unsigned long flags;
struct usb_serial *serial = get_usb_serial (port, __FUNCTION__); struct usb_serial *serial = get_usb_serial (port, __FUNCTION__);
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
...@@ -438,7 +435,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -438,7 +435,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
return; return;
} }
spin_lock_irqsave(&priv->lock, flags); spin_lock(&priv->lock);
/* only do something if we have more data to send */ /* only do something if we have more data to send */
if( priv->wrfilled ) { if( priv->wrfilled ) {
...@@ -446,7 +443,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -446,7 +443,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
if (port->write_urb->status == -EINPROGRESS) { if (port->write_urb->status == -EINPROGRESS) {
dbg("%s - already writing", __FUNCTION__); dbg("%s - already writing", __FUNCTION__);
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock(&priv->lock);
return; return;
} }
...@@ -492,7 +489,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -492,7 +489,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
} }
exit: exit:
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock(&priv->lock);
schedule_work(&port->work); schedule_work(&port->work);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment