From: Oliver Neukum on
Am Dienstag, 6. Oktober 2009 17:06:46 schrieb Alan Cox:
> +++ b/drivers/usb/serial/opticon.c
> @@ -498,12 +498,12 @@ static int opticon_resume(struct usb_interface *intf)
>         struct usb_serial_port *port = serial->port[0];
>         int result;
>  
> -       mutex_lock(&port->mutex);
> +       mutex_lock(&port->port.mutex);
>         if (port->port.count)
>                 result = usb_submit_urb(priv->bulk_read_urb, GFP_NOIO);
>         else
>                 result = 0;
> -       mutex_unlock(&port->mutex);
> +       mutex_unlock(&port->port.mutex);
>         return result;
>  }

We will need some generic way to autoresume from open.
Resume will need to lock against open() and need to be called
from within open(). Any ideas for an unugly interface?

Regards
Oliver

From: Alan Stern on
On Wed, 7 Oct 2009, Oliver Neukum wrote:

> We will need some generic way to autoresume from open.
> Resume will need to lock against open() and need to be called
> from within open(). Any ideas for an unugly interface?

It's not quite that bad. Resume doesn't need to lock against open.
If open is called while resume is running then when it tries to do its
own resume, it will either block (waiting for the pm_mutex) or return
immediately (if it sees the device is already resumed).

A more interesting question is how to synchronize both open/close and
suspend/resume against throttle/unthrottle.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oliver Neukum on
Am Mittwoch, 7. Oktober 2009 18:03:08 schrieb Alan Stern:
> On Wed, 7 Oct 2009, Oliver Neukum wrote:
> > We will need some generic way to autoresume from open.
> > Resume will need to lock against open() and need to be called
> > from within open(). Any ideas for an unugly interface?
>
> It's not quite that bad. �Resume doesn't need to lock against open.
> If open is called while resume is running then when it tries to do its
> own resume, it will either block (waiting for the pm_mutex) or return
> immediately (if it sees the device is already resumed).

But resume() needs to know whether the read URBs need to be
submitted or not.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Wed, 7 Oct 2009, Oliver Neukum wrote:

> Am Mittwoch, 7. Oktober 2009 18:03:08 schrieb Alan Stern:
> > On Wed, 7 Oct 2009, Oliver Neukum wrote:
> > > We will need some generic way to autoresume from open.
> > > Resume will need to lock against open() and need to be called
> > > from within open(). Any ideas for an unugly interface?
> >
> > It's not quite that bad. �Resume doesn't need to lock against open.
> > If open is called while resume is running then when it tries to do its
> > own resume, it will either block (waiting for the pm_mutex) or return
> > immediately (if it sees the device is already resumed).
>
> But resume() needs to know whether the read URBs need to be
> submitted or not.

Given that there are several pathways for turning on or turning off the
read URBs, the best answer is to use a flag.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
On Wed, 7 Oct 2009 12:03:08 -0400 (EDT)
Alan Stern <stern(a)rowland.harvard.edu> wrote:

> On Wed, 7 Oct 2009, Oliver Neukum wrote:
>
> > We will need some generic way to autoresume from open.
> > Resume will need to lock against open() and need to be called
> > from within open(). Any ideas for an unugly interface?
>
> It's not quite that bad. Resume doesn't need to lock against open.
> If open is called while resume is running then when it tries to do its
> own resume, it will either block (waiting for the pm_mutex) or return
> immediately (if it sees the device is already resumed).

It would probably be cleaner if they could lock against each other

> A more interesting question is how to synchronize both open/close and
> suspend/resume against throttle/unthrottle.

throttle and unthrottle can sleep and obviously have to as they do a fair
bit of work sometimes (xon/xoff, mode line waggling etc)

The current ordering here is quite ugly because we open the ldisc before
the tty which means the ldisc sometimes calls unthrottle before the tty
is opened which is not nice. On the close side we have the same thing via
tty_ldisc_release.

We can take the port->mutex lock in the throttle/unthrottle methods as
far as I can see - there are no obvious problem cases. We do call
->throttle and ->unthrottle from the ldisc open but this occurs outside
of any call to the tty driver open/close method so I don't see any
deadlock.

It adds an ordering of termios lock before port mutex when taking both
but that's not a problem and really implicit in the structure of the code
anyway.

Alan


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/