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

> 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

What you mean isn't clear. After all, open sometimes has to call
resume. So how could resume lock against open?

> > 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.

Does this imply that unthrottle should try to autoresume? There does
appear to be a potential race between unthrottle and autosuspend.

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
> > It would probably be cleaner if they could lock against each other
>
> What you mean isn't clear. After all, open sometimes has to call
> resume. So how could resume lock against open?

Probably it needs a counting lock as the code is currently structured -
which is a bit ugly. What paths do we end up going through the device
open method into resume in the same thread ?

> Does this imply that unthrottle should try to autoresume? There does
> appear to be a potential race between unthrottle and autosuspend.

The more I look at it the more it implies to me that the ldiscs doing
this should instead be taught some better manners instead. The real nasty
is that a driver might not have initialised the locking it needs do that
exclusion until open occurs. I think n_tty is probably the only offender
and if so I'd rather fix that and make it a rule that you don't do that,
trying to fix it other ways is going to be more horrible I imagine.
--
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, Alan Cox wrote:

> > > It would probably be cleaner if they could lock against each other
> >
> > What you mean isn't clear. After all, open sometimes has to call
> > resume. So how could resume lock against open?
>
> Probably it needs a counting lock as the code is currently structured -
> which is a bit ugly. What paths do we end up going through the device
> open method into resume in the same thread ?

Currently there are no such paths. I keep forgetting that the resume
is done in serial_install() rather than serial_open(). Eventually the
tty_port reorganization will probably force the resume to move into the
activate method.

However in the option and sierra drivers there is a perverse path from
close to resume: Both their close methods call
usb_autopm_get_interface(). This could be removed without much
trouble; perhaps we should do so.

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 20:52:21 schrieb Alan Stern:
> However in the option and sierra drivers there is a perverse path from
> close to resume: Both their close methods call
> usb_autopm_get_interface(). �This could be removed without much
> trouble; perhaps we should do so.

I am afraid this won't do in the long run. Some drivers will want to
shut down devices by communicating with them in close().

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 Cox on
On Wed, 7 Oct 2009 22:56:20 +0200
Oliver Neukum <oliver(a)neukum.org> wrote:

> Am Mittwoch, 7. Oktober 2009 20:52:21 schrieb Alan Stern:
> > However in the option and sierra drivers there is a perverse path from
> > close to resume: Both their close methods call
> > usb_autopm_get_interface(). �This could be removed without much
> > trouble; perhaps we should do so.
>
> I am afraid this won't do in the long run. Some drivers will want to
> shut down devices by communicating with them in close().

drivers/serial will need a power management hook to use
tty_port_{open/close} so perhaps that can be covered for both. In the
serial case it needs to kick stuff out of PCI D3 mostly and could
probably be fudged but if USB needs it perhaps it should be explicit.

--
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/