On Thu, Oct 24, 2002 at 09:34:02AM +0200, Oliver Neukum wrote:
Think of it as a proof-by-induction that shutdown will work. When
there's a point past which no more activities can be started (like
when device state becomes GONE), and existing activities all get
canceled, shutdown will
Hi Alan,
We seem to agree.
It seems clear that the best way for the core to handle this situation is:
Keep track of all active URBs on a per-device list.
Being done today for HCDs using the new hc_driver framework.
When a device unplugs from the bus, set the device state
to GONE and
Am Mittwoch, 23. Oktober 2002 00:34 schrieb David Brownell:
IMHO usb_stor_clear_halt and usb_stor_reset_common should take
dev_semaphore.
What's the race?
False alarm, it seems like. It's taken higher up.
Regards
Oliver
On Thu, 24 Oct 2002, Oliver Neukum wrote:
Hi,
The point I was raising is that once khubd starts its cleanup, only
one activity makes sense: unlinking urbs. But today, it's _also_
Correct, so we want the core to do that, don't we?
possible to continue submitting urbs. That can create
On Thu, Oct 24, 2002 at 10:14:48AM +0200, Oliver Neukum wrote:
Am Donnerstag, 24. Oktober 2002 09:48 schrieb Greg KH:
No, the last state will happen when the last reference count of the
device is removed. So as long as someone (driver, core, who ever) has
called usb_get_dev(), the device
On Wed, 23 Oct 2002, Oliver Neukum wrote:
Am Mittwoch, 23. Oktober 2002 01:25 schrieb David Brownell:
IMHO usb_stor_clear_halt and usb_stor_reset_common should take
dev_semaphore.
What's the race?
usb_stor_clear_halt uses a synchronous control message.
It will not be unlinked
Am Mittwoch, 23. Oktober 2002 15:41 schrieb Alan Stern:
Adding a semaphore is definitely the wrong solution.
The problem of immediately cancelling all URBs when a disconnect occurs
exists throughout the usb-storage driver, not just in usb_stor_clear_halt
and usb_stor_reset_common. It needs
On Wed, 23 Oct 2002, Oliver Neukum wrote:
Am Mittwoch, 23. Oktober 2002 15:41 schrieb Alan Stern:
The problem of immediately cancelling all URBs when a disconnect occurs
exists throughout the usb-storage driver, not just in usb_stor_clear_halt
and usb_stor_reset_common. It needs to be
I'm going to guess that you will object to the race condition that exists
between the time the flag is tested and the URB is submitted. Yes,
there's a race. But it is unavoidable, unless you want to surround every
call to the usb core with a spinlock_irq or the equivalent. And in any
On Wed, Oct 23, 2002 at 05:34:53PM +0200, Oliver Neukum wrote:
I'm going to guess that you will object to the race condition that exists
between the time the flag is tested and the URB is submitted. Yes,
there's a race. But it is unavoidable, unless you want to surround every
call to
On Wed, Oct 23, 2002 at 01:28:00PM -0400, Alan Stern wrote:
Actually, I was sort of hoping to drag Matt Dharm into this discussion.
He certainly understands the issues involved and the storage driver,
probably better than anyone.
And some days, I wish I didn't
BTW, in case anyone is
Of course, drivers don't _need_ usbcore to protect them from this
race, they can handle it themselves.
But here's a case where I think it'd be wrong to use a semaphore.
Some sort of shared lock for routine use, with unbind driver from
interface processing always getting it in exclusive mode, is
Here's a suggestion that would help all existing usb drivers. Add an
additional parameter to usb_control_msg (and the other synchronous message
subroutines as well) that takes a struct urb **. The subroutines can use
it to store a pointer to the dynamically allocated URB, so that the driver
Yes it does. But completion routines get called at interrupt time.
However, this is not really relevant to the problem at hand.
For these cases we need a spinlock, yes.
case it doesn't matter, since we only want to guarantee that no URB
will be submitted after disconnect() _returns_.
Perhaps the real question is: Do we need to guarantee that no URBs will
remain outstanding after disconnect() returns? This seems to be what you
are getting at. Unfortunately, I haven't seen written down anywhere
exactly what conditions the driver needs to satisfy. It would be nice if
this
This could also be entirely solved by making the requirements that:
(1) The core or HCD will unlink all URBs for a removed device when
it's removed
I've wanted to see this for some time. It's now easy enough to do
for anything using the hcd.c code, since it has a list of those urbs.
In fact
On Wed, 23 Oct 2002, David Brownell wrote:
What I'd thought about was instead to just provide the URB to unlink
if the timeout failed, device was disconnected, or whatever. Why ask
for some new api notion? In fact I think I have code sitting somewhere
around here that does exactly that,
dev-some_urb = ... alloc+save, locked against disconnect ...
sync_control_message (urb, bRequest, bRequestType, ...,
INTERRUPTIBLE or maybe not, TIMEOUT or zero)
switch (urb-status) {
...
}
free dev-some_urb
and the disconnect code could just unlink all active/submitted urbs.
Is this
Ah, but re-submission of an interrupt URB from within the completion
handler will run in interrupt context.
That we'll have to deal with later.
This could also be entirely solved by making the requirements that:
(1) The core or HCD will unlink all URBs for a removed device when
(1) The core or HCD will unlink all URBs for a removed device when
it's removed
To do so you need to add the urb to a per device list, which has to be
locked. The problems arise if you loose the race. Where do you put the
lock ?
The hcd.c code which maintains that list is protected by
What I'd thought about was instead to just provide the URB to unlink
if the timeout failed, device was disconnected, or whatever. Why ask
for some new api notion? In fact I think I have code sitting somewhere
around here that does exactly that, maybe I should dig it up...
That means
Am Mittwoch, 23. Oktober 2002 20:47 schrieb David Brownell:
This could also be entirely solved by making the requirements that:
(1) The core or HCD will unlink all URBs for a removed device when
it's removed
I've wanted to see this for some time. It's now easy enough to do
for
Hi,
hcd_data_lock is taken too late.
static int hcd_submit_urb (struct urb *urb, int mem_flags)
{
int status;
struct usb_hcd *hcd = urb-dev-bus-hcpriv;
You've just followed a pointer that you haven't validated.
If the driver has improper locking,
Am Mittwoch, 23. Oktober 2002 23:29 schrieb David Brownell:
hcd_data_lock is taken too late.
That'll go away in the final core patch to remove automagic
resubmit for interrupt ... which will no longer zero urb-dev.
That's good news, but where do you want to take it?
I don't see any
On Wed, Oct 23, 2002 at 11:47:43AM -0700, David Brownell wrote:
If we could set usb_device-dev.state to DEVICE_GONE before we start
to disconect(), we could do even better: we could guarantee that any
new urb submissions would fail, so even a driver _trying_ to do the
wrong thing wouldn't
On Wed, Oct 23, 2002 at 10:23:02PM +0200, Oliver Neukum wrote:
This could also be entirely solved by making the requirements that:
(1) The core or HCD will unlink all URBs for a removed device when
it's removed
To do so you need to add the urb to a per device list, which has to be
On Wed, Oct 23, 2002 at 01:28:00PM -0400, Alan Stern wrote:
That shouldn't pose a problem. Greg KH can correct me if I'm wrong, but
the freeze means only that no new features are to be added. Existing ones
can still be fixed or removed.
That is true. It's also true that the freeze is for
Am Donnerstag, 24. Oktober 2002 00:45 schrieb Matthew Dharm:
On Wed, Oct 23, 2002 at 10:23:02PM +0200, Oliver Neukum wrote:
This could also be entirely solved by making the requirements that:
(1) The core or HCD will unlink all URBs for a removed device when
it's removed
To do so
I wonder if he'd comment on what it'd mean to call device_unregister()
earlier in usb_disconnect() ... like right at the top, not the last
thing. The docs imply that'd be OK.
Which device_unregister? For the interfaces or the device?
Well the whole device is DEVICE_GONE, including
On Wed, Oct 23, 2002 at 05:32:50PM -0700, David Brownell wrote:
I wonder if he'd comment on what it'd mean to call device_unregister()
earlier in usb_disconnect() ... like right at the top, not the last
thing. The docs imply that'd be OK.
Which device_unregister? For the interfaces or
Certainly the device itself, since
that's what currently drives the submit logic ... so that's what
would need to know that it's GONE, to let the submit logic reject
new urb submissions.
Yes, usb_submit_urb() can just check the device state. But what's to
keep the device from being removed
Am Mittwoch, 23. Oktober 2002 00:34 schrieb David Brownell:
IMHO usb_stor_clear_halt and usb_stor_reset_common should take
dev_semaphore.
What's the race?
usb_stor_clear_halt uses a synchronous control message.
It will not be unlinked upon disconnect. Disconnect must not
return until the
Am Mittwoch, 23. Oktober 2002 01:25 schrieb David Brownell:
IMHO usb_stor_clear_halt and usb_stor_reset_common should take
dev_semaphore.
What's the race?
usb_stor_clear_halt uses a synchronous control message.
It will not be unlinked upon disconnect. Disconnect must not
return until
33 matches
Mail list logo
