Re: [PATCH RFC] ALSA: control: Avoid nested locks at notification

[Takashi Iwai <tiwai@suse.de>]

On Tue, 23 May 2023 18:53:11 +0200,
Jaroslav Kysela wrote:
> 
> On 23. 05. 23 17:52, Takashi Iwai wrote:
> > The new control layer stuff introduced the nested rwsem for managing
> > the list of registered control layer ops.  Since then, a global
> > snd_ctl_layer_rwsem is always read at each time a control notification
> > is sent via snd_ctl_notify*() in a nested matter inside the card's
> > controls_rwsem lock.  This also required a bit complicated way of the
> > lock at snd_ctl_activate_id() and snd_ctl_elem_write() with the
> > downgrade of rwsem.
> > 
> > This patch is an attempt to simplify the handling of ctl layer ops.
> > Now, instead of traversing the global linked list, we keep a local
> > list of lops in each card instance.  This reduces the need of the
> > global snd_ctl_layer_rwsem lock at snd_ctl_notify*() invocation.
> > And, since the nested lock is avoided in most places, we can also
> > avoid the rwsem downgrade hack in the above, too.
> > 
> > Since the local list entry is created dynamically,
> > snd_ctl_register_layer() may return an error, and the caller needs to
> > check the return value.
> 
> I'm not convinced about this transition. What about to move the layer
> notifications to a workqueue to reorder the controls_rwsem locking
> (kctl access) ?

It'll need allocation of each new notify event, no?

IMO, keeping the list of ops in each card instance may have a room for
more possible improvement.  The current patch chains always all lops
in the card's lops_list, but when the register callback returns a
certain error code for the uninteresting lops, we can skip chaining
it.  Then the unnecessary hook invocation at each notification can be
avoided.

> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> > 
> > I noticed the nested lock while looking at the pending bug report
> 
> From the log or code ?

The code.

And, control_led.c has also a global mutex lock that is applied for
almost all code paths.  If the hook is called from all cards, this
should be optimized as well.  (OTOH, if it's called only from one
bound card, it won't matter much.)


thanks,

Takashi