All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Tom Yan <tom.ty89@gmail.com>
Cc: alsa-devel@alsa-project.org,
	pulseaudio-discuss@lists.freedesktop.org,
	alsa-user@alsa-project.org
Subject: Re: Why doesn't mixer control (values) have some kind of locking mechanism? (mutex?)
Date: Thu, 6 Aug 2020 11:06:01 +0900	[thread overview]
Message-ID: <20200806020601.GA6286@laptop> (raw)
In-Reply-To: <CAGnHSEkpYqyZJjG587FSVUzYX2zV1tm83zj+uGjF4e24o4iAMA@mail.gmail.com>

Hi,

On Thu, Aug 06, 2020 at 01:31:03AM +0800, Tom Yan wrote:
> Hi all,
> 
> I just wonder if it's a "no one cares" or a "no one was aware of it"
> issue (or maybe both?).
> 
> When you change (integer) values (e.g. volume) of a mixer control, it
> usually (if not always) involves calling two functions/methods of a
> snd_kcontrol_new, which are get and put, in order to do relative
> volume adjustments. (Apparently it is often done relatively even if we
> have absolute values, for reasons.)
> 
> While these two "actions" can be and probably are mostly "atomic"
> (with the help of mutex) in the kernel drivers *respectively*, they
> are not and cannot be atomic as a whole.
> 
> This won't really be an issue when the actions (either for one or
> multiple channels) are done "synchronously" in *one* program run (e.g.
> amixer -c STX set Master 1+). However, if such a program run is issued
> multiple times "asynchronously" (e.g. binding it to some
> XF86Audio{Raise,Lower}Volume scroll wheel), volume adjustment becomes
> a total mess / failure.
> 
> If it isn't obvious enough. it could happen like the following:
> get1(100 100)
> set1(101 100)
> get2(101 100)
> set2(102 100)
> ...
> 
> Or worse:
> get1(100 100)
> get2(100 100)
> set1(101 100)
> set2(100 101)
> ...
> 
> Not only that it may/will not finish the first set of adjustments for
> all channels before the second, get() from the second set could happen
> before set() from the first, reverting the effect of the earlier
> one(s).
> 
> Certainly one can use something like `flock` with amixer to make sure
> the atomicity of each issue/run, but not only that it looks silly and
> primitive, we don't always manipulate the mixer control with an
> "executable". For example, this weird issue in pulseaudio is probably
> related: https://bugs.freedesktop.org/show_bug.cgi?id=92717
> 
> So I wonder, is there a particular reason that mixer control doesn't
> possess some form of lock, which allows any form of userspace
> manipulation to lock it until what should be / is considered atomic is
> finished?

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].

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

  parent reply	other threads:[~2020-08-06  2:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 17:31 Why doesn't mixer control (values) have some kind of locking mechanism? (mutex?) Tom Yan
2020-08-05 18:40 ` Pierre-Louis Bossart
2020-08-06  2:06 ` Takashi Sakamoto [this message]
2020-08-06  8:57   ` Tom Yan
2020-08-06  9:14     ` Takashi Sakamoto
2020-08-06 12:31       ` Tom Yan
2020-08-06 14:47         ` Takashi Sakamoto
2020-08-06 15:34           ` Tom Yan
2020-08-06 17:19             ` Takashi Sakamoto
2020-08-06 18:45               ` Tom Yan
2020-08-07  9:08           ` Jaroslav Kysela
2020-08-06 15:30   ` [pulseaudio-discuss] " Pierre-Louis Bossart
2020-08-06 17:47     ` Takashi Sakamoto
2020-08-07  0:12       ` Pierre-Louis Bossart
2020-08-07  2:34         ` Takashi Sakamoto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200806020601.GA6286@laptop \
    --to=o-takashi@sakamocchi.jp \
    --cc=alsa-devel@alsa-project.org \
    --cc=alsa-user@alsa-project.org \
    --cc=pulseaudio-discuss@lists.freedesktop.org \
    --cc=tom.ty89@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.