cvs commit: src/sys/dev/usb ugen.c

[Brian Fundakowski Feldman]

On Wed, Sep 08, 2004 at 11:21:00AM -0600, M. Warner Losh wrote:
> In message: <20040908152956.GD928 at green.homeunix.org>
>             Brian Fundakowski Feldman <green at freebsd.org> writes:
> : On Wed, Sep 08, 2004 at 08:32:15AM -0600, M. Warner Losh wrote:
> : > In message: <20040908130741.GC928 at green.homeunix.org>
> : >             Brian Fundakowski Feldman <green at FreeBSD.ORG> writes:
> : > : On Wed, Sep 08, 2004 at 01:20:31AM -0600, M. Warner Losh wrote:
> : > : > In message: <200409080713.i887Dd54058789 at repoman.freebsd.org>
> : > : >             Warner Losh <imp at FreeBSD.org> writes:
> : > : > : imp         2004-09-08 07:13:39 UTC
> : > : > : 
> : > : > :   FreeBSD src repository
> : > : > : 
> : > : > :   Modified files:
> : > : > :     sys/dev/usb          ugen.c 
> : > : > :   Log:
> : > : > :   Back out 1.88.
> : > : >              1.87 I mean.
> : > : > :   The reference counts are there to block detach until the sleepers in
> : > : > :   read/write/ioctl have gotten out, not to prevent the open device from
> : > : > :   going away.  Restore the old behavior so that we have a chance to wake
> : > : > :   up sleepers when the usb device goes away, so they can properly return
> : > : > :   EIO back to the caller when this happens.
> : > : > :   
> : > : > :   Otherwise, we have a guarnateed panic waiting to happen when a device
> : > : > :   detaches with an active read channel.
> : > : > :   
> : > : > :   This should be merged to 5 asap.
> : > : 
> : > : Now I'll get guaranteed panics using both my Palm and any CardBus hardware.
> : > 
> : > Can you send me a traceback for that problem then?  The 'reference
> : > count' here is very much supposed to be a 'how many sleepers do you
> : > have' sort of thing and is present in nearly all of the usb drivers.
> : 
> : No, I'd prefer not to have to crash my machine more.  It already does that
> : (or hangs, or whatever) in -CURRENT often enough.
> : 
> : (Holding breath for open sourced Solaris.)
> 
> Brian,
> 
> 	Since I backed out your fix, that puts me on the hook to fix
> the problem that you made the change for.  If you provide me with some
> information about how to reproduce it, I'll be happy to look into the
> problem.  The above is only enough to let me guess at what might be
> going on.
> 
> I'm sorry I committed without talking to you first, but the above
> change cost myself and my boss a couple of days of debugging time on
> one of our products that keeps ugen open to monitor it until it goes
> away.  I was understandably grumpy at having my time wasted like this
> when I discovered why things broke :-(.

Why would you get a panic at detach if the refcount is held for every
open channel?  Whether or not a read is in progress, the reference
held for the entirety of the channel being open should help.  I don't
see how you would _not_ get the EIO in any place you would have gotten
it before.  If you destroy the device before the device is closed
then subsequent d_close() calls are going to crash when the freed
softc is referenced.

If I had to name every bug or misfeature I had to spend countless hours
trying to find and fix, I would be sitting here unhappy for a very long
time, too.

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green at FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\