Why doesn't mixer control (values) have some kind of locking mechanism? (mutex?)

[Tom Yan]

Hi,

On Thu, 6 Aug 2020 at 10:06, Takashi Sakamoto <o-takashi at sakamocchi.jp> wrote:
>
> Hi,
>
> ALSA control core allows applications to lock/unlock a control element
> so that any write opreation to the control element fails for processes
> except for owner process.
>
> When a process requests `SNDRV_CTL_IOCTL_ELEM_LOCK`[1] against a
> control element. After operating the request, the control element is
> under 'owned by the process' state. In this state, any request of
> `SNDRV_CTL_IOCTL_ELEM_WRITE` from the other processes fails with
> `-EPERM`[2]. The write operation from the owner process is successful
> only. When the owner process is going to finish, the state is
> released[3].

That doesn't really address the problem anyway. As I've mentioned,
implementation of volume put() in kernel drivers often / can easily be
made atomic anyway (that might be the reason why this write lock isn't
popular). The problem/race I am trying to point out is, one process
can get()/read before another finishing its get()+put() pair (which is
required by volume setting/adjusting), so something like
get1()->get2()->put1()->put2() could easily happen (where each put()
relies on / is "configured" with volumes of their respective get()).
The lock will need to intentionally stall further get()/read as well.

If we for some reason want to avoid using locks, put() needs to be
atomic by design (like, "embed" get() in itself and use arrays for
volume values, instead of requiring those to be implemented in the
userspace manually / with a loop). Unfortunately that isn't the case
in ALSA.

>
> ALSA userspace library, a.k.a alsa-lib, has a pair of
> `snd_ctl_elem_lock()` and `snd_ctl_elem_unlock()` as its exported
> API[4].
>
> If application developers would like to bring failure to
> requests of `SNDRV_CTL_IOCTL_ELEM_WRITE` from the other processes in
> the period that the process requests `SNDRV_CTL_IOCTL_ELEM_READ` and
> `SNDRV_CTL_IOCTL_ELEM_WRITE` as a transaction, the lock/unlock
> mechanism is available. However, as long as I know, it's not used
> popularly.
>
>
> This is a simple demonstration about the above mechanism. PyGObject and
> alsa-gobject[5] is required to install:
>
> ```
> #!/usr/bin/env python3
>
> import gi
> gi.require_version('ALSACtl', '0.0')
> from gi.repository import ALSACtl
>
> import subprocess
>
> def run_amixer(should_err):
>   cmd = ('amixer', '-c', str(card_id),
>     'cset',
>     'iface={},name="{}",index={},device={},subdevice={},numid={}'.format(
>         eid.get_iface().value_nick, eid.get_name(),
>         eid.get_index(), eid.get_device_id(),
>         eid.get_subdevice_id(), eid.get_numid()),
>     '0,0',
>   )
>
>   result = subprocess.run(cmd, capture_output=True)
>   if result.stderr:
>     err = result.stderr.decode('UTF-8').rstrip()
>     print(' ', 'expected' if should_err else 'unexpected')
>     print('   ', err)
>   if result.stdout:
>     output = result.stdout.decode('UTF-8').rstrip().split('\n')
>     print(' ', 'expected' if not should_err else 'unexpected')
>     print('   ', output[-2])
>
> card_id = 0
> card = ALSACtl.Card.new()
> card.open(card_id, 0)
>
> for eid in card.get_elem_id_list():
>   prev_info = card.get_elem_info(eid)
>   if (prev_info.get_property('type') != ALSACtl.ElemType.INTEGER or
>       'write' not in prev_info.get_property('access').value_nicks or
>       'lock' in prev_info.get_property('access').value_nicks):
>       continue
>
>   card.lock_elem(eid, True)
>   print('  my program locks: "{}"'.format(eid.get_name()))
>   run_amixer_subprocess(True)
>
>   card.lock_elem(eid, False)
>   print('  my program unlocks: "{}"'.format(eid.get_name()))
>   run_amixer_subprocess(False)
> ```
>
> You can see the result of amixer execution is different in the cases of
> locked and unlocked, like:
>
> ```
> $ /tmp/lock-demo
>   ...
>   my program locks: "Headphone Playback Volume"
>   expected
>     amixer: Control hw:1 element write error: Operation not permitted
>   my program unlocks: "Headphone Playback Volume"
>   expected
>       : values=0,0
>   ...
> ```
>
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/include/uapi/sound/asound.h#n1083
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/sound/core/control.c#n1108
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/sound/core/control.c#n122
> [4] https://www.alsa-project.org/alsa-doc/alsa-lib/group___control.html#ga1fba1f7e08ab11505a617af5d54f4580
> [5] https://github.com/alsa-project/alsa-gobject
>
>
> Regards
>
> Takashi Sakamoto