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