-----Original Message-----
From: cip-dev@... <cip-dev@...> On
Behalf Of Alexander Grund
Sent: Saturday, March 25, 2023 2:18 AM
To: cip-dev@...
Cc: uli+cip@...
Subject: [cip-dev] [PATCH 4.4 3/3] ALSA: control: use counting semaphore as
write lock for ELEM_WRITE operation

From: Takashi Sakamoto <o-takashi@...>

In ALSA control interface, applications can execute two types of request for
value of members on each element; ELEM_READ and ELEM_WRITE. In ALSA
control core, these two requests are handled within read lock of a counting
semaphore, therefore several processes can run to execute these two requests
at the same time. This has an issue because ELEM_WRITE requests have an
effect to change state of the target element. Concurrent access should be
controlled for each of ELEM_READ/ELEM_WRITE case.

This commit uses the counting semaphore as write lock for ELEM_WRITE
requests, while use it as read lock for ELEM_READ requests. The state of a
target element is maintained exclusively between ELEM_WRITE/ELEM_READ
operations.

There's a concern. If the counting semaphore is acquired for read lock in
implementations of 'struct snd_kcontrol.put()' in each driver, this commit shall
cause dead lock. As of v4.13-rc5, 'snd-mixer-oss.ko', 'snd-emu10k1.ko' and
'snd-soc-sst-atom-hifi2-platform.ko' includes codes for read locks, but these
are not in a call graph from 'struct snd_kcontrol.put(). Therefore, this commit is
safe.

In current implementation, the same solution is applied for the other operations
to element; e.g. ELEM_LOCK and ELEM_UNLOCK. There's another discussion
about an overhead to maintain concurrent access to an element during
operating the other elements on the same card instance, because the lock
primitive is originally implemented to maintain a list of elements on the card
instance. There's a substantial difference between per-element-list lock and
per-element lock.

Here, let me investigate another idea to add per-element lock to maintain the
concurrent accesses with inquiry/change requests to an element. It's not so
frequent for applications to operate members on elements, while adding a new
lock primitive to structure increases memory footprint for all of element sets
somehow. Experimentally, inquiry operation is more frequent than change
operation and usage of counting semaphore for the inquiry operation brings no
blocking to the other inquiry operations. Thus the overhead is not so critical for
usual applications. For the above reasons, in this commit, the per-element lock
is not introduced.

Signed-off-by: Takashi Sakamoto <o-takashi@...>
Signed-off-by: Takashi Iwai <tiwai@...>
Signed-off-by: Alexander Grund <theflamefire89@...>
---
sound/core/control.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/control.c b/sound/core/control.c index
3ca81e85a1492..04b939df3768b 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -963,9 +963,9 @@ static int snd_ctl_elem_write_user(struct snd_ctl_file
*file,
snd_power_lock(card);
result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
if (result >= 0) {
- down_read(&card->controls_rwsem);
+ down_write(&card->controls_rwsem);
result = snd_ctl_elem_write(card, file, control);
- up_read(&card->controls_rwsem);
+ up_write(&card->controls_rwsem);
}
snd_power_unlock(card);
if (result >= 0)
--
2.40.0