Re: [PATCH]usx2y: prevent oops & dead keyboard on hot usb unplugging

[Takashi Iwai <tiwai@suse.de>]

At Fri, 15 Apr 2005 17:14:40 +0200 (CEST),
karsten wiese wrote:
> 
> 
> --- Takashi Iwai <tiwai@suse.de> wrote:
> > At Fri, 15 Apr 2005 16:28:54 +0200 (CEST),
> > karsten wiese wrote:
> > > 
> > > 
> > > --- Takashi Iwai <tiwai@suse.de> wrote:
> > > > At Fri, 15 Apr 2005 15:59:25 +0200 (CEST),
> > > > karsten wiese wrote:
> > > > > 
> > > > > 
> > > > > --- Lee Revell <rlrevell@joe-job.com> wrote:
> > > > > > On Thu, 2005-04-14 at 20:59 +0200, karsten wiese
> > > > wrote:
> > > > > > > Hi Clemens & Takashi
> > > > > > > 
> > > > > > > Please commit the patch.
> > > > > > 
> > > > > > Is the ALSA in kernel 2.6.11 affected?  If so,
> > this
> > > > > 
> > > > > Think so, yes. The keyboard dead effect + described
> > > > oops
> > > > > has been around for quiet a long time.
> > > > > (There is a lkml mail from yesterday about an
> > > > usb-cellphone
> > > > > setup,
> > > > > where the same oops + dead keyboard happens also:
> > > > >
> > > >
> > >
> >
> http://marc.theaimsgroup.com/?i=<20050414230621.49663f75.vsu%20()%20altlinux%20!%20ru>
> > > > > )
> > > > > There is also an alsa-bug with snd-usb-audio in the
> > > > > bugbase.
> > > > > Clemens seams not to be convinced about this
> > aproach
> > > > yet.
> > > > > maybe 'cause the comment of snd_card_free_in_thread
> > > > > explicitly states that its the one to be called for
> > > > > hotplug:
> > > > > /**
> > > > >  *  snd_card_free_in_thread - call snd_card_free()
> > in
> > > > > thread
> > > > >  *  @card: soundcard structure
> > > > >  *
> > > > >  *  This function schedules the call of
> > snd_card_free()
> > > > > function in a
> > > > >  *  work queue.  When all devices are released
> > > > (non-busy),
> > > > > the work
> > > > >  *  is woken up and calls snd_card_free().
> > > > >  *
> > > > >  *  When a card can be disconnected at any time by
> > > > hotplug
> > > > > service,
> > > > >  *  this function should be used in disconnect (or
> > > > detach)
> > > > > callback
> > > > >  *  instead of calling snd_card_free() directly.
> > > > >  *  
> > > > >  *  Returns - zero otherwise a negative error code
> > if
> > > > the
> > > > > start of thread failed.
> > > > >  */
> > > > > 
> > > > > Also I remember , that snd_card_free didn't work ok
> > > > when i
> > > > > used it in the first implementations of
> > usx2y-module
> > > > some 2
> > > > > years ago.
> > > > > but that was on kernel 2.4.something.......
> > > > > maybe above comment is from kernel 2.4 also and the
> > > > hotplug
> > > > > system has changed ? anybody?
> > > > 
> > > > Might be.  To be sure, check whether the release fops
> > is
> > > > called
> > > > properly before you calling snd_card_free().
> > > 
> > > You mean snd_card_disconnect()?
> > > That's called with the same argument and before
> > > snd_card_free().
> > 
> > No, I mean snd_card_free(), which you changed from
> > snd_card_free_in_thread().  Someone has to release the
> > files
> > (i.e. call release fops eventually) before clean up
> > everything via
> > snd_card_free().
> > 
> Erm, you mean the close (for
> snd_pcm_ops_t,snd_rawmidi_ops_t) and release (for
> snd_hwdep_ops_t) callbacks then?

I meant the "file operations".  Anyway, the callback of PCM or rawmidi
will be called from the release callback of fops, though...

>  They should all be called
> before snd_card_free()?

Yes.

> in other words:
> <snip>
> int snd_card_free_in_thread(snd_card_t * card)
> {
> 	if (card->files == NULL) {
> 		snd_card_free(card);
> 		return 0;
> 	}
> </snip>
> the "(card->files == NULL)" MUST be true before
> snd_card_free() can be called safely?

Yes, exactly.
Otherwise, snd_card_free() removes the device entry, and the remainig
files will have no way to release themselves after that.


Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click