| Message ID | 1404723967-24245-2-git-send-email-olivier@sobrie.be |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Olivier Sobrie > When the module sends bursts of data, sometimes a deadlock happens in > the hso driver when the tty buffer doesn't get the chance to be flushed > quickly enough. > > To avoid this, first, we remove the endless while loop in > put_rx_bufdata() which is the root cause of the deadlock. > Secondly, when there is no room anymore in the tty buffer, we set up a > timer of 100 msecs to give a chance to the upper layer to flush the tty > buffer and make room for new data. What is the timer for? You need to get the sending code woken up by the urb completion. David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, On Mon, Jul 07, 2014 at 09:13:53AM +0000, David Laight wrote: > From: Olivier Sobrie > > When the module sends bursts of data, sometimes a deadlock happens in > > the hso driver when the tty buffer doesn't get the chance to be flushed > > quickly enough. > > > > To avoid this, first, we remove the endless while loop in > > put_rx_bufdata() which is the root cause of the deadlock. > > Secondly, when there is no room anymore in the tty buffer, we set up a > > timer of 100 msecs to give a chance to the upper layer to flush the tty > > buffer and make room for new data. > > What is the timer for? > You need to get the sending code woken up by the urb completion. In put_rxbuf_data() (which can be called under irq disabled), tty_flip_buffer_push() is called and schedules a push of the tty buffer. When the buffer is full, I give some time to the above layer in order to flush it. The timer is used to recall put_rxbuf_data_and_resubmit_bulk_urb() later in order to read the remaining data stored in "urb->transfer_buffer" and then to resubmit the urb to receive more data from the gsm module. I don't understand what you mean by "getting the sending code woken up". Calling tty_port_tty_wakeup()?? We are in the receive path... Thanks,
From: Olivier Sobrie > Hi David, > > On Mon, Jul 07, 2014 at 09:13:53AM +0000, David Laight wrote: > > From: Olivier Sobrie > > > When the module sends bursts of data, sometimes a deadlock happens in > > > the hso driver when the tty buffer doesn't get the chance to be flushed > > > quickly enough. > > > > > > To avoid this, first, we remove the endless while loop in > > > put_rx_bufdata() which is the root cause of the deadlock. > > > Secondly, when there is no room anymore in the tty buffer, we set up a > > > timer of 100 msecs to give a chance to the upper layer to flush the tty > > > buffer and make room for new data. > > > > What is the timer for? > > You need to get the sending code woken up by the urb completion. > > In put_rxbuf_data() (which can be called under irq disabled), > tty_flip_buffer_push() is called and schedules a push of the tty buffer. > When the buffer is full, I give some time to the above layer in order > to flush it. > The timer is used to recall put_rxbuf_data_and_resubmit_bulk_urb() > later in order to read the remaining data stored in > "urb->transfer_buffer" and then to resubmit the urb to receive more data > from the gsm module. > I don't understand what you mean by "getting the sending code woken up". > Calling tty_port_tty_wakeup()?? We are in the receive path... Sorry, it isn't at all clear from the general description whether you are referring to the transmit or receive path. 'flush' can mean all sorts of things ... In either case a 100ms delay to data doesn't seem right at all. David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 07, 2014 at 12:55:44PM +0000, David Laight wrote: > From: Olivier Sobrie > > Hi David, > > > > On Mon, Jul 07, 2014 at 09:13:53AM +0000, David Laight wrote: > > > From: Olivier Sobrie > > > > When the module sends bursts of data, sometimes a deadlock happens in > > > > the hso driver when the tty buffer doesn't get the chance to be flushed > > > > quickly enough. > > > > > > > > To avoid this, first, we remove the endless while loop in > > > > put_rx_bufdata() which is the root cause of the deadlock. > > > > Secondly, when there is no room anymore in the tty buffer, we set up a > > > > timer of 100 msecs to give a chance to the upper layer to flush the tty > > > > buffer and make room for new data. > > > > > > What is the timer for? > > > You need to get the sending code woken up by the urb completion. > > > > In put_rxbuf_data() (which can be called under irq disabled), > > tty_flip_buffer_push() is called and schedules a push of the tty buffer. > > When the buffer is full, I give some time to the above layer in order > > to flush it. > > The timer is used to recall put_rxbuf_data_and_resubmit_bulk_urb() > > later in order to read the remaining data stored in > > "urb->transfer_buffer" and then to resubmit the urb to receive more data > > from the gsm module. > > I don't understand what you mean by "getting the sending code woken up". > > Calling tty_port_tty_wakeup()?? We are in the receive path... > > Sorry, it isn't at all clear from the general description whether you > are referring to the transmit or receive path. > > 'flush' can mean all sorts of things ... > > In either case a 100ms delay to data doesn't seem right at all. An option to avoid the delay, is to replace the timer by a workqueue and to schedule work in put_rxbuf_data_and_resubmit_bulk_urb() when put_rxbuf_data() returns something greater than 0. If you have better ideas, let me know. Thanks,
On Mon, 2014-07-07 at 11:06 +0200, Olivier Sobrie wrote: > When the module sends bursts of data, sometimes a deadlock happens in > the hso driver when the tty buffer doesn't get the chance to be flushed > quickly enough. > > To avoid this, first, we remove the endless while loop in > put_rx_bufdata() which is the root cause of the deadlock. > Secondly, when there is no room anymore in the tty buffer, we set up a > timer of 100 msecs to give a chance to the upper layer to flush the tty > buffer and make room for new data. I assume this problem happens when using PPP for data transfer, instead of using the network port that the modules expose? Or maybe the NMEA port? I can't imagine that AT commands would make this happen... Dan > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > --- > drivers/net/usb/hso.c | 51 +++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c > index 9ca2b41..1dff74f 100644 > --- a/drivers/net/usb/hso.c > +++ b/drivers/net/usb/hso.c > @@ -106,6 +106,8 @@ > > #define MAX_RX_URBS 2 > > +#define UNTHROTTLE_TIMEOUT 100 /* msecs */ > + > /*****************************************************************************/ > /* Debugging functions */ > /*****************************************************************************/ > @@ -261,6 +263,7 @@ struct hso_serial { > u16 curr_rx_urb_offset; > u8 rx_urb_filled[MAX_RX_URBS]; > struct tasklet_struct unthrottle_tasklet; > + struct timer_list unthrottle_timer; > }; > > struct hso_device { > @@ -1161,13 +1164,18 @@ static void put_rxbuf_data_and_resubmit_bulk_urb(struct hso_serial *serial) > while (serial->rx_urb_filled[serial->curr_rx_urb_idx]) { > curr_urb = serial->rx_urb[serial->curr_rx_urb_idx]; > count = put_rxbuf_data(curr_urb, serial); > - if (count == -1) > - return; > if (count == 0) { > serial->curr_rx_urb_idx++; > if (serial->curr_rx_urb_idx >= serial->num_rx_urbs) > serial->curr_rx_urb_idx = 0; > hso_resubmit_rx_bulk_urb(serial, curr_urb); > + } else if (count > 0) { > + mod_timer(&serial->unthrottle_timer, > + jiffies > + + msecs_to_jiffies(UNTHROTTLE_TIMEOUT)); > + break; > + } else { > + break; > } > } > } > @@ -1251,6 +1259,11 @@ static void hso_unthrottle(struct tty_struct *tty) > tasklet_hi_schedule(&serial->unthrottle_tasklet); > } > > +static void hso_unthrottle_schedule(unsigned long data) > +{ > + tasklet_schedule((struct tasklet_struct *)data); > +} > + > /* open the requested serial port */ > static int hso_serial_open(struct tty_struct *tty, struct file *filp) > { > @@ -1286,6 +1299,10 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) > tasklet_init(&serial->unthrottle_tasklet, > (void (*)(unsigned long))hso_unthrottle_tasklet, > (unsigned long)serial); > + serial->unthrottle_timer.function = hso_unthrottle_schedule; > + serial->unthrottle_timer.data = > + (unsigned long)&serial->unthrottle_tasklet; > + init_timer(&serial->unthrottle_timer); > result = hso_start_serial_device(serial->parent, GFP_KERNEL); > if (result) { > hso_stop_serial_device(serial->parent); > @@ -1333,6 +1350,7 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) > tty_port_tty_set(&serial->port, NULL); > if (!usb_gone) > hso_stop_serial_device(serial->parent); > + del_timer_sync(&serial->unthrottle_timer); > tasklet_kill(&serial->unthrottle_tasklet); > } > > @@ -2012,22 +2030,23 @@ static int put_rxbuf_data(struct urb *urb, struct hso_serial *serial) > > tty = tty_port_tty_get(&serial->port); > > + if (tty && test_bit(TTY_THROTTLED, &tty->flags)) { > + tty_kref_put(tty); > + return -1; > + } > + > /* Push data to tty */ > - write_length_remaining = urb->actual_length - > - serial->curr_rx_urb_offset; > D1("data to push to tty"); > - while (write_length_remaining) { > - if (tty && test_bit(TTY_THROTTLED, &tty->flags)) { > - tty_kref_put(tty); > - return -1; > - } > - curr_write_len = tty_insert_flip_string(&serial->port, > - urb->transfer_buffer + serial->curr_rx_urb_offset, > - write_length_remaining); > - serial->curr_rx_urb_offset += curr_write_len; > - write_length_remaining -= curr_write_len; > - tty_flip_buffer_push(&serial->port); > - } > + write_length_remaining = urb->actual_length - > + serial->curr_rx_urb_offset; > + curr_write_len = > + tty_insert_flip_string(&serial->port, > + urb->transfer_buffer > + + serial->curr_rx_urb_offset, > + write_length_remaining); > + serial->curr_rx_urb_offset += curr_write_len; > + write_length_remaining -= curr_write_len; > + tty_flip_buffer_push(&serial->port); > tty_kref_put(tty); > > if (write_length_remaining == 0) { -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dan, On Mon, Jul 07, 2014 at 11:41:20AM -0500, Dan Williams wrote: > On Mon, 2014-07-07 at 11:06 +0200, Olivier Sobrie wrote: > > When the module sends bursts of data, sometimes a deadlock happens in > > the hso driver when the tty buffer doesn't get the chance to be flushed > > quickly enough. > > > > To avoid this, first, we remove the endless while loop in > > put_rx_bufdata() which is the root cause of the deadlock. > > Secondly, when there is no room anymore in the tty buffer, we set up a > > timer of 100 msecs to give a chance to the upper layer to flush the tty > > buffer and make room for new data. > > I assume this problem happens when using PPP for data transfer, instead > of using the network port that the modules expose? Or maybe the NMEA > port? I can't imagine that AT commands would make this happen... Yes it happens when using ppp for data transfer. The transfer of a large file sometimes triggers the problem.
From: Olivier Sobrie <olivier@sobrie.be> Date: Mon, 7 Jul 2014 11:06:07 +0200 > When the module sends bursts of data, sometimes a deadlock happens in > the hso driver when the tty buffer doesn't get the chance to be flushed > quickly enough. > > To avoid this, first, we remove the endless while loop in > put_rx_bufdata() which is the root cause of the deadlock. > Secondly, when there is no room anymore in the tty buffer, we set up a > timer of 100 msecs to give a chance to the upper layer to flush the tty > buffer and make room for new data. > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> I agree with the feedback you've been given in that adding a delay like this is really not a reasonable solution. Why is it so difficult to make the event which places the data there trigger to necessary calls to pull the data out of the URB transfer buffer? This should be totally and completely event based. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, On Tue, Jul 08, 2014 at 04:16:33PM -0700, David Miller wrote: > From: Olivier Sobrie <olivier@sobrie.be> > Date: Mon, 7 Jul 2014 11:06:07 +0200 > > > When the module sends bursts of data, sometimes a deadlock happens in > > the hso driver when the tty buffer doesn't get the chance to be flushed > > quickly enough. > > > > To avoid this, first, we remove the endless while loop in > > put_rx_bufdata() which is the root cause of the deadlock. > > Secondly, when there is no room anymore in the tty buffer, we set up a > > timer of 100 msecs to give a chance to the upper layer to flush the tty > > buffer and make room for new data. > > > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > > I agree with the feedback you've been given in that adding a delay > like this is really not a reasonable solution. > > Why is it so difficult to make the event which places the data there > trigger to necessary calls to pull the data out of the URB transfer > buffer? > > This should be totally and completely event based. The function put_rxbuf_data() is called from the urb completion handler. It puts the data of the urb transfer in the tty buffer with tty_insert_flip_string_flags() and schedules a work queue in order to push the data to the ldisc. Problem is that we are in a urb completion handler so we can't wait until there is room in the tty buffer. An option I see is: If tty_insert_flip_string_flags() returns 0, start a workqueue that will insert the remaining data in the tty buffer and then restart the urb. But I'm not convinced that it is a good solution. I should miss something... In put_rxbuf_data(), when tty_insert_flip_string_flags() returns 0, would it be correct to set the TTY_THROTTLED flag? I assume not... I'll have a look in other drivers how such cases are handled. Thanks,
From: Olivier Sobrie ... > The function put_rxbuf_data() is called from the urb completion handler. > It puts the data of the urb transfer in the tty buffer with > tty_insert_flip_string_flags() and schedules a work queue in order to > push the data to the ldisc. > Problem is that we are in a urb completion handler so we can't wait > until there is room in the tty buffer. Surely you can just keep the urb? Resubmit it later when all the data has been transferred. David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 10 Jul 2014 14:37:37 +0000 David Laight <David.Laight@ACULAB.COM> wrote: > From: Olivier Sobrie > ... > > The function put_rxbuf_data() is called from the urb completion handler. > > It puts the data of the urb transfer in the tty buffer with > > tty_insert_flip_string_flags() and schedules a work queue in order to > > push the data to the ldisc. > > Problem is that we are in a urb completion handler so we can't wait > > until there is room in the tty buffer. The tty provides the input queue, if the queue is full then just chuck the data in the bitbucket. hso is trying to be far too clever. If hso is fast enough that the buffering isn't sufficient on the tty side then we need to fix the tty buffer size. Arguably what we need are tty fastpaths for non N_TTY but thats rather more work. There's no fundamental reason that hso can't throw the buffer at buffer straight at the PPP ldisc and straight into the network stack - just that 1. The tty mid layer glue is standing in the way 2. The change of line discipline code has to lock against it Alan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: One Thousand Gnomes > On Thu, 10 Jul 2014 14:37:37 +0000 > David Laight <David.Laight@ACULAB.COM> wrote: > > > From: Olivier Sobrie > > ... > > > The function put_rxbuf_data() is called from the urb completion handler. > > > It puts the data of the urb transfer in the tty buffer with > > > tty_insert_flip_string_flags() and schedules a work queue in order to > > > push the data to the ldisc. > > > Problem is that we are in a urb completion handler so we can't wait > > > until there is room in the tty buffer. > > The tty provides the input queue, if the queue is full then just chuck > the data in the bitbucket. hso is trying to be far too clever. > > If hso is fast enough that the buffering isn't sufficient on the tty side > then we need to fix the tty buffer size. You really want to apply flow control back over the 'serial' link. That may just cause data discards earlier on the local system. But it is possible that not resubmitting the receive urb will cause the modem to flow control back to the sender. In which case there is some chance that data won't be lost. David > > Arguably what we need are tty fastpaths for non N_TTY but thats rather > more work. There's no fundamental reason that hso can't throw the buffer > at buffer straight at the PPP ldisc and straight into the network stack - > just that > > 1. The tty mid layer glue is standing in the way > 2. The change of line discipline code has to lock against it > > Alan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> You really want to apply flow control back over the 'serial' link. > That may just cause data discards earlier on the local system. > But it is possible that not resubmitting the receive urb will cause the > modem to flow control back to the sender. > In which case there is some chance that data won't be lost. If you are doing PPP and you can't keep up the sooner you chuck data the better. Flow control actually works against performance and good network behaviour. It's counter intuitive but TCP/IP works best if any performance bottlenecks are immediately visible and not covered over. Alan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alan and Davids, On Thu, Jul 10, 2014 at 04:50:03PM +0100, One Thousand Gnomes wrote: > On Thu, 10 Jul 2014 14:37:37 +0000 > David Laight <David.Laight@ACULAB.COM> wrote: > > > From: Olivier Sobrie > > ... > > > The function put_rxbuf_data() is called from the urb completion handler. > > > It puts the data of the urb transfer in the tty buffer with > > > tty_insert_flip_string_flags() and schedules a work queue in order to > > > push the data to the ldisc. > > > Problem is that we are in a urb completion handler so we can't wait > > > until there is room in the tty buffer. > > The tty provides the input queue, if the queue is full then just chuck > the data in the bitbucket. hso is trying to be far too clever. > > If hso is fast enough that the buffering isn't sufficient on the tty side > then we need to fix the tty buffer size. Ok I'll adapt the patch to drop the data that can't be put in the tty buffer. I test this and resend a new patch. Thanks for your help!
From: Olivier Sobrie Olivier Sobrie > Hi Alan and Davids, > > On Thu, Jul 10, 2014 at 04:50:03PM +0100, One Thousand Gnomes wrote: > > On Thu, 10 Jul 2014 14:37:37 +0000 > > David Laight <David.Laight@ACULAB.COM> wrote: > > > > > From: Olivier Sobrie > > > ... > > > > The function put_rxbuf_data() is called from the urb completion handler. > > > > It puts the data of the urb transfer in the tty buffer with > > > > tty_insert_flip_string_flags() and schedules a work queue in order to > > > > push the data to the ldisc. > > > > Problem is that we are in a urb completion handler so we can't wait > > > > until there is room in the tty buffer. > > > > The tty provides the input queue, if the queue is full then just chuck > > the data in the bitbucket. hso is trying to be far too clever. > > > > If hso is fast enough that the buffering isn't sufficient on the tty side > > then we need to fix the tty buffer size. > > Ok I'll adapt the patch to drop the data that can't be put in the tty > buffer. I test this and resend a new patch. If you are going to drop data, then ideally you want to discard entire ppp packets. Depending on exactly how the interface works it might be that urb are likely to contain complete packets. So discarding entire urb might work better than discarding a few bytes. (But don't even think of scanning the data stream in the usb driver - except for experiments.) David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 11, 2014 at 09:28:47AM +0000, David Laight wrote: > From: Olivier Sobrie Olivier Sobrie > > Hi Alan and Davids, > > > > On Thu, Jul 10, 2014 at 04:50:03PM +0100, One Thousand Gnomes wrote: > > > On Thu, 10 Jul 2014 14:37:37 +0000 > > > David Laight <David.Laight@ACULAB.COM> wrote: > > > > > > > From: Olivier Sobrie > > > > ... > > > > > The function put_rxbuf_data() is called from the urb completion handler. > > > > > It puts the data of the urb transfer in the tty buffer with > > > > > tty_insert_flip_string_flags() and schedules a work queue in order to > > > > > push the data to the ldisc. > > > > > Problem is that we are in a urb completion handler so we can't wait > > > > > until there is room in the tty buffer. > > > > > > The tty provides the input queue, if the queue is full then just chuck > > > the data in the bitbucket. hso is trying to be far too clever. > > > > > > If hso is fast enough that the buffering isn't sufficient on the tty side > > > then we need to fix the tty buffer size. > > > > Ok I'll adapt the patch to drop the data that can't be put in the tty > > buffer. I test this and resend a new patch. > > If you are going to drop data, then ideally you want to discard entire ppp > packets. Depending on exactly how the interface works it might be that > urb are likely to contain complete packets. Indeed, urbs contains complete ppp packets. I see that the first and last byte are equal to 0x7e. > > So discarding entire urb might work better than discarding a few bytes. > (But don't even think of scanning the data stream in the usb driver - except > for experiments.) I will check with tty_buffer_request_room() that there is enough room to receive the frame before inserting the data in the tty buffer with tty_insert_flip_string().
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 9ca2b41..1dff74f 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -106,6 +106,8 @@ #define MAX_RX_URBS 2 +#define UNTHROTTLE_TIMEOUT 100 /* msecs */ + /*****************************************************************************/ /* Debugging functions */ /*****************************************************************************/ @@ -261,6 +263,7 @@ struct hso_serial { u16 curr_rx_urb_offset; u8 rx_urb_filled[MAX_RX_URBS]; struct tasklet_struct unthrottle_tasklet; + struct timer_list unthrottle_timer; }; struct hso_device { @@ -1161,13 +1164,18 @@ static void put_rxbuf_data_and_resubmit_bulk_urb(struct hso_serial *serial) while (serial->rx_urb_filled[serial->curr_rx_urb_idx]) { curr_urb = serial->rx_urb[serial->curr_rx_urb_idx]; count = put_rxbuf_data(curr_urb, serial); - if (count == -1) - return; if (count == 0) { serial->curr_rx_urb_idx++; if (serial->curr_rx_urb_idx >= serial->num_rx_urbs) serial->curr_rx_urb_idx = 0; hso_resubmit_rx_bulk_urb(serial, curr_urb); + } else if (count > 0) { + mod_timer(&serial->unthrottle_timer, + jiffies + + msecs_to_jiffies(UNTHROTTLE_TIMEOUT)); + break; + } else { + break; } } } @@ -1251,6 +1259,11 @@ static void hso_unthrottle(struct tty_struct *tty) tasklet_hi_schedule(&serial->unthrottle_tasklet); } +static void hso_unthrottle_schedule(unsigned long data) +{ + tasklet_schedule((struct tasklet_struct *)data); +} + /* open the requested serial port */ static int hso_serial_open(struct tty_struct *tty, struct file *filp) { @@ -1286,6 +1299,10 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) tasklet_init(&serial->unthrottle_tasklet, (void (*)(unsigned long))hso_unthrottle_tasklet, (unsigned long)serial); + serial->unthrottle_timer.function = hso_unthrottle_schedule; + serial->unthrottle_timer.data = + (unsigned long)&serial->unthrottle_tasklet; + init_timer(&serial->unthrottle_timer); result = hso_start_serial_device(serial->parent, GFP_KERNEL); if (result) { hso_stop_serial_device(serial->parent); @@ -1333,6 +1350,7 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) tty_port_tty_set(&serial->port, NULL); if (!usb_gone) hso_stop_serial_device(serial->parent); + del_timer_sync(&serial->unthrottle_timer); tasklet_kill(&serial->unthrottle_tasklet); } @@ -2012,22 +2030,23 @@ static int put_rxbuf_data(struct urb *urb, struct hso_serial *serial) tty = tty_port_tty_get(&serial->port); + if (tty && test_bit(TTY_THROTTLED, &tty->flags)) { + tty_kref_put(tty); + return -1; + } + /* Push data to tty */ - write_length_remaining = urb->actual_length - - serial->curr_rx_urb_offset; D1("data to push to tty"); - while (write_length_remaining) { - if (tty && test_bit(TTY_THROTTLED, &tty->flags)) { - tty_kref_put(tty); - return -1; - } - curr_write_len = tty_insert_flip_string(&serial->port, - urb->transfer_buffer + serial->curr_rx_urb_offset, - write_length_remaining); - serial->curr_rx_urb_offset += curr_write_len; - write_length_remaining -= curr_write_len; - tty_flip_buffer_push(&serial->port); - } + write_length_remaining = urb->actual_length - + serial->curr_rx_urb_offset; + curr_write_len = + tty_insert_flip_string(&serial->port, + urb->transfer_buffer + + serial->curr_rx_urb_offset, + write_length_remaining); + serial->curr_rx_urb_offset += curr_write_len; + write_length_remaining -= curr_write_len; + tty_flip_buffer_push(&serial->port); tty_kref_put(tty); if (write_length_remaining == 0) {
When the module sends bursts of data, sometimes a deadlock happens in the hso driver when the tty buffer doesn't get the chance to be flushed quickly enough. To avoid this, first, we remove the endless while loop in put_rx_bufdata() which is the root cause of the deadlock. Secondly, when there is no room anymore in the tty buffer, we set up a timer of 100 msecs to give a chance to the upper layer to flush the tty buffer and make room for new data. Signed-off-by: Olivier Sobrie <olivier@sobrie.be> --- drivers/net/usb/hso.c | 51 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 16 deletions(-)