Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Clemens Ladisch <clemens@ladisch.de>
Cc: Jaroslav Kysela <perex@suse.cz>, alsa-devel@lists.sourceforge.net
Subject: Re: [PATCH] MPU-401 fixes
Date: Tue, 06 May 2003 13:33:14 +0200	[thread overview]
Message-ID: <s5hvfwob98l.wl@alsa2.suse.de> (raw)
In-Reply-To: <Pine.HPX.4.33n.0305051715000.3930-100000@studcom.urz.uni-halle.de>

At Mon, 05 May 2003 17:51:37 +0200 (METDST),
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.
> 
> 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.

yes, we need to avoid the double entrance of input_read() from
trigger().  but it's not necessary to be a atomic counter if the bit
flag is set/reset inside the spinlock but is only checked in
trigger() function.

the code is on cvs now.  please check it out.


ciao,

Takashi


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

  reply	other threads:[~2003-05-06 11:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-05  6:33 [PATCH] MPU-401 fixes Clemens Ladisch
2003-05-05 10:32 ` Jaroslav Kysela
2003-05-05 11:19   ` Clemens Ladisch
2003-05-05 14:15     ` Takashi Iwai
2003-05-05 15:51       ` Clemens Ladisch
2003-05-06 11:33         ` Takashi Iwai [this message]
2003-05-06 15:09         ` Jaroslav Kysela
2003-05-06 16:33           ` Takashi Iwai
2003-05-07  7:56             ` Jaroslav Kysela
2003-05-07  8:31               ` Clemens Ladisch
2003-05-07  9:03                 ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hvfwob98l.wl@alsa2.suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@lists.sourceforge.net \
    --cc=clemens@ladisch.de \
    --cc=perex@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox