Date: Wed, 8 Sep 2004 13:45:06 -0400 From: Brian Fundakowski Feldman <green@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/usb ugen.c Message-ID: <20040908174506.GE928@green.homeunix.org> In-Reply-To: <20040908.112100.102577181.imp@bsdimp.com> References: <20040908130741.GC928@green.homeunix.org> <20040908.083215.102654981.imp@bsdimp.com> <20040908152956.GD928@green.homeunix.org> <20040908.112100.102577181.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 08, 2004 at 11:21:00AM -0600, M. Warner Losh wrote: > In message: <20040908152956.GD928@green.homeunix.org> > Brian Fundakowski Feldman <green@freebsd.org> writes: > : On Wed, Sep 08, 2004 at 08:32:15AM -0600, M. Warner Losh wrote: > : > In message: <20040908130741.GC928@green.homeunix.org> > : > Brian Fundakowski Feldman <green@FreeBSD.ORG> writes: > : > : On Wed, Sep 08, 2004 at 01:20:31AM -0600, M. Warner Losh wrote: > : > : > In message: <200409080713.i887Dd54058789@repoman.freebsd.org> > : > : > Warner Losh <imp@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@FreeBSD.org \ The Power to Serve! \ Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040908174506.GE928>