Here is another version of my patch to provide proper synchronous unlinks.
This one uses a struct completion rather than a spinlock, and it also
eliminates all the awkwardness of the "splice" mechanism currently in use.
Alan Stern
= include/linux/usb.h 1.124 vs edited =
--- 1.124/inclu
When David mentioned that unlink_urb tests urb->hcpriv to see whether the
low-level driver has finished processing the urb, I realized that my patch
needed to be fixed. Here's the new version.
Alan Stern
= include/linux/usb.h 1.124 vs edited =
--- 1.124/include/linux/usb.h Wed Jan 22
On Wed, 29 Jan 2003, David Brownell wrote:
> > I don't think I understand your objection. Are you saying that if a
> > driver calls usb_unlink_urb() while the urb's completion handler is
> > running, then back when the handler was originally called the value of
> > urb->status wasn't -ECONNRESET
Hi,
Calls to usb_unlink_urb() while the completion handler is running
won't do anything until the handler has returned.
It still violates the API spec: there's no completion callback with
an unlink status (-ECONNRESET, -ENOENT). It shouldn't masquerade as
a call that behaved correctly (unli
Duncan Sands wrote:
I've still got a bunch of messages in my inbox, maybe one of them
will persuade me that (2) needs solving. I can't yet see why though;
particularly the "... and its handler has exited" part, since that's
easily guaranteed by calling synchronize_kernel().
Excellent remark, b
> I've still got a bunch of messages in my inbox, maybe one of them
> will persuade me that (2) needs solving. I can't yet see why though;
> particularly the "... and its handler has exited" part, since that's
> easily guaranteed by calling synchronize_kernel().
Excellent remark, but only applica
On Tue, 28 Jan 2003, David Brownell wrote:
> Alan Stern wrote:
> >
> > Calls to usb_unlink_urb() while the completion handler is running
> > won't do anything until the handler has returned.
>
> It still violates the API spec: there's no completion callback with
> an unlink status (-ECO
Alan Stern wrote:
The whole idea of this exercise is to simplify
things for device drivers by giving them an easy way to unload cleanly.
Drivers don't need API changes for that. They just need to guarantee
they've implemented disconnect() correctly. Unloading is part of
a different kernel s
Alan Stern wrote:
Just for kicks I created a patch (included below) that allocates those
bits from urb->transfer_flags. That's not such a great place, because
it's liable to be overwritten by device drivers. But the only alternative
is to add a whole extra field (like internal_state).
As soon
Alan Stern wrote:
Calls to usb_unlink_urb() while the completion handler is running
won't do anything until the handler has returned.
It still violates the API spec: there's no completion callback with
an unlink status (-ECONNRESET, -ENOENT). It shouldn't masquerade as
a call that behaved c
Am Montag, 27. Januar 2003 16:41 schrieb Alan Stern:
> Oliver:
>
> In case my message from last Friday fell through the cracks, I'm resending
> it. This patch should do what you want:
Sorry, too much work to do. It's seems alright to me.
Regards
Oliver
Oliver:
In case my message from last Friday fell through the cracks, I'm resending
it. This patch should do what you want:
Calls to usb_unlink_urb() while the completion handler is running
won't do anything until the handler has returned.
Synchronous calls to usb_unlink
On Wed, 22 Jan 2003, Oliver Neukum wrote:
> Am Mittwoch, 22. Januar 2003 22:32 schrieb Alan Stern:
> > Here is my patch to implement proper timing of synchronous unlinks.
> > Basically it just uses a spinlock to cause unlink requests during a
> > completion handler to wait until the handler has re
Am Mittwoch, 22. Januar 2003 22:32 schrieb Alan Stern:
> Here is my patch to implement proper timing of synchronous unlinks.
> Basically it just uses a spinlock to cause unlink requests during a
> completion handler to wait until the handler has returned before carrying
> out the unlink. This seem
Here is my patch to implement proper timing of synchronous unlinks.
Basically it just uses a spinlock to cause unlink requests during a
completion handler to wait until the handler has returned before carrying
out the unlink. This seems like a pretty small and uninvasive sort of
change.
Alan
On Tue, 21 Jan 2003, Oliver Neukum wrote:
>
> > > It would be great if we could eleminate the need for handling unlinking
> > > during completion at all. Is there a reason you cannot make them mutually
> > > exclusive? IMHO it should be possible to do that if you make it a
> > > requirement to ca
Am Dienstag, 21. Januar 2003 23:47 schrieb David Brownell:
> Oliver Neukum wrote:
> >>If automatic resubmission is gone in 2.5.x, why should I code an
> >>usb_unlink_urb() from the completion handler? Do I miss something?
> >
> > We are talking about unlinking while a completion handler is running,
Oliver Neukum wrote:
If automatic resubmission is gone in 2.5.x, why should I code an
usb_unlink_urb() from the completion handler? Do I miss something?
We are talking about unlinking while a completion handler is running,
but not from within the completion handler.
Which does happen to be th
Am Dienstag, 21. Januar 2003 23:10 schrieb Wolfgang Mües:
> Hello, Oliver and Alan,
>
> On Tuesday 21 January 2003 21:41, Oliver Neukum wrote:
> > It would be great if we could eleminate the need for handling unlinking
> > during completion at all. Is there a reason you cannot make them mutually
>
> > It would be great if we could eleminate the need for handling unlinking
> > during completion at all. Is there a reason you cannot make them mutually
> > exclusive? IMHO it should be possible to do that if you make it a
> > requirement to call giveback with handler_lock held.
>
> Sure, that co
Hello, Oliver and Alan,
On Tuesday 21 January 2003 21:41, Oliver Neukum wrote:
> It would be great if we could eleminate the need for handling unlinking
> during completion at all. Is there a reason you cannot make them mutually
> exclusive? IMHO it should be possible to do that if you make it a
On Tue, 21 Jan 2003, Oliver Neukum wrote:
> > But the only alternative
> > is to add a whole extra field (like internal_state).
>
> I believe that you should do that.
I agree.
> It would be great if we could eleminate the need for handling unlinking
> during completion at all. Is there a reason
Am Dienstag, 21. Januar 2003 20:47 schrieb Alan Stern:
> Oliver et al.:
>
> In response to one of your previous messages, I tried reducing the scope
> of the changes required for properly tracking urb states and synchronous
> unlinking. It turns out that in addition to the extra spinlock, I needed
Oliver et al.:
In response to one of your previous messages, I tried reducing the scope
of the changes required for properly tracking urb states and synchronous
unlinking. It turns out that in addition to the extra spinlock, I needed
2 bits: one to indicate when the urb is in use (owned by the co
Alan Stern wrote:
Okay, let's forget about bandwidth reservation for the time being.
Sure, particularly since I didn't have a chance to investigate that
too deeply ... there did seem to be some logic intending to recycle
allocations in uhci-hcd, maybe it's reallly correct. And I suspect
other
On Fri, 17 Jan 2003, Oliver Neukum wrote:
> I've been thinking somewhat especially about David's points of criticism.
> Perhaps I am overdesigning.
It can't be that much of an overdesign. You've only added a state
variable and a spinlock.
> Would a spinlock held during a call to the
> completi
> > How do you like this version?
>
> Aside from the things mentioned above, it looks fine.
I've been thinking somewhat especially about David's points of criticism.
Perhaps I am overdesigning. Would a spinlock held during a call to the
completion handler and while hcd_unlink_urb() holds urb.lock
[ Since Greg KH and Duncan Sands haven't contributed directly to this
discussion, I'm removing their names from the CC: above -- they can read
this via the regular mailing list. ]
On Wed, 15 Jan 2003, Oliver Neukum wrote:
> I take it back, there's a need for a TERMINATING state.
> Your argument
On Thu, 16 Jan 2003, Oliver Neukum wrote:
>
> > But I have a problem with usb_unlink_urb() and resubmitting, as you
> > suggests.
> >
> > First of all, usb_unlink_urb() comes in two flavours, asynchronous and
> > synchronous. If I use asychronous unlink, it may well be the aim of the
> > device d
On Wed, 15 Jan 2003, David Brownell wrote:
> Alan Stern wrote:
>
> > This is where my lack of knowledge of the details underlying the usb core
> > shows through. Okay, looking at hcd.c I see where urb_unlink() is called
> > and how it releases the bandwidth. I'm surprised; is it really supposed
> But I have a problem with usb_unlink_urb() and resubmitting, as you
> suggests.
>
> First of all, usb_unlink_urb() comes in two flavours, asynchronous and
> synchronous. If I use asychronous unlink, it may well be the aim of the
> device driver writer to do a resubmit in the completion function
> Now that I know that giveback_urb cannot be called reentrantly, I see
> there's no need to worry about double locking. That routine is
> the only place that needs to acquire handler_lock while there is a
> possibility of holding another spinlock, so the issue doesn't arise.
>
> If you feel that
Am Mittwoch, 15. Januar 2003 21:54 schrieb Alan Stern:
> On Wed, 15 Jan 2003, Oliver Neukum wrote:
> > Am Mittwoch, 15. Januar 2003 17:04 schrieb Alan Stern:
> > > resubmitted urbs. Second (and more subtle), there are differences
> > > between resubmitting an urb from the handler as opposed to sub
Hello David,
On Wednesday 15 January 2003 00:12, David Brownell wrote:
> > Good. Now eliminate the unsuccessfull synchronous unlinks. Nobody wants
> > them. What should a device driver do if there is an error in
> > usb_unlink_urb()?
>
> Getting back to one of my first responses on this thread.
Hello Oliver,
On Wednesday 15 January 2003 00:46, Oliver Neukum wrote:
[stop resubmitting in the completion function]
> Not good. If the drivers need to implement additional logic, then the
> existing API is insufficient. The point is not that the job can be done
> with the API. It's undoubtedly
Alan Stern wrote:
This is where my lack of knowledge of the details underlying the usb core
shows through. Okay, looking at hcd.c I see where urb_unlink() is called
and how it releases the bandwidth. I'm surprised; is it really supposed
to work this way?
At this point, only "uhci-hcd" relies
On Wed, 15 Jan 2003, Oliver Neukum wrote:
> Am Mittwoch, 15. Januar 2003 17:04 schrieb Alan Stern:
> > resubmitted urbs. Second (and more subtle), there are differences between
> > resubmitting an urb from the handler as opposed to submitting it again
> > after the handler is done. For example,
Am Mittwoch, 15. Januar 2003 17:04 schrieb Alan Stern:
> I agree with Oliver on this. Although the existing API provides enough
> mechanism for drivers to make sure that all their urbs are unlinked and
> all completion handlers are through, it's not easy to do correctly. Since
> practically every
I agree with Oliver on this. Although the existing API provides enough
mechanism for drivers to make sure that all their urbs are unlinked and
all completion handlers are through, it's not easy to do correctly. Since
practically every driver has to deal with this problem during unloading,
the fun
Am Mittwoch, 15. Januar 2003 00:12 schrieb David Brownell:
> Wolfgang Mües wrote:
> > On Tuesday 14 January 2003 22:13, David Brownell wrote:
> >>That is, you want to solve the general "callback into module
> >>being unloaded" problem.
> >
> > I think he want to solve the general "usb_unlink_urb()
Wolfgang Mües wrote:
On Tuesday 14 January 2003 22:13, David Brownell wrote:
That is, you want to solve the general "callback into module
being unloaded" problem.
I think he want to solve the general "usb_unlink_urb() does its job" problem.
The driver may free internal data structures after
On Tuesday 14 January 2003 22:13, David Brownell wrote:
> That is, you want to solve the general "callback into module
> being unloaded" problem.
I think he want to solve the general "usb_unlink_urb() does its job" problem.
The driver may free internal data structures after killing the urb.
> As
Alan Stern wrote:
Furthermore, there is a conceptual problem that has not been fully
addressed. When a usb_unlink_urb() is called for an URB whose completion
handler is running, which incarnation are we being asked to cancel? The
one that just completed or the one that the completion handler
Oliver Neukum wrote:
Am Montag, 13. Januar 2003 00:03 schrieb David Brownell:
Let me explain.
The first problem we avoid is with unloading. We want to be sure that
the completion handler is not running after return from usb_unlink_urb().
If we wish to make the guarantee, usb_unlink_urb() must not
Am Dienstag, 14. Januar 2003 18:29 schrieb Alan Stern:
> On Tue, 14 Jan 2003, Oliver Neukum wrote:
> > Hi,
> >
> > thanks for your suggestion about a spinlock.
> > Here it is again, after a redesign based on your suggestion.
> >
> > I'll start testing this, when I'm back home.
> >
> > Regards
>
On Tue, 14 Jan 2003, Oliver Neukum wrote:
> Hi,
>
> thanks for your suggestion about a spinlock.
> Here it is again, after a redesign based on your suggestion.
>
> I'll start testing this, when I'm back home.
>
> Regards
> Oliver
Your patch looks like a pretty good start to
Hi,
thanks for your suggestion about a spinlock.
Here it is again, after a redesign based on your suggestion.
I'll start testing this, when I'm back home.
Regards
Oliver
You can import this changeset into BK by piping this whole message to:
'| bk receive [path to reposit
On Mon, 13 Jan 2003, Oliver Neukum wrote:
> > I disagree. How about having the core code that calls the completion
> > handler manage the necessary synchronization? Of course, this would have
> > to be coordinated with usb_submit_urb() and usb_unlink_urb().
>
> Yes, this is the other option. I
On Monday 13 January 2003 16:16, Alan Stern wrote:
> As far as guaranteeing that when usb_unlink_urb() returns from a
> synchronous unlink, the completion handler will have already finished --
> so far as I can tell there is currently no such guarantee.
I am a device driver writer.
I need this g
Am Montag, 13. Januar 2003 17:37 schrieb Alan Stern:
> On Mon, 13 Jan 2003, Oliver Neukum wrote:
> > > As far as guaranteeing that when usb_unlink_urb() returns from a
> > > synchronous unlink, the completion handler will have already finished
> > > -- so far as I can tell there is currently no suc
On Mon, 13 Jan 2003, Oliver Neukum wrote:
> > As far as guaranteeing that when usb_unlink_urb() returns from a
> > synchronous unlink, the completion handler will have already finished --
> > so far as I can tell there is currently no such guarantee. At least, the
>
> There's if the unlink doesn
Am Montag, 13. Januar 2003 16:16 schrieb Alan Stern:
> On Mon, 13 Jan 2003, Oliver Neukum wrote:
> > Let me explain.
> > The first problem we avoid is with unloading. We want to be sure that
> > the completion handler is not running after return from usb_unlink_urb().
> > If we wish to make the gua
On Mon, 13 Jan 2003, Oliver Neukum wrote:
> Let me explain.
> The first problem we avoid is with unloading. We want to be sure that
> the completion handler is not running after return from usb_unlink_urb().
> If we wish to make the guarantee, usb_unlink_urb() must not simply
> fail if the complet
Am Montag, 13. Januar 2003 00:03 schrieb David Brownell:
> Oliver Neukum wrote:
> > Am Sonntag, 12. Januar 2003 18:41 schrieb David Brownell:
> >>If there's a CANCELED state it should just be a sub-state
> >>of SUBMITTED ... one where urb->status isn't -EINPROGRESS,
> >>but is -ECONNRESET (async un
Oliver Neukum wrote:
Am Sonntag, 12. Januar 2003 18:41 schrieb David Brownell:
If there's a CANCELED state it should just be a sub-state
of SUBMITTED ... one where urb->status isn't -EINPROGRESS,
but is -ECONNRESET (async unlink), -ENOENT (synchronous
unlink) or -ESHUTDOWN (cleanup after host co
Am Sonntag, 12. Januar 2003 18:41 schrieb David Brownell:
> > As far as I can tell an URB should have four states, with preliminary
> > names:
> >
> > 1 - URB_FREE
>
> Try "IDLE" or something; if it's freed, it's owned by the
> memory management code, and usb would never see it.
OK.
> > 2 - URB_S
As far as I can tell an URB should have four states, with preliminary names:
1 - URB_FREE
Try "IDLE" or something; if it's freed, it's owned by the
memory management code, and usb would never see it.
2 - URB_SUBMITTED
3 - URB_COMPLETING
4 - URB_CANCELED
If there's a CANCELED state it shou
57 matches
Mail list logo
