Re: [PATCH 3/3] ALSA: emu10k1: (re-)add mixer locking

[Takashi Iwai <tiwai@suse.de>]

On Wed, 12 Jul 2023 16:57:50 +0200,
Oswald Buddenhagen wrote:
> 
> Takashi now "prefers" that the drivers do not rely on the core's locking
> of card->controls_rwsem, so we undo 06405d8ee8 ("ALSA: emu10k1: remove
> now superfluous mixer locking") and add more locks that were already
> missing.
> 
> This adds some rather significant critical sections with IRQs disabled,
> as emu->reg_lock is also accessed by the PCM trigger callbacks, which
> are called with the hardirq-safe (self-)group lock held.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> ---
> the long irq-disabled sections could be avoided by introducing a
> separate control mutex. i surveyed a few drivers, and the precedents
> are very mixed, so i'm not sure this would be considered worth it.
> 
> ---
> of the few drivers i checked, pcsp, ak4xxx-adda, pt2258, hal2,
> sgio2audio, au88x0_eq, and ca0106_mixer appear to be missing locking
> upon superficial inspection, a percentage well into the double digits.
> 
> given that there are ~3700 hits for snd_kcontrol_new (and many more
> _put() methods to check, due to initializer arrays), the whole endeavor
> seems just as utterly hopeless to me as i expected.
> 
> so i recommend that you reconsider, and consequently reject this patch.

I applied this patch now.  We may optimize out the whole locking
conditionally for the known-good drivers, instead of relying on a
(hackish) big iron lock that wasn't considered to be used originally
at all.


thanks,

Takashi