Re: [PATCH] MPU-401 fixes

[Takashi Iwai <tiwai@suse.de>]

At Wed, 07 May 2003 10:31:46 +0200 (METDST),
Clemens Ladisch wrote:
> 
> Jaroslav Kysela wrote:
> > On Tue, 6 May 2003, Takashi Iwai wrote:
> > > > 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.
> >
> > I see. So the protectors are back in trigger() callbacks.
> 
> Now the recursive call sequence describe by Takashi is still possible:
> 
> >	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()
> 
> ... so the interrupt probably should set the bit, too. OTOH the recursion
> isn't infinite anymore, so the current code really doesn't need any more
> fixing.

yep, i agree that the flag should be set in the interrupt route, too.

i believe my last change (rev. 1.25) should work fine.
it sets the flag in input_read() (i.e. in spinlock).  instead of
spin_trylock() the bit is checked but it's safe because
- input_read is protected by spinlock between interrupt() and
  input_trigger()
- recursive call and double spinlocks of input_read() is avoided
  by checking the bit


ciao,

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