> From: Georg Acher <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Subject: Re: [linux-usb-devel] Once again about usb-uhci and locks
> Reply-To: [EMAIL PROTECTED]
> Date: Mon, 4 Jun 2001 22:33:18 +0200
> > If problems crop up so thick, surely something is very wrong,
> > so I started to make lists and charts, and found something curious.
> >
> > It seems to me that all places where urb is locked by itself are
> > under the list lock too. The exception is when list lock is
> > illegally dropped and reaquired in process_urb, which we have
> > to remove anyways due to ordering. Nowhere else we use the
> > urb lock by itself.
> >
> > I suggest we get rid of any instances of urb->lock in usb-uhci.
> > It does not serve any useful purpose, and is a terrible bug
> > generator.
>
> Good proposal, but how do you prevent the URB from being deleted by
> unlink_urb when the completion works on this URB and has released the
> list lock? This can of course only happen on SMP, but it is possible.
> This is the sole purpose of the list lock, it wasn't there at the beginning.
The best way, IMHO, would be to prohibit it altogether.
E.g. if URB is submitted (returned from usb_submit_urb
successfuly), then it cannot be unlinked until completition
is called [see note about cancel below].
At present, we kinda allow it, and kinda support it:
in unlink_urb, we look at the urb->status in unlink_urb and
friends: if it is -EINPROGRESS, then do not actually unlink.
For this scheme to work, we retain urb->lock across
a completition callback. Unfortunately, it is simply
not possible to that correctly, for two reasons: 1. lock ordering
violation and deadlock, and 2. impossible to unlock outside (can't
touch after completition, the original oops that David-B saw),
impossible to unlock inside (because other HCs call
unlocked), and impossible to free without unlocking
(works on x86 but blows up on ia64). So, it's broken
and unfixable.
Based on the above, I think that unlinking and completition
cannot work together as long as they abuse urb->status to signal
if the urb is referenced. The only solution is, probably,
to split status and "in use" flag, which would be equivalent
to refcounting of URBs, if you insist on unlinking at any time.
I have no idea if any drivers try to unlink submitted URBs.
We have no API to "cancel" a URB, so perhaps a driver writer
somewhere thought that he can emulate a cancel by unlinking.
In that case we cannot prohibit unlinking-while-live and
refcounting or equivalent is the only right fix.
> > P.S. I looked briefly how Johannes does it, because I know that
> > he uses per-urb lock. He does a more fine grained locking,
> > e.g. everything is under urb lock, but sometimes picks list
> > lock (N.B.: opposite lock order). [...]
>
> Your opinion. Have you checked that there are no races with unlink_urb and
> transfer_result in uhci?
No, I haven't, and uhci is not immune to the problem that we are
discussing.
I went with usb-uhci because of a multitude of teething
problems with uhci, but generally uhci looks like a sane structure.
-- Pete
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
