From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] MPU-401 fixes Date: Tue, 06 May 2003 18:33:28 +0200 Sender: alsa-devel-admin@lists.sourceforge.net Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.4 - "Hosorogi") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: Errors-To: alsa-devel-admin@lists.sourceforge.net List-Help: List-Post: List-Subscribe: , List-Unsubscribe: , List-Archive: To: Jaroslav Kysela Cc: Clemens Ladisch , "alsa-devel@lists.sourceforge.net" List-Id: alsa-devel@alsa-project.org At Tue, 6 May 2003 17:09:38 +0200 (CEST), Jaroslav wrote: > > On Mon, 5 May 2003, Clemens Ladisch wrote: > > > Takashi Iwai wrote: > > > Clemens Ladisch wrote: > > > > Jaroslav Kysela wrote: > > > > > On Mon, 5 May 2003, Clemens Ladisch wrote: > > > > > > > > > > > - fix deadlock: when snd_mpu401_uart_input_trigger() is interrupted > > > > > > immediately after the call to spin_unlock_irqrestore(), the interrupt > > > > > > handler won't clear the interrupt condition because the RX_LOOP bit is > > > > > > still set > > > > > > > > > > Thanks. The RX_LOOP bit is cleared before unlock now. > > > > > > > > The same thing will happen when the interrupt occurs immediately before > > > > the call to spin_lock_irqsave(). > > > > > > > > The real problem is that the interrupt handler _must_ read the data from > > > > the device to clear the MPU401's interrupt condition. Therefore, there > > > > must not be any condition which can prevent the handler from reading data. > > > > > > hmm, this problem is not easy to solve. > > > > > > originally, the bit check was introduced to prevent double spinlocks > > > in the path: > > > > > > irq > > > -> snd_mpu401_uart_interrupt() > > > -> lock > > > -> snd_mpu401_uart_input_read() > > > -> snd_rawmidi_receive() > > > -> rawmidi->event() > > > -> rawmidi->trigger() (in seq_midi.c) > > > -> snd_mpu401_uart_input_trigger() > > > > AFAICS the only protection needed is against a recursive call of > > _input_trigger() but not of _interrupt() because the interrupt routine > > cannot be called inside the spin_lock_irqsave(), i.e., the code being > > interrupted will not be the _input_read() function. > > It's not true on MP. The interrupt handler can be run on different CPU > than trigger. yes, but it's protected by spinlock anyway. > > So a solution would be that _interrupt() sets the RX_LOOP bit, but does > > _not_ check it. But then RX_LOOP can be set twice, so we'd need an > > atomic_t counter instead. > > Ok, I'm now using the spin_trylock check in the trigger function and > RX_LOOP and TX_LOOP protectors are removed now. Hopefully, it finally > solves our problem. i'm afraid no. as mentioned before, we need to consider to avoid two things: 1. (infinite) recursive call of trigger() (via input_read()) 2. double spinlocks around input_read() from irq handler and the suceeding call of trigger() introducing spin_trylock() solves the case #2 but not case #1, since spin_trylock() always returns 1 on UP. for #1, an extra (formerly RX_LOOP bit) flag is still necessary. Takashi ------------------------------------------------------- Enterprise Linux Forum Conference & Expo, June 4-6, 2003, Santa Clara The only event dedicated to issues related to Linux enterprise solutions www.enterpriselinuxforum.com