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

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

I'm losing track of the original point of this thread. IIRC, the
problem is how the resume method should know whether or not to submit
the receive URB(s). It can't afford to acquire the port mutex because
it might be called by open or close, at which time the mutex is already
held.

Other schemes could work, but to me it seems simplest to rely on a flag
protected by a spinlock. The flag would mean "URBs are supposed to be
queued unless we are suspended". It would be set by open and
unthrottle, and cleared by close and throttle.

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 19:23:59 schrieb Alan Stern:
> > 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.

The race exists. But I don't think unthrottle should autoresume.
It would be better to just set a flag and defer this to resume.
If the device supports remote wakeup there'll be no need to autoresume,
if not, throttle/unthrottle are too rare to justify the complexity.

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: Oliver Neukum on
Am Mittwoch, 7. Oktober 2009 23:34:12 schrieb Alan Stern:
> I'm losing track of the original point of this thread. �IIRC, the
> problem is how the resume method should know whether or not to submit
> the receive URB(s). �It can't afford to acquire the port mutex because
> it might be called by open or close, at which time the mutex is already
> held.
>
> Other schemes could work, but to me it seems simplest to rely on a flag
> protected by a spinlock. �The flag would mean "URBs are supposed to be
> queued unless we are suspended". �It would be set by open and
> unthrottle, and cleared by close and throttle.

1. Why a spinlock?
2. Can we get by with only one flag?

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 Thu, 8 Oct 2009, Oliver Neukum wrote:

> Am Mittwoch, 7. Oktober 2009 23:34:12 schrieb Alan Stern:
> > I'm losing track of the original point of this thread. �IIRC, the
> > problem is how the resume method should know whether or not to submit
> > the receive URB(s). �It can't afford to acquire the port mutex because
> > it might be called by open or close, at which time the mutex is already
> > held.
> >
> > Other schemes could work, but to me it seems simplest to rely on a flag
> > protected by a spinlock. �The flag would mean "URBs are supposed to be
> > queued unless we are suspended". �It would be set by open and
> > unthrottle, and cleared by close and throttle.
>
> 1. Why a spinlock?

Because the amount of work involved seems too small for a mutex. But
you could use a mutex if you wanted, since everything occurs in process
context.

> 2. Can we get by with only one flag?

If all you want to do is answer a single question ("Should URBs be
submitted") then a single flag should be all you need. Why, do you
think more information will be necessary? You can always add more.

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 Donnerstag, 8. Oktober 2009 16:58:39 schrieb Alan Stern:
> On Thu, 8 Oct 2009, Oliver Neukum wrote:
> > Am Mittwoch, 7. Oktober 2009 23:34:12 schrieb Alan Stern:

> > > Other schemes could work, but to me it seems simplest to rely on a flag
> > > protected by a spinlock. �The flag would mean "URBs are supposed to be
> > > queued unless we are suspended". �It would be set by open and
> > > unthrottle, and cleared by close and throttle.
> >
> > 1. Why a spinlock?
>
> Because the amount of work involved seems too small for a mutex. But
> you could use a mutex if you wanted, since everything occurs in process
> context.

We have to submit URBs under that lock.

> > 2. Can we get by with only one flag?
>
> If all you want to do is answer a single question ("Should URBs be
> submitted") then a single flag should be all you need. Why, do you
> think more information will be necessary? You can always add more.

We have at least three reasons URBs should not be submitted.
- closure
- throttling
- suspension
Resume() should not submit if either closure or throttling are active,
neither should unthrottle() resubmit if closure or suspension are active.

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/