Re: [PATCH 2/3] ALSA: emu10k1: remove superfluous IRQ enable state saving

[Takashi Iwai <tiwai@suse.de>]

On Thu, 13 Jul 2023 10:15:21 +0200,
Oswald Buddenhagen wrote:
> 
> On Thu, Jul 13, 2023 at 07:55:31AM +0200, Takashi Iwai wrote:
> > On Wed, 12 Jul 2023 16:57:49 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> The mixer, PCM prepare, MIDI, synth driver, and procfs callbacks are all
> >> always invoked with IRQs enabled, so there is no point in saving the
> >> state.
> >> 
> >> snd_emu1010_load_firmware_entry() is called from emu1010_firmware_work()
> >> and snd_emu10k1_emu1010_init(); the latter from snd_emu10k1_create() and
> >> snd_emu10k1_resume(), all of which have IRQs enabled.
> >> 
> >> The voice and memory functions are called from mixed contexts, so they
> >> keep the state saving.
> >> 
> >> The low-level functions all keep the state saving, because it's not
> >> feasible to keep track of what is called where.
> >> 
> > Wouldn't it make more sense if you replace it with a mutex?
> > It'll become more obvious that it's only for non-IRQ context, too.
> > 
> huh?
> at least some of the ~six different locks touched by this patch
> absolutely _are_ used in irq context. this patch is concerned only
> about the specific call sites, where we know that local irqs are
> enabled, so we can unconditionally re-enable them rather than
> restoring the old state (the latter being a much more expensive
> operation). the code already contains precedents for this, and the
> complementary optimization of not disabling/restoring irqs where we
> know that they are already disabled.
> 
> the reg_lock would be convertible to a mixer_mutex in most mixer
> callbacks, but that is an orthogonal question, which is raised in the
> next commit.

Ah, sorry, I misread as if it were dropping the whole *_irq.
Then the patch should be fine.


Takashi