From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH]usx2y: prevent oops & dead keyboard on hot usb unplugging Date: Fri, 15 Apr 2005 17:50:36 +0200 Message-ID: References: <20050415154544.40500.qmail@web26508.mail.ukl.yahoo.com> Mime-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <20050415154544.40500.qmail@web26508.mail.ukl.yahoo.com> Sender: alsa-devel-admin@lists.sourceforge.net Errors-To: alsa-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , List-Archive: To: karsten wiese Cc: Lee Revell , Clemens Ladisch , Devel Alsa List-Id: alsa-devel@alsa-project.org At Fri, 15 Apr 2005 17:45:44 +0200 (CEST), karsten wiese wrote: > > --- Takashi Iwai schrieb: > > At Fri, 15 Apr 2005 17:14:40 +0200 (CEST), > > karsten wiese wrote: > > > > > > > > > --- Takashi Iwai wrote: > > > > At Fri, 15 Apr 2005 16:28:54 +0200 (CEST), > > > > karsten wiese wrote: > > > > > > > > > > > > > > > --- Takashi Iwai wrote: > > > > > > At Fri, 15 Apr 2005 15:59:25 +0200 (CEST), > > > > > > karsten wiese wrote: > > > > > > > > > > > > > > > > > > > > > --- Lee Revell 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: > > > > > > int snd_card_free_in_thread(snd_card_t * card) > > > { > > > if (card->files == NULL) { > > > snd_card_free(card); > > > return 0; > > > } > > > > > > 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. > > Just found that snd_card_free() also waits for all files > being released: > > int snd_card_free(snd_card_t * card) > { > struct snd_shutdown_f_ops *s_f_ops; > > if (card == NULL) > return -EINVAL; > write_lock(&snd_card_rwlock); > snd_cards[card->number] = NULL; > write_unlock(&snd_card_rwlock); > > #ifdef CONFIG_PM > wake_up(&card->power_sleep); > #ifdef CONFIG_SND_GENERIC_PM > if (card->pm_dev) { > snd_generic_device_unregister(card->pm_dev); > card->pm_dev = NULL; > } > #endif > #endif > > /* wait, until all devices are ready for the free > operation */ > wait_event(card->shutdown_sleep, card->files == NULL); > > > (-: You apply my patch now? No, not enough :) This is exactly why snd_card_free_in_thread() was introduced. As I asked, please make sure that the files are all released before snd_card_free(). Otherwise the disconnect callback hangs at this point because of this check. (IOW, the fact that calling snd_card_free doesn't hang implies that the files are released. But it's better to check this explicitly.) 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