lkml.org 
[lkml]   [2019]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RESEND PATCH V3 1/3] USB: serial: f81232: clear overrun flag
From
Date
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

\
 
 \ /
  Last update: 2019-04-01 11:02    [W:0.063 / U:5.616 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site