On Sat, Mar 31, 2007 at 09:26:52AM -0700, David Brownell wrote:
> > There still remains the question of how to do this cleanly.  Obviously 
> > some sort of synchronization primitive is needed.  wait_for_completion() 
> > includes all the necessary memory barriers, whereas wait_event() does not.
> > Or a spinlock could be used, although it would be somewhat awkward.
> 
> When it's the right model, wait_for_completion() should be used.
> When it's the right model, spinlock should be used.
> When it's the right model, RCU should be used.
> When it's the right model, ...
> 
> I don't see spinlocks as being awkward, but the issue they solve is
> not the issue being solved by wait_for_completion(), or wait_event().
> 
> Regardless, some explicit synchronization is the right model to
> be aiming for.  The two drivers mentioned so far (parport, serial)
> are sufficiently low throughput that locking overhead will not be
> an issue ... in fact, even for high throughput drivers it's rarely
> close to an issue, since any sanely designed locking scheme won't
> have enough contention to worry about.

Ok, is _everyone_ forgetting one of the most basics facts about urbs
these days?  They are reference counted for a reason!

The way to properly solve this is to just never care about recycling
urbs.  Look at what the visor driver does as an example of how you can
just create a new urb for _every_ instance, fire it off, and then forget
about it.  It gets used and then automatically cleaned up.

The only thing you need to do is keep track of how many outstanding urbs
are in flight, if you don't want to suck up all of the system's memory
(like the old visor driver could do, just do a simple
        cat /dev/mem > /dev/ttyUSB0
as an example of that...)

And for handling the number of outstanding urbs, use a simple lock and
integer, or an atomic_t (pretty much the same thing...)  If you only
need/want one urb, like for the printer driver, use a completion
callback and set the error flag in the device specific structure if
something bad happens.

So, maybe it's just time to stop pre-allocating these urbs in the probe
call, that would take care of all of these issues and no crazy write
barriers are needed at all.

And before someone complains about doing this kind of allocation for
every single urb, we are already maxing out the hardware by doing
exactly this today, THROUGH USBFS!  And by the time USB 3.0 ever ends
up shipping, our processors will be going even faster, and then possibly
we can create urb pools like skbufs are currently doing if there's
really a need.  But let's not prematurely try to optimize for a problem
that isn't there.

Remember, reference counting is your friend...

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to