From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH]snd-usb-usx2y 0.8.1 against alsa-kernel cvshead: OHCI doesn't crash anymore Date: Tue, 19 Oct 2004 15:33:44 +0200 Sender: alsa-devel-admin@lists.sourceforge.net Message-ID: References: <200409302046.29798.annabellesgarden@yahoo.de> <200410030038.04347.annabellesgarden@yahoo.de> <200410181406.43903.annabellesgarden@yahoo.de> Mime-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <200410181406.43903.annabellesgarden@yahoo.de> Errors-To: alsa-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , List-Archive: To: Karsten Wiese Cc: alsa-devel@lists.sourceforge.net, Rui Nuno Capela List-Id: alsa-devel@alsa-project.org At Mon, 18 Oct 2004 14:06:43 +0200, Karsten Wiese wrote: > > [1 ] > Am Montag 04 Oktober 2004 18:22 schrieb Takashi Iwai: > > At Sun, 3 Oct 2004 00:38:04 +0200, > > > > Karsten Wiese wrote: > > > > One thing I'm not sure is that the possible race of usX2Y_urbs_start() > > > > and usX2Y_subs_startup_finish(). Better to use complete handler? > > > > > > did I care for any possible racing now? > > > > You need to call spin_lock(&usX2Y->prepare_lock) before checking > > subs->prepared in usX2Y_urbs_start(). In this kind of implementation, > > all the access to subs->prepared should be protected in the spinlock > > to avoid races. > > > > Also, the spin_lock(&usX2Y->prepare_lock) in usX2Y_urbs_start() may > > cause deadlock. Use spin_lock_irq(&usX2y->prepare_lock) instead. > > > Hi Takashi > > No spinlock no more. atomic_t instead. Please check. I'm afraid it's still racy. In usX2Y_urbs_start(), the complete callback may be called between usb_submit_urb() and the loop of schedule_timeout(). So, the loop should check at first the condition before sleeping like below: while (atomic_read(&subs->state) != state_PREPARED) { timeout = schedule_timeout(timeout); if (signal_pending(current)) ... if (! timeout) ... } Regarding the new config stuff: Let's kill CONFIG_SND_USB_USX2Y_NRPACKS_VARIABLE option. No one will want to disable such a module option. Also, please avoid the Kconfig int value as much as possible if you want to support the driver for 2.2/2.4 kernels, too. I believe it's ok to put a hardcoded value 4 like usbaudio.c if you describe the module option properly in the document. Takashi ------------------------------------------------------- This SF.net email is sponsored by: IT Product Guide on ITManagersJournal Use IT products in your business? Tell us what you think of them. Give us Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more http://productguide.itmanagersjournal.com/guidepromo.tmpl