On Tue, 11 Oct 2005, Franck wrote:
> > Consider a simple example of reading:
> >
> > spin_lock(&urb->lock);
> > if (urb->status == -EINPROGRESS)
> > printk("URB is in progress\n");
> > else
> > printk("URB is not in progress\n");
> > spni_unlock(urb->lock);
> >
> > Now imagine what would happen if the code didn't acquire the lock. The
> > value could be changed by another thread in between the "if" statement's
> > test and the printk statement. But who cares? The overall effect is
> > exactly the same as if the other thread had changed the value _after_ the
> > printk statement executed.
>
> hmm, this example may be correct not because you're _reading_ the
> value but because printk statement doesn't _use_ urb's data
> structure.
Or because the printk statement doesn't require the URB's internal data
not to change. Yes, but reading (or rather, not writing) is also part of
it. If you write urb->status without holding urb->lock, you run the risk
of the error I described earlier.
Remember that the only data protected by urb->lock is urb->status,
urb->urb_list, and urb->reject. Your HCD should change nothing but the
status.
> > > Another point in my hcd is that an urb can already be transfered by hw
> > > before hcd->urb_enqueue method is called. Actually as soon as this urb
> > > is linked into the corresponding ep's urb list, the hw can send it. Do
> > > you think it can be an issue ?
> >
> > Yes. Your code does not own the ep's list, usbcore does. Your HCD should
> > not even be aware of an URB until urb_enqueue is called. You need to
> > maintain your own lists, using the urb->hcpriv private data structure.
> >
>
> Well, I have my own lists of active ep (actually it's the same
> mechanism as sl811-hcd driver by David Brownell). Each active ep have
> a reference to its associated usbcore ep. It allows to retrieve list
> of URB for that ep.
I suppose that's okay, provided you remember that new entries can be added
to the list before your driver has accepted them.
> > What would happen if your urb_enqueue method had to reject the submission
> > because it couldn't allocate memory for the private data structure? The
> > hardware might already be transferring the rejected URB!
> >
>
> In that case it can't happen. Allocation happens only when scheduling
> an ep the first time. At this time this ep in not in my list of active
> ep so it's not a candidate for transfering URB.
So your urb_enqueue method can never fail, except on the first URB for an
endpoint? In that case, the cause of your bad kref access must have been
something else.
> If you want to take a look to this hcd, I can send it to you. Your
> comments are _very_ welcome.
While I'd like to help, I'm too busy to read through anything as complex
as an HCD.
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
