2014-07-14 10:08:57

by Olivier Sobrie

[permalink] [raw]
Subject: [PATCH v2 1/2] hso: remove unused workqueue

The workqueue "retry_unthrottle_workqueue" is not scheduled anywhere
in the code. So, remove it.

Signed-off-by: Olivier Sobrie <[email protected]>
---
drivers/net/usb/hso.c | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index a3a0586..9ca2b41 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -261,7 +261,6 @@ struct hso_serial {
u16 curr_rx_urb_offset;
u8 rx_urb_filled[MAX_RX_URBS];
struct tasklet_struct unthrottle_tasklet;
- struct work_struct retry_unthrottle_workqueue;
};

struct hso_device {
@@ -1252,14 +1251,6 @@ static void hso_unthrottle(struct tty_struct *tty)
tasklet_hi_schedule(&serial->unthrottle_tasklet);
}

-static void hso_unthrottle_workfunc(struct work_struct *work)
-{
- struct hso_serial *serial =
- container_of(work, struct hso_serial,
- retry_unthrottle_workqueue);
- hso_unthrottle_tasklet(serial);
-}
-
/* open the requested serial port */
static int hso_serial_open(struct tty_struct *tty, struct file *filp)
{
@@ -1295,8 +1286,6 @@ 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);
- INIT_WORK(&serial->retry_unthrottle_workqueue,
- hso_unthrottle_workfunc);
result = hso_start_serial_device(serial->parent, GFP_KERNEL);
if (result) {
hso_stop_serial_device(serial->parent);
@@ -1345,7 +1334,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
if (!usb_gone)
hso_stop_serial_device(serial->parent);
tasklet_kill(&serial->unthrottle_tasklet);
- cancel_work_sync(&serial->retry_unthrottle_workqueue);
}

if (!usb_gone)
--
1.7.9.5


2014-07-14 10:09:20

by Olivier Sobrie

[permalink] [raw]
Subject: [PATCH v2 2/2] hso: fix deadlock when receiving bursts of data

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.

Remove the endless while loop in function put_rxbuf_data() which is
called by the urb completion handler.
If there isn't enough room in the tty buffer, discards all the data
received in the URB.

Cc: David Miller <[email protected]>
Cc: David Laight <[email protected]>
Cc: One Thousand Gnomes <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Jan Dumon <[email protected]>
Signed-off-by: Olivier Sobrie <[email protected]>
---
Changes in v2:
- remove the unthrottle timer added in the previous patch version
- drop entire rx urb data if there is not enough space in tty buffer
- remove variable curr_rx_urb_offset from hso_serial structure

drivers/net/usb/hso.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 9ca2b41..a4272ed 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -258,7 +258,6 @@ struct hso_serial {
* so as not to drop characters on the floor.
*/
int curr_rx_urb_idx;
- u16 curr_rx_urb_offset;
u8 rx_urb_filled[MAX_RX_URBS];
struct tasklet_struct unthrottle_tasklet;
};
@@ -2001,8 +2000,7 @@ static void ctrl_callback(struct urb *urb)
static int put_rxbuf_data(struct urb *urb, struct hso_serial *serial)
{
struct tty_struct *tty;
- int write_length_remaining = 0;
- int curr_write_len;
+ int count;

/* Sanity check */
if (urb == NULL || serial == NULL) {
@@ -2012,29 +2010,28 @@ 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;
+ count = tty_buffer_request_room(&serial->port, urb->actual_length);
+ if (count >= urb->actual_length) {
+ tty_insert_flip_string(&serial->port, urb->transfer_buffer,
+ urb->actual_length);
tty_flip_buffer_push(&serial->port);
+ } else {
+ dev_warn(&serial->parent->usb->dev,
+ "dropping data, %d bytes lost\n", urb->actual_length);
}
+
tty_kref_put(tty);

- if (write_length_remaining == 0) {
- serial->curr_rx_urb_offset = 0;
- serial->rx_urb_filled[hso_urb_to_index(serial, urb)] = 0;
- }
- return write_length_remaining;
+ serial->rx_urb_filled[hso_urb_to_index(serial, urb)] = 0;
+
+ return 0;
}


@@ -2205,7 +2202,6 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
}
}
serial->curr_rx_urb_idx = 0;
- serial->curr_rx_urb_offset = 0;

if (serial->tx_urb)
usb_kill_urb(serial->tx_urb);
--
1.7.9.5

2014-07-14 13:28:03

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] hso: fix deadlock when receiving bursts of data

On Mon, 14 Jul 2014 12:08:50 +0200
Olivier Sobrie <[email protected]> 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.
>
> Remove the endless while loop in function put_rxbuf_data() which is
> called by the urb completion handler.
> If there isn't enough room in the tty buffer, discards all the data
> received in the URB.
>
> Cc: David Miller <[email protected]>
> Cc: David Laight <[email protected]>
> Cc: One Thousand Gnomes <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Jan Dumon <[email protected]>
> Signed-off-by: Olivier Sobrie <[email protected]>

Acked-by: Alan Cox <[email protected]>

2014-07-15 02:27:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] hso: remove unused workqueue

From: Olivier Sobrie <[email protected]>
Date: Mon, 14 Jul 2014 12:08:49 +0200

> The workqueue "retry_unthrottle_workqueue" is not scheduled anywhere
> in the code. So, remove it.
>
> Signed-off-by: Olivier Sobrie <[email protected]>

Applied.

2014-07-15 02:28:04

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] hso: fix deadlock when receiving bursts of data

From: Olivier Sobrie <[email protected]>
Date: Mon, 14 Jul 2014 12:08:50 +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.
>
> Remove the endless while loop in function put_rxbuf_data() which is
> called by the urb completion handler.
> If there isn't enough room in the tty buffer, discards all the data
> received in the URB.
>
> Cc: David Miller <[email protected]>
> Cc: David Laight <[email protected]>
> Cc: One Thousand Gnomes <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Jan Dumon <[email protected]>
> Signed-off-by: Olivier Sobrie <[email protected]>
> ---
> Changes in v2:
> - remove the unthrottle timer added in the previous patch version
> - drop entire rx urb data if there is not enough space in tty buffer
> - remove variable curr_rx_urb_offset from hso_serial structure

Applied, thanks.