Oliver Neukum 於 2019/4/1 下午 04:34 寫道:
On Mo, 2019-04-01 at 14:00 +0800, Ji-Ze Hong (Peter Hong) wrote:
Hi,
I am afraid there is a race condiion in this code.
@@ -315,6 +318,7 @@ static void f81232_process_read_urb(struct urb *urb)
if (lsr & UART_LSR_OE) {
port->icount.overrun++;
+ schedule_work(&priv->lsr_work);
Unconditionally scheduled
tty_insert_flip_char(&port->port, 0,
TTY_OVERRUN);
}
[..]
+static int f81232_suspend(struct usb_serial *serial, pm_message_t message)
+{
+ struct f81232_private *port_priv;
+
+ port_priv = usb_get_serial_port_data(serial->port[0]);
+ flush_work(&port_priv->lsr_work);
+
+ return 0;
+}
+
static struct usb_serial_driver f81232_device = {
.driver = {
.owner = THIS_MODULE,
@@ -655,6 +688,7 @@ static struct usb_serial_driver f81232_device = {
.read_int_callback = f81232_read_int_callback,
.port_probe = f81232_port_probe,
.port_remove = f81232_port_remove,
+ .suspend = f81232_suspend,
Please have a look at:
int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
{
struct usb_serial *serial = usb_get_intfdata(intf);
int i, r = 0;
serial->suspending = 1;
/*
* serial->type->suspend() MUST return 0 in system sleep context,
* otherwise, the resume callback has to recover device from
* previous suspend failure.
*/
if (serial->type->suspend) {
r = serial->type->suspend(serial, message);
if (r < 0) {
serial->suspending = 0;
goto err_out;
}
}
for (i = 0; i < serial->num_ports; ++i)
usb_serial_port_poison_urbs(serial->port[i]);
err_out:
return r;
}
EXPORT_SYMBOL(usb_serial_suspend);
As you can see, the suspend method is called first and then the URBs
are poisoned. That means that after you have flushed the work, it may
be submitted again. The fix would be to test the 'suspending' flag
before you schedule work (and recheck the need to schedule it
during resume)
Thanks for report the race condition issue. It's seems the same bug
in f81534.c. I'll try to fix it on f81232.c then fix f81534.c too.
--
With Best Regards,
Peter Hong
