Re: [PATCH] RME HDSPM MADI lock fix

["Jörn Nettingsmeier" <nettings@stackingdwarves.net>]

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On 06/06/2011 11:05 AM, Takashi Iwai wrote:
> At Sun, 05 Jun 2011 11:57:57 +0200,
> Jörn Nettingsmeier wrote:
>>
>> hi *!
>>
>> while we were trying to debug the midi-related xruns in the current
>> driver, i came across what looks like an unrelated locking bug to me:
>>
>> static int snd_hdspm_midi_input_read (struct hdspm_midi *hmidi)
>> ...
>>           spin_lock_irqsave (&hmidi->lock, flags);<---------------
>>           n_pending = snd_hdspm_midi_input_available (hmidi->hdspm,
>> hmidi->id);
>> ...
>>           hmidi->pending = 0;
>>
>>           hmidi->hdspm->control_register |= hmidi->ie;<------- !!!
>>           hdspm_write(hmidi->hdspm, HDSPM_controlRegister,
>>                       hmidi->hdspm->control_register);
>>
>>           spin_unlock_irqrestore (&hmidi->lock, flags);<---------
>>           return snd_hdspm_midi_output_write (hmidi);
>> }
>>
>>
>> if i understand this whole business correctly, we should be holding the
>> hdspm lock here, not the hmidi one.
>>
>> please review the attached patch and apply if you think it's ok.
>
> Well, it's a wrong lock for snd_hdspm_midi_input_available().
> So you'll need to lock twice, either once unlock and lock another or
> nested.  Re-locking would be less messy in this case, I suppose.

ok, here's a revised patch. has been running for 20 or so hours without 
problems, although the load was light most of the time.

please consider for inclusion.


best,


jörn



-- 
Jörn Nettingsmeier
Lortzingstr. 11, 45128 Essen, Tel. +49 177 7937487

Meister für Veranstaltungstechnik (Bühne/Studio)
Tonmeister (VDT)

http://stackingdwarves.net

[-- Attachment #2: hdspm-lock-fix.diff --]
[-- Type: text/x-patch, Size: 612 bytes --]

--- linux-2.6.39/sound/pci/rme9652/hdspm.c.orig	2011-05-19 06:06:34.000000000 +0200
+++ linux-2.6.39/sound/pci/rme9652/hdspm.c	2011-06-07 10:26:35.778933230 +0200
@@ -1639,12 +1639,14 @@
 		}
 	}
 	hmidi->pending = 0;
+	spin_unlock_irqrestore (&hmidi->lock, flags);
 
+	spin_lock_irqsave (&hmidi->hdspm->lock, flags);
 	hmidi->hdspm->control_register |= hmidi->ie;
 	hdspm_write(hmidi->hdspm, HDSPM_controlRegister,
 		    hmidi->hdspm->control_register);
+	spin_unlock_irqrestore (&hmidi->hdspm->lock, flags);
 
-	spin_unlock_irqrestore (&hmidi->lock, flags);
 	return snd_hdspm_midi_output_write (hmidi);
 }
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]