On Tue, 11 Mar 2003, David Brownell wrote:

> Alan Stern wrote:
> > I agree about the need for synchronization.  The point is that with the
> > only data structure available to the client being a pointer to an urb,
> > there _is_ no way to carry out this synchronization.  Doing so would
> > require using one of the synchronization objects contained within the urb,
> > and they aren't available once the urb has been freed.
> 
> You're overlooking the per-device state, which already needs to
> have synchronization state (for all the different tasks, and
> completion/irq handlers, involved).
> 
> 
> > Of course, there's always the possibility of augmenting the data 
> > structure.  Something containing a pointer to an urb and a spinlock, for 
> > example.  Then you're starting to get close to the proposal I sent you 
> > about a week ago.
> 
> I'd put the spinlock into that per-device state, along with the
> urb handle ... ;)

I'm not sure whether you are referring to the per-device state stored in 
the core or the state stored in the driver, but never mind.  Your comments 
have given me a new idea.

The major purpose of adding the facility to cancel synchronous messages is
to allow drivers to unlink the corresponding urbs during disconnect().  
Okay, so let's keep the existing API pretty much as it is, but add an
additional argument to usb_bulk/control_msg().  This extra argument will
be something like a list_head; the driver can store the structure in its
per-device data.  The idea is that the core maintains on this list all the
outstanding synchronous messages for that device.  Then we just have to
add a function that can be called during disconnect() to cancel all the
messages on the list, say usb_unlink_msgs().  That solves the problem in 
one fell swoop, while adding a minimum of additional complication.

> > Can you spell out more precisely which assumptions you don't like?  And 
> > also which synchronous calls you mean -- my discussion of complete_all() 
> > being called from within a completion routine was meant to refer to an 
> > entirely asynchronous context.
> 
> Now you're mixing up the synchronous and asynchronous scenarios again.

Believe me, I know which is which, even if it doesn't seem that way 
sometimes from my incoherent email.  But I'm starting to feel a little 
like the King of Hearts during the trial in _Alice_: "Synchronous -- 
asynchronous -- asynchronous -- synchronous ---"

(Which reminds me of a completely off-topic question: How do you pronounce 
"isochronous"?)

> Yes and no.  I think we still have too many API calls, not all
> of which play well together.  (The synchronous calls seem to
> trigger the most confusion.)  Adding more calls doesn't make
> things better.

I kind of agree.  This sort of thing is common among systems that grow 
incrementally rather than being planned fully at the start.

> > I considered your model for the scatter-gather I/O functions, where the
> > first call merely initializes the data structures and the urbs aren't
> > actually submitted until the start-wait function is called.  It seemed
> > like unnecessary complication and overhead, at least when handling a
> > single control or bulk message.  There was no reason not to simply start
> > the I/O during the first function call.
> 
> The reason is to get rid of a window in SMP/preempt cases:
> 
>    thread 1:  (a) export urb to thread 2 (b) submit/wait (c)completion
>    thread 2:  (d) cancel the urb
> 
> Sequence (a) (b) (c) is normal.
> For unlink, (a) (b) (d) (c) is normal
> 
> BUT SMP/preempt can produce (a) (d) (b) (c) and that should
> behave sanely ... in this case, by ensuring the submit doesn't happen.

I don't think that is an accurate description of my approach.  Try this 
instead:

    thread 1:  (a) submit request  (b) export urb to thread 2  (c) wait
    thread 2:  (d) cancel the urb

As you said, (a) (b) (c) and (a) (b) (d) (c) are both normal.  But (a) (d)  
(b) (c) and (d) (a) (b) (c) can't happen unless thread 2's actions aren't
properly synchronized with thread 1.  Thread 2 would be trying to unlink
an urb that it doesn't yet have access to.

Or suppose we turn things around, and assume that the export happens 
automatically as soon as the urb is created.  Then we have

    thread 1:  (b) create/export urb  (a) submit request  (c) wait
    thread 2:  (d) cancel the urb

Now we have the possibility of (b) (d) (a) (c), which resembles the
problem you described.  It would again require lack of proper
synchronization, but it certainly could happen.  However, it's not clear
to me that your approach solves this problem either.  Your approach is:

    thread 1:  (b) create/export usb_sg_request  (a) initialize request
                 (c) submit/wait
    thread 2:  (d) cancel the request

What's to prevent (b) (d) (a) (c)?  That's going to be just as bad.

Alan Stern



-------------------------------------------------------
This SF.net email is sponsored by:Crypto Challenge is now open! 
Get cracking and register here for some mind boggling fun and 
the chance of winning an Apple iPod:
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0031en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to