From 8dce39b8955be6164172cb6204ef8fc21de27431 Mon Sep 17 00:00:00 2001 From: Krzysztof Helt Date: Wed, 7 Oct 2009 22:51:34 +0200 Subject: [PATCH] ALSA: opl3: circular locking in the snd_opl3_note_on() and snd_opl3_note_off() Fix following circular locking in the opl3 driver. ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.32-rc3 #87 ------------------------------------------------------- swapper/0 is trying to acquire lock: (&opl3->voice_lock){..-...}, at: [] snd_opl3_note_off+0x1e/0xe0 [snd_opl3_synth] but task is already holding lock: (&opl3->sys_timer_lock){..-...}, at: [] snd_opl3_timer_func+0x19/0xc0 [snd_opl3_synth] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&opl3->sys_timer_lock){..-...}: [] validate_chain+0xa25/0x1040 [] __lock_acquire+0x2da/0xab0 [] lock_acquire+0x7a/0xa0 [] _spin_lock_irqsave+0x40/0x60 [] snd_opl3_note_on+0x686/0x790 [snd_opl3_synth] [] snd_midi_process_event+0x322/0x590 [snd_seq_midi_emul] [] snd_opl3_synth_event_input+0x15/0x20 [snd_opl3_synth] [] snd_seq_deliver_single_event+0x100/0x200 [snd_seq] [] snd_seq_deliver_event+0x47/0x1f0 [snd_seq] [] snd_seq_dispatch_event+0x3b/0x140 [snd_seq] [] snd_seq_check_queue+0x10c/0x120 [snd_seq] [] snd_seq_enqueue_event+0x6b/0xe0 [snd_seq] [] snd_seq_client_enqueue_event+0xdd/0x100 [snd_seq] [] snd_seq_write+0xea/0x190 [snd_seq] [] vfs_write+0x96/0x160 [] sys_write+0x3d/0x70 [] syscall_call+0x7/0xb -> #0 (&opl3->voice_lock){..-...}: [] validate_chain+0x1036/0x1040 [] __lock_acquire+0x2da/0xab0 [] lock_acquire+0x7a/0xa0 [] _spin_lock_irqsave+0x40/0x60 [] snd_opl3_note_off+0x1e/0xe0 [snd_opl3_synth] [] snd_opl3_timer_func+0xa0/0xc0 [snd_opl3_synth] [] run_timer_softirq+0x166/0x1e0 [] __do_softirq+0x78/0x110 [] do_softirq+0x46/0x50 [] irq_exit+0x36/0x40 [] do_IRQ+0x42/0xb0 [] common_interrupt+0x2e/0x40 [] apm_cpu_idle+0x10f/0x290 [] cpu_idle+0x21/0x40 [] rest_init+0x4d/0x60 [] start_kernel+0x235/0x280 [] i386_start_kernel+0x66/0x70 other info that might help us debug this: 2 locks held by swapper/0: #0: (&opl3->tlist){+.-...}, at: [] run_timer_softirq+0xf0/0x1e0 #1: (&opl3->sys_timer_lock){..-...}, at: [] snd_opl3_timer_func+0x19/0xc0 [snd_opl3_synth] stack backtrace: Pid: 0, comm: swapper Not tainted 2.6.32-rc3 #87 Call Trace: [] print_circular_bug+0xc8/0xd0 [] validate_chain+0x1036/0x1040 [] ? check_usage_forwards+0x54/0xd0 [] __lock_acquire+0x2da/0xab0 [] lock_acquire+0x7a/0xa0 [] ? snd_opl3_note_off+0x1e/0xe0 [snd_opl3_synth] [] _spin_lock_irqsave+0x40/0x60 [] ? snd_opl3_note_off+0x1e/0xe0 [snd_opl3_synth] [] snd_opl3_note_off+0x1e/0xe0 [snd_opl3_synth] [] ? _spin_lock_irqsave+0x47/0x60 [] snd_opl3_timer_func+0xa0/0xc0 [snd_opl3_synth] [] run_timer_softirq+0x166/0x1e0 [] ? run_timer_softirq+0xf0/0x1e0 [] ? snd_opl3_timer_func+0x0/0xc0 [snd_opl3_synth] [] __do_softirq+0x78/0x110 [] ? _spin_unlock+0x1d/0x20 [] ? handle_level_irq+0xaf/0xe0 [] do_softirq+0x46/0x50 [] irq_exit+0x36/0x40 [] do_IRQ+0x42/0xb0 [] ? trace_hardirqs_on_caller+0x12c/0x180 [] common_interrupt+0x2e/0x40 [] ? default_idle+0x38/0x50 [] apm_cpu_idle+0x10f/0x290 [] cpu_idle+0x21/0x40 [] rest_init+0x4d/0x60 [] start_kernel+0x235/0x280 [] ? unknown_bootoption+0x0/0x210 [] i386_start_kernel+0x66/0x70 Signed-off-by: Krzysztof Helt Signed-off-by: Takashi Iwai --- sound/drivers/opl3/opl3_midi.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/sound/drivers/opl3/opl3_midi.c b/sound/drivers/opl3/opl3_midi.c index 6e7d09ae0e82..7d722a025d0d 100644 --- a/sound/drivers/opl3/opl3_midi.c +++ b/sound/drivers/opl3/opl3_midi.c @@ -29,6 +29,8 @@ extern char snd_opl3_regmap[MAX_OPL2_VOICES][4]; extern int use_internal_drums; +static void snd_opl3_note_off_unsafe(void *p, int note, int vel, + struct snd_midi_channel *chan); /* * The next table looks magical, but it certainly is not. Its values have * been calculated as table[i]=8*log(i/64)/log(2) with an obvious exception @@ -242,16 +244,20 @@ void snd_opl3_timer_func(unsigned long data) int again = 0; int i; - spin_lock_irqsave(&opl3->sys_timer_lock, flags); + spin_lock_irqsave(&opl3->voice_lock, flags); for (i = 0; i < opl3->max_voices; i++) { struct snd_opl3_voice *vp = &opl3->voices[i]; if (vp->state > 0 && vp->note_off_check) { if (vp->note_off == jiffies) - snd_opl3_note_off(opl3, vp->note, 0, vp->chan); + snd_opl3_note_off_unsafe(opl3, vp->note, 0, + vp->chan); else again++; } } + spin_unlock_irqrestore(&opl3->voice_lock, flags); + + spin_lock_irqsave(&opl3->sys_timer_lock, flags); if (again) { opl3->tlist.expires = jiffies + 1; /* invoke again */ add_timer(&opl3->tlist); @@ -658,15 +664,14 @@ static void snd_opl3_kill_voice(struct snd_opl3 *opl3, int voice) /* * Release a note in response to a midi note off. */ -void snd_opl3_note_off(void *p, int note, int vel, struct snd_midi_channel *chan) +static void snd_opl3_note_off_unsafe(void *p, int note, int vel, + struct snd_midi_channel *chan) { struct snd_opl3 *opl3; int voice; struct snd_opl3_voice *vp; - unsigned long flags; - opl3 = p; #ifdef DEBUG_MIDI @@ -674,12 +679,9 @@ void snd_opl3_note_off(void *p, int note, int vel, struct snd_midi_channel *chan chan->number, chan->midi_program, note); #endif - spin_lock_irqsave(&opl3->voice_lock, flags); - if (opl3->synth_mode == SNDRV_OPL3_MODE_SEQ) { if (chan->drum_channel && use_internal_drums) { snd_opl3_drum_switch(opl3, note, vel, 0, chan); - spin_unlock_irqrestore(&opl3->voice_lock, flags); return; } /* this loop will hopefully kill all extra voices, because @@ -697,6 +699,16 @@ void snd_opl3_note_off(void *p, int note, int vel, struct snd_midi_channel *chan snd_opl3_kill_voice(opl3, voice); } } +} + +void snd_opl3_note_off(void *p, int note, int vel, + struct snd_midi_channel *chan) +{ + struct snd_opl3 *opl3 = p; + unsigned long flags; + + spin_lock_irqsave(&opl3->voice_lock, flags); + snd_opl3_note_off_unsafe(p, note, vel, chan); spin_unlock_irqrestore(&opl3->voice_lock, flags); }