* [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API
@ 2016-07-05 15:20 Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 1/5] pcm: " Takashi Iwai
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
Hi,
here is an experimental patchset to add multi thread safety to PCM
functions. Basically ALSA PCM functions are thread-unsafe, and
applications are supposed to do the proper protection against racy
accesses. The reality is, however, that application developers don't
care such, as alsa-lib works in most cases. Of course, users still
get occasionally mysterious crashes.
As a workaround, this patchset adds the pthread mutex protection to
most of exported PCM functions. This is slightly an overkill, but the
biggest merit is that it's easy to implement; I just wrapped the
functions and replaced the internal ones with unlocked versions.
To be noted, there is an optimization for the direct hw PCM access.
Performance-sensitive applications like JACK should work like before
without any overhead.
Another bonus by this addition is that we can finally get rid of home
brew (and deadly smelling) atomic macros from the tree, since it's now
more widely protected.
I lightly tested on my local machines and it seems working well, so
far.
Comments and suggestions welcome.
thanks,
Takashi
===
Takashi Iwai (5):
pcm: Add thread-safety to PCM API
test: Add pcm-multi-thread program
Add pcm-multi-thread to .gitignore
pcm: Remove superfluous rmb() from PCM meter plugin
pcm: Remove home brew atomic operations
.gitignore | 1 +
configure.ac | 15 ++
include/Makefile.am | 2 +-
include/iatomic.h | 170 ------------------
src/pcm/Makefile.am | 2 +-
src/pcm/atomic.c | 43 -----
src/pcm/pcm.c | 468 +++++++++++++++++++++++++++++++++++++++++-------
src/pcm/pcm_direct.c | 4 +-
src/pcm/pcm_dmix.c | 13 +-
src/pcm/pcm_dshare.c | 13 +-
src/pcm/pcm_dsnoop.c | 15 +-
src/pcm/pcm_file.c | 21 ++-
src/pcm/pcm_generic.c | 10 +-
src/pcm/pcm_hw.c | 3 +
src/pcm/pcm_ioplug.c | 11 +-
src/pcm/pcm_local.h | 62 ++++++-
src/pcm/pcm_meter.c | 1 -
src/pcm/pcm_mmap.c | 16 +-
src/pcm/pcm_params.c | 2 +-
src/pcm/pcm_plugin.c | 72 ++------
src/pcm/pcm_plugin.h | 2 -
src/pcm/pcm_rate.c | 41 ++---
src/pcm/pcm_route.c | 2 +-
test/Makefile.am | 3 +-
test/pcm-multi-thread.c | 263 +++++++++++++++++++++++++++
25 files changed, 851 insertions(+), 404 deletions(-)
delete mode 100644 include/iatomic.h
delete mode 100644 src/pcm/atomic.c
create mode 100644 test/pcm-multi-thread.c
--
2.9.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH RFC alsa-lib 1/5] pcm: Add thread-safety to PCM API
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
@ 2016-07-05 15:20 ` Takashi Iwai
2016-07-05 16:30 ` Clemens Ladisch
2016-07-05 15:20 ` [PATCH RFC alsa-lib 2/5] test: Add pcm-multi-thread program Takashi Iwai
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
Traditionally, many of ALSA library functions are supposed to be
thread-unsafe, and applications are required to take care of thread
safety by themselves. However, people never be careful enough, and
almost all applications fail in this regard.
This patch is an attempt to harden the thread safety in exported PCM
functions in a simplistic way: namely, just wrap the exported
functions with the pthread mutex of each PCM object. Some functions
are split to both unlocked and locked versions for avoiding
deadlocks.
Two new fields are added snd_pcm_t when the option is enabled:
thread_safe and lock. The former indicates that the plugin is
thread-safe that doesn't need this workaround and the latter is the
pthread mutex. Currently only hw plugin has thread_safe=1. So, the
most of real-time sensitive apps won't be influenced by this change at
all.
Although the patch covers most of PCM ops, a few snd_pcm_fast_ops are
left without the extra mutex locking: namely, the ones that may have
blocking behavior, i.e. resume, drain, readi, writei, readn and
writen. These are supposed to handle own locking in the callbacks.
If anyone wants to disable this new thread-safe API feature, it can be
still turned off via --disable-thread-safety configure option.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
configure.ac | 15 ++
src/pcm/pcm.c | 468 +++++++++++++++++++++++++++++++++++++++++++-------
src/pcm/pcm_direct.c | 4 +-
src/pcm/pcm_dmix.c | 13 +-
src/pcm/pcm_dshare.c | 13 +-
src/pcm/pcm_dsnoop.c | 15 +-
src/pcm/pcm_file.c | 21 ++-
src/pcm/pcm_generic.c | 10 +-
src/pcm/pcm_hw.c | 3 +
src/pcm/pcm_ioplug.c | 11 +-
src/pcm/pcm_local.h | 62 ++++++-
src/pcm/pcm_mmap.c | 16 +-
src/pcm/pcm_params.c | 2 +-
src/pcm/pcm_rate.c | 7 +-
src/pcm/pcm_route.c | 2 +-
15 files changed, 565 insertions(+), 97 deletions(-)
diff --git a/configure.ac b/configure.ac
index 53fca33e201a..ce324411fd61 100644
--- a/configure.ac
+++ b/configure.ac
@@ -632,6 +632,21 @@ elif test "$max_cards" -gt 256; then
fi
AC_DEFINE_UNQUOTED(SND_MAX_CARDS, $max_cards, [Max number of cards])
+dnl Check for thread-safe API functions
+if test "$HAVE_LIBPTHREAD" = "yes"; then
+AC_MSG_CHECKING(for thread-safe API functions)
+AC_ARG_ENABLE(thread-safety,
+ AS_HELP_STRING([--disable-thread-safety],
+ [disable thread-safe API functions]),
+ threadsafe="$enableval", threadsafe="yes")
+if test "$threadsafe" = "yes"; then
+ AC_MSG_RESULT(yes)
+ AC_DEFINE([THREAD_SAFE_API], "1", [Build thread-safe API functions])
+else
+ AC_MSG_RESULT(no)
+fi
+fi
+
dnl Make a symlink for inclusion of alsa/xxx.h
if test ! -L "$srcdir"/include/alsa ; then
echo "Making a symlink include/alsa"
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 0d0d093deb49..413cc94b8f2e 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -686,6 +686,8 @@ snd_pcm_stream_t snd_pcm_stream(snd_pcm_t *pcm)
*
* Closes the specified PCM handle and frees all associated
* resources.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_close(snd_pcm_t *pcm)
{
@@ -697,6 +699,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
if (err < 0)
res = err;
}
+ snd_pcm_lock(pcm);
if (pcm->mmap_channels)
snd_pcm_munmap(pcm);
while (!list_empty(&pcm->async_handlers)) {
@@ -704,6 +707,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
snd_async_del_handler(h);
}
err = pcm->ops->close(pcm->op_arg);
+ snd_pcm_unlock(pcm);
if (err < 0)
res = err;
err = snd_pcm_free(pcm);
@@ -712,16 +716,32 @@ int snd_pcm_close(snd_pcm_t *pcm)
return res;
}
+static int __snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock);
+
/**
* \brief set nonblock mode
* \param pcm PCM handle
* \param nonblock 0 = block, 1 = nonblock mode, 2 = abort
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
{
int err;
+
assert(pcm);
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_nonblock(pcm, nonblock);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOXYGEN
+static int __snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
+{
+ int err;
+
if ((err = pcm->ops->nonblock(pcm->op_arg, nonblock)) < 0)
return err;
if (nonblock == 2) {
@@ -738,7 +758,6 @@ int snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
return 0;
}
-#ifndef DOC_HIDDEN
/**
* \brief set async mode
* \param pcm PCM handle
@@ -764,17 +783,26 @@ int snd_pcm_async(snd_pcm_t *pcm, int sig, pid_t pid)
* \param pcm PCM handle
* \param info Information container
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_info(snd_pcm_t *pcm, snd_pcm_info_t *info)
{
+ int err;
+
assert(pcm && info);
- return pcm->ops->info(pcm->op_arg, info);
+ snd_pcm_lock(pcm);
+ err = pcm->ops->info(pcm->op_arg, info);
+ snd_pcm_unlock(pcm);
+ return err;
}
/** \brief Retreive current PCM hardware configuration chosen with #snd_pcm_hw_params
* \param pcm PCM handle
* \param params Configuration space definition container
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_hw_params_current(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
{
@@ -783,6 +811,7 @@ int snd_pcm_hw_params_current(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
assert(pcm && params);
if (!pcm->setup)
return -EBADFD;
+ snd_pcm_lock(pcm);
memset(params, 0, snd_pcm_hw_params_sizeof());
params->flags = pcm->hw_flags;
snd_mask_set(¶ms->masks[SND_PCM_HW_PARAM_ACCESS - SND_PCM_HW_PARAM_FIRST_MASK], pcm->access);
@@ -803,6 +832,7 @@ int snd_pcm_hw_params_current(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
params->rate_num = pcm->rate_num;
params->rate_den = pcm->rate_den;
params->fifo_size = pcm->fifo_size;
+ snd_pcm_unlock(pcm);
return 0;
}
@@ -826,12 +856,16 @@ int snd_pcm_hw_params_current(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
*
* The configuration space will be updated to reflect the chosen
* parameters.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
{
int err;
assert(pcm && params);
+ snd_pcm_lock(pcm);
err = _snd_pcm_hw_params_internal(pcm, params);
+ snd_pcm_unlock(pcm);
if (err < 0)
return err;
err = snd_pcm_prepare(pcm);
@@ -841,21 +875,27 @@ int snd_pcm_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
/** \brief Remove PCM hardware configuration and free associated resources
* \param pcm PCM handle
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_hw_free(snd_pcm_t *pcm)
{
- int err;
+ int err = 0;
+
+ snd_pcm_lock(pcm);
if (! pcm->setup)
- return 0;
+ goto unlock;
if (pcm->mmap_channels) {
err = snd_pcm_munmap(pcm);
if (err < 0)
- return err;
+ goto unlock;
}
- // assert(snd_pcm_state(pcm) == SND_PCM_STATE_SETUP ||
- // snd_pcm_state(pcm) == SND_PCM_STATE_PREPARED);
+ // assert(__snd_pcm_state(pcm) == SND_PCM_STATE_SETUP ||
+ // __snd_pcm_state(pcm) == SND_PCM_STATE_PREPARED);
err = pcm->ops->hw_free(pcm->op_arg);
pcm->setup = 0;
+ unlock:
+ snd_pcm_unlock(pcm);
if (err < 0)
return err;
return 0;
@@ -869,6 +909,8 @@ int snd_pcm_hw_free(snd_pcm_t *pcm)
* The software parameters can be changed at any time.
* The hardware parameters cannot be changed when the stream is
* running (active).
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
{
@@ -892,6 +934,18 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
return -EINVAL;
}
#endif
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_sw_params(pcm, params);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOC_HIDDEN
+/* locked version */
+int __snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
+{
+ int err;
+
err = pcm->ops->sw_params(pcm->op_arg, params);
if (err < 0)
return err;
@@ -907,17 +961,24 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params)
pcm->boundary = params->boundary;
return 0;
}
+#endif
/**
* \brief Obtain status (runtime) information for PCM handle
* \param pcm PCM handle
* \param status Status container
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status)
{
+ int err;
+
assert(pcm && status);
- return pcm->fast_ops->status(pcm->fast_op_arg, status);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->status(pcm->fast_op_arg, status);
+ snd_pcm_unlock(pcm);
}
/**
@@ -927,13 +988,33 @@ int snd_pcm_status(snd_pcm_t *pcm, snd_pcm_status_t *status)
*
* This is a faster way to obtain only the PCM state without calling
* \link ::snd_pcm_status() \endlink.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_state_t snd_pcm_state(snd_pcm_t *pcm)
{
+ snd_pcm_state_t state;
+
assert(pcm);
+ snd_pcm_lock(pcm);
+ state = __snd_pcm_state(pcm);
+ snd_pcm_unlock(pcm);
+ return state;
+}
+
+#ifndef DOC_HIDDEN
+/* locked version */
+snd_pcm_state_t __snd_pcm_state(snd_pcm_t *pcm)
+{
return pcm->fast_ops->state(pcm->fast_op_arg);
}
+static inline int pcm_hwsync(snd_pcm_t *pcm)
+{
+ return pcm->fast_ops->hwsync(pcm->fast_op_arg);
+}
+#endif
+
/**
* \brief (DEPRECATED) Synchronize stream position with hardware
* \param pcm PCM handle
@@ -942,15 +1023,22 @@ snd_pcm_state_t snd_pcm_state(snd_pcm_t *pcm)
* Note this function does not update the actual r/w pointer
* for applications. The function #snd_pcm_avail_update()
* have to be called before any mmap begin+commit operation.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_hwsync(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->hwsync(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm_hwsync(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
}
#ifndef DOC_HIDDEN
link_warning(snd_pcm_hwsync, "Warning: snd_pcm_hwsync() is deprecated, consider to use snd_pcm_avail()");
@@ -983,15 +1071,22 @@ link_warning(snd_pcm_hwsync, "Warning: snd_pcm_hwsync() is deprecated, consider
* Note this function does not update the actual r/w pointer
* for applications. The function #snd_pcm_avail_update()
* have to be called before any begin+commit operation.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->delay(pcm->fast_op_arg, delayp);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->delay(pcm->fast_op_arg, delayp);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1005,6 +1100,8 @@ int snd_pcm_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
* to do the fine resume from this state. Not all hardware supports
* this feature, when an -ENOSYS error is returned, use the \link ::snd_pcm_prepare() \endlink
* function to recovery.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_resume(snd_pcm_t *pcm)
{
@@ -1013,6 +1110,7 @@ int snd_pcm_resume(snd_pcm_t *pcm)
SNDMSG("PCM not set up");
return -EIO;
}
+ /* lock handled in the callback */
return pcm->fast_ops->resume(pcm->fast_op_arg);
}
@@ -1025,30 +1123,44 @@ int snd_pcm_resume(snd_pcm_t *pcm)
*
* Note this function does not update the actual r/w pointer
* for applications.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_htimestamp(snd_pcm_t *pcm, snd_pcm_uframes_t *avail, snd_htimestamp_t *tstamp)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->htimestamp(pcm->fast_op_arg, avail, tstamp);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->htimestamp(pcm->fast_op_arg, avail, tstamp);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
* \brief Prepare PCM for use
* \param pcm PCM handle
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_prepare(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->prepare(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->prepare(pcm->fast_op_arg);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1057,30 +1169,51 @@ int snd_pcm_prepare(snd_pcm_t *pcm)
* \return 0 on success otherwise a negative error code
*
* Reduce PCM delay to 0.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_reset(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->reset(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->reset(pcm->fast_op_arg);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOXYGEN
+static int pcm_start(snd_pcm_t *pcm)
+{
+ return pcm->fast_ops->start(pcm->fast_op_arg);
}
+#endif
/**
* \brief Start a PCM
* \param pcm PCM handle
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_start(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->start(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm_start(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1093,15 +1226,22 @@ int snd_pcm_start(snd_pcm_t *pcm)
*
* For processing all pending samples, use \link ::snd_pcm_drain() \endlink
* instead.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_drop(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->drop(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->drop(pcm->fast_op_arg);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1116,6 +1256,8 @@ int snd_pcm_drop(snd_pcm_t *pcm)
*
* For stopping the PCM stream immediately, use \link ::snd_pcm_drop() \endlink
* instead.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_drain(snd_pcm_t *pcm)
{
@@ -1124,6 +1266,7 @@ int snd_pcm_drain(snd_pcm_t *pcm)
SNDMSG("PCM not set up");
return -EIO;
}
+ /* lock handled in the callback */
return pcm->fast_ops->drain(pcm->fast_op_arg);
}
@@ -1136,15 +1279,22 @@ int snd_pcm_drain(snd_pcm_t *pcm)
* Note that this function works only on the hardware which supports
* pause feature. You can check it via \link ::snd_pcm_hw_params_can_pause() \endlink
* function.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_pause(snd_pcm_t *pcm, int enable)
{
+ int err;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->pause(pcm->fast_op_arg, enable);
+ snd_pcm_lock(pcm);
+ err = pcm->fast_ops->pause(pcm->fast_op_arg, enable);
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1155,15 +1305,22 @@ int snd_pcm_pause(snd_pcm_t *pcm, int enable)
* Note: The snd_pcm_rewind() can accept bigger value than returned
* by this function. But it is not guaranteed that output stream
* will be consistent with bigger value.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_rewindable(snd_pcm_t *pcm)
{
+ snd_pcm_sframes_t result;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->rewindable(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ result = pcm->fast_ops->rewindable(pcm->fast_op_arg);
+ snd_pcm_unlock(pcm);
+ return result;
}
/**
@@ -1172,9 +1329,13 @@ snd_pcm_sframes_t snd_pcm_rewindable(snd_pcm_t *pcm)
* \param frames wanted displacement in frames
* \return a positive number for actual displacement otherwise a
* negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
{
+ snd_pcm_sframes_t result;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
@@ -1182,7 +1343,10 @@ snd_pcm_sframes_t snd_pcm_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
}
if (frames == 0)
return 0;
- return pcm->fast_ops->rewind(pcm->fast_op_arg, frames);
+ snd_pcm_lock(pcm);
+ result = pcm->fast_ops->rewind(pcm->fast_op_arg, frames);
+ snd_pcm_unlock(pcm);
+ return result;
}
/**
@@ -1193,15 +1357,22 @@ snd_pcm_sframes_t snd_pcm_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
* Note: The snd_pcm_forward() can accept bigger value than returned
* by this function. But it is not guaranteed that output stream
* will be consistent with bigger value.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_forwardable(snd_pcm_t *pcm)
{
+ snd_pcm_sframes_t result;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- return pcm->fast_ops->forwardable(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ result = pcm->fast_ops->forwardable(pcm->fast_op_arg);
+ snd_pcm_unlock(pcm);
+ return result;
}
/**
@@ -1210,6 +1381,8 @@ snd_pcm_sframes_t snd_pcm_forwardable(snd_pcm_t *pcm)
* \param frames wanted skip in frames
* \return a positive number for actual skip otherwise a negative error code
* \retval 0 means no action
+ *
+ * The function is thread-safe when built with the proper option.
*/
#ifndef DOXYGEN
snd_pcm_sframes_t INTERNAL(snd_pcm_forward)(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
@@ -1217,6 +1390,8 @@ snd_pcm_sframes_t INTERNAL(snd_pcm_forward)(snd_pcm_t *pcm, snd_pcm_uframes_t fr
snd_pcm_sframes_t snd_pcm_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
#endif
{
+ snd_pcm_sframes_t result;
+
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
@@ -1224,7 +1399,10 @@ snd_pcm_sframes_t snd_pcm_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
}
if (frames == 0)
return 0;
- return pcm->fast_ops->forward(pcm->fast_op_arg, frames);
+ snd_pcm_lock(pcm);
+ result = pcm->fast_ops->forward(pcm->fast_op_arg, frames);
+ snd_pcm_unlock(pcm);
+ return result;
}
use_default_symbol_version(__snd_pcm_forward, snd_pcm_forward, ALSA_0.9.0rc8);
@@ -1244,6 +1422,8 @@ use_default_symbol_version(__snd_pcm_forward, snd_pcm_forward, ALSA_0.9.0rc8);
* The returned number of frames can be less only if a signal or underrun occurred.
*
* If the non-blocking behaviour is selected, then routine doesn't wait at all.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size)
{
@@ -1276,6 +1456,8 @@ snd_pcm_sframes_t snd_pcm_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_ufr
* The returned number of frames can be less only if a signal or underrun occurred.
*
* If the non-blocking behaviour is selected, then routine doesn't wait at all.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
@@ -1308,6 +1490,8 @@ snd_pcm_sframes_t snd_pcm_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t
* if a signal or underrun occurred.
*
* If the non-blocking behaviour is selected, then routine doesn't wait at all.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
{
@@ -1340,6 +1524,8 @@ snd_pcm_sframes_t snd_pcm_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t
* if a signal or underrun occurred.
*
* If the non-blocking behaviour is selected, then routine doesn't wait at all.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
@@ -1363,27 +1549,43 @@ snd_pcm_sframes_t snd_pcm_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t s
* \return 0 on success otherwise a negative error code
*
* The two PCMs will start/stop/prepare in sync.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_link(snd_pcm_t *pcm1, snd_pcm_t *pcm2)
{
+ int err;
+
assert(pcm1);
assert(pcm2);
+ snd_pcm_lock(pcm1);
if (pcm1->fast_ops->link)
- return pcm1->fast_ops->link(pcm1, pcm2);
- return -ENOSYS;
+ err = pcm1->fast_ops->link(pcm1, pcm2);
+ else
+ err = -ENOSYS;
+ snd_pcm_unlock(pcm1);
+ return err;
}
/**
* \brief Remove a PCM from a linked group
* \param pcm PCM handle
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_unlink(snd_pcm_t *pcm)
{
+ int err;
+
assert(pcm);
+ snd_pcm_lock(pcm);
if (pcm->fast_ops->unlink)
- return pcm->fast_ops->unlink(pcm);
- return -ENOSYS;
+ err = pcm->fast_ops->unlink(pcm);
+ else
+ err = -ENOSYS;
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -1393,12 +1595,24 @@ int snd_pcm_unlink(snd_pcm_t *pcm)
*/
int snd_pcm_poll_descriptors_count(snd_pcm_t *pcm)
{
+ int count;
+
assert(pcm);
+ snd_pcm_lock(pcm);
+ count = __snd_pcm_poll_descriptors_count(pcm);
+ snd_pcm_unlock(pcm);
+ return count;
+}
+
+#ifndef DOXYGEN
+/* locked version */
+int __snd_pcm_poll_descriptors_count(snd_pcm_t *pcm)
+{
if (pcm->fast_ops->poll_descriptors_count)
return pcm->fast_ops->poll_descriptors_count(pcm->fast_op_arg);
return pcm->poll_fd_count;
}
-
+#endif
/**
* \brief get poll descriptors
@@ -1428,7 +1642,20 @@ int snd_pcm_poll_descriptors_count(snd_pcm_t *pcm)
*/
int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int space)
{
+ int err;
+
assert(pcm && pfds);
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_poll_descriptors(pcm, pfds, space);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOXYGEN
+/* locked version */
+int __snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds,
+ unsigned int space)
+{
if (pcm->fast_ops->poll_descriptors)
return pcm->fast_ops->poll_descriptors(pcm->fast_op_arg, pfds, space);
if (pcm->poll_fd < 0) {
@@ -1443,6 +1670,10 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s
}
return 1;
}
+#endif
+
+static int pcm_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds,
+ unsigned int nfds, unsigned short *revents);
/**
* \brief get returned events from poll descriptors
@@ -1462,20 +1693,33 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s
*
* Note: Even if multiple poll descriptors are used (i.e. pfds > 1),
* this function returns only a single event.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_poll_descriptors_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int nfds, unsigned short *revents)
{
+ int err;
+
assert(pcm && pfds && revents);
+ snd_pcm_lock(pcm);
+ err = pcm_poll_revents(pcm, pfds, nfds, revents);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOC_HIDDEN
+static int pcm_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds,
+ unsigned int nfds, unsigned short *revents)
+{
if (pcm->fast_ops->poll_revents)
return pcm->fast_ops->poll_revents(pcm->fast_op_arg, pfds, nfds, revents);
- if (nfds == 1) {
+ if (nfds == 1)
*revents = pfds->revents;
- return 0;
- }
- return -EINVAL;
+ else
+ return -EINVAL;
+ return 0;
}
-#ifndef DOC_HIDDEN
#define PCMTYPE(v) [SND_PCM_TYPE_##v] = #v
#define STATE(v) [SND_PCM_STATE_##v] = #v
#define STREAM(v) [SND_PCM_STREAM_##v] = #v
@@ -1974,12 +2218,16 @@ int snd_pcm_status_dump(snd_pcm_status_t *status, snd_output_t *out)
* \param pcm PCM handle
* \param out Output handle
* \return 0 on success otherwise a negative error code
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_dump(snd_pcm_t *pcm, snd_output_t *out)
{
assert(pcm);
assert(out);
+ snd_pcm_lock(pcm);
pcm->ops->dump(pcm->op_arg, out);
+ snd_pcm_unlock(pcm);
return 0;
}
@@ -2056,6 +2304,8 @@ ssize_t snd_pcm_samples_to_bytes(snd_pcm_t *pcm, long samples)
* \return 0 otherwise a negative error code on failure
*
* The asynchronous callback is called when period boundary elapses.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_async_add_pcm_handler(snd_async_handler_t **handler, snd_pcm_t *pcm,
snd_async_callback_t callback, void *private_data)
@@ -2063,10 +2313,11 @@ int snd_async_add_pcm_handler(snd_async_handler_t **handler, snd_pcm_t *pcm,
int err;
int was_empty;
snd_async_handler_t *h;
+ snd_pcm_lock(pcm);
err = snd_async_add_handler(&h, _snd_pcm_async_descriptor(pcm),
callback, private_data);
if (err < 0)
- return err;
+ goto unlock;
h->type = SND_ASYNC_HANDLER_PCM;
h->u.pcm = pcm;
was_empty = list_empty(&pcm->async_handlers);
@@ -2075,11 +2326,13 @@ int snd_async_add_pcm_handler(snd_async_handler_t **handler, snd_pcm_t *pcm,
err = snd_pcm_async(pcm, snd_async_handler_get_signo(h), getpid());
if (err < 0) {
snd_async_del_handler(h);
- return err;
+ goto unlock;
}
}
*handler = h;
- return 0;
+ unlock:
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -2359,6 +2612,9 @@ int snd_pcm_new(snd_pcm_t **pcmp, snd_pcm_type_t type, const char *name,
pcm->op_arg = pcm;
pcm->fast_op_arg = pcm;
INIT_LIST_HEAD(&pcm->async_handlers);
+#ifdef THREAD_SAFE_API
+ pthread_mutex_init(&pcm->lock, NULL);
+#endif
*pcmp = pcm;
return 0;
}
@@ -2370,6 +2626,9 @@ int snd_pcm_free(snd_pcm_t *pcm)
free(pcm->hw.link_dst);
free(pcm->appl.link_dst);
snd_dlobj_cache_put(pcm->open_func);
+#ifdef THREAD_SAFE_API
+ pthread_mutex_destroy(&pcm->lock);
+#endif
free(pcm);
return 0;
}
@@ -2401,12 +2660,26 @@ int snd_pcm_open_named_slave(snd_pcm_t **pcmp, const char *name,
* others for general errors)
* \retval 0 timeout occurred
* \retval 1 PCM stream is ready for I/O
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
{
+ int err;
+
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_wait_in_lock(pcm, timeout);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOC_HIDDEN
+/* locked version */
+int __snd_pcm_wait_in_lock(snd_pcm_t *pcm, int timeout)
+{
if (!snd_pcm_may_wait_for_avail_min(pcm, snd_pcm_mmap_avail(pcm))) {
/* check more precisely */
- switch (snd_pcm_state(pcm)) {
+ switch (__snd_pcm_state(pcm)) {
case SND_PCM_STATE_XRUN:
return -EPIPE;
case SND_PCM_STATE_SUSPENDED:
@@ -2420,11 +2693,12 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
return snd_pcm_wait_nocheck(pcm, timeout);
}
-#ifndef DOC_HIDDEN
/*
* like snd_pcm_wait() but doesn't check mmap_avail before calling poll()
*
* used in drain code in some plugins
+ *
+ * This function is called inside pcm lock.
*/
int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
{
@@ -2432,13 +2706,13 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
unsigned short revents = 0;
int npfds, err, err_poll;
- npfds = snd_pcm_poll_descriptors_count(pcm);
+ npfds = __snd_pcm_poll_descriptors_count(pcm);
if (npfds <= 0 || npfds >= 16) {
SNDERR("Invalid poll_fds %d\n", npfds);
return -EIO;
}
pfd = alloca(sizeof(*pfd) * npfds);
- err = snd_pcm_poll_descriptors(pcm, pfd, npfds);
+ err = __snd_pcm_poll_descriptors(pcm, pfd, npfds);
if (err < 0)
return err;
if (err != npfds) {
@@ -2446,7 +2720,9 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
return -EIO;
}
do {
+ snd_pcm_unlock(pcm);
err_poll = poll(pfd, npfds, timeout);
+ snd_pcm_lock(pcm);
if (err_poll < 0) {
if (errno == EINTR && !PCMINABORT(pcm))
continue;
@@ -2454,12 +2730,12 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
}
if (! err_poll)
break;
- err = snd_pcm_poll_descriptors_revents(pcm, pfd, npfds, &revents);
+ err = pcm_poll_revents(pcm, pfd, npfds, &revents);
if (err < 0)
return err;
if (revents & (POLLERR | POLLNVAL)) {
/* check more precisely */
- switch (snd_pcm_state(pcm)) {
+ switch (__snd_pcm_state(pcm)) {
case SND_PCM_STATE_XRUN:
return -EPIPE;
case SND_PCM_STATE_SUSPENDED:
@@ -2474,8 +2750,8 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
#if 0 /* very useful code to test poll related problems */
{
snd_pcm_sframes_t avail_update;
- snd_pcm_hwsync(pcm);
- avail_update = snd_pcm_avail_update(pcm);
+ pcm_hwsync(pcm);
+ avail_update = pcm_avail_update(pcm);
if (avail_update < (snd_pcm_sframes_t)pcm->avail_min) {
printf("*** snd_pcm_wait() FATAL ERROR!!!\n");
printf("avail_min = %li, avail_update = %li\n", pcm->avail_min, avail_update);
@@ -2506,10 +2782,17 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
* Also this function might be called after #snd_pcm_delay() or
* #snd_pcm_hwsync() functions to move private ring buffer pointers
* in alsa-lib (the internal plugin chain).
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_avail_update(snd_pcm_t *pcm)
{
- return pcm->fast_ops->avail_update(pcm->fast_op_arg);
+ snd_pcm_sframes_t result;
+
+ snd_pcm_lock(pcm);
+ result = __snd_pcm_avail_update(pcm);
+ snd_pcm_unlock(pcm);
+ return result;
}
/**
@@ -2523,20 +2806,27 @@ snd_pcm_sframes_t snd_pcm_avail_update(snd_pcm_t *pcm)
*
* The position is synced with hardware (driver) position in the sound
* ring buffer in this functions.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_avail(snd_pcm_t *pcm)
{
int err;
+ snd_pcm_sframes_t result;
assert(pcm);
if (CHECK_SANITY(! pcm->setup)) {
SNDMSG("PCM not set up");
return -EIO;
}
- err = pcm->fast_ops->hwsync(pcm->fast_op_arg);
+ snd_pcm_lock(pcm);
+ err = pcm_hwsync(pcm);
if (err < 0)
- return err;
- return pcm->fast_ops->avail_update(pcm->fast_op_arg);
+ result = err;
+ else
+ result = __snd_pcm_avail_update(pcm);
+ snd_pcm_unlock(pcm);
+ return result;
}
/**
@@ -2547,6 +2837,8 @@ snd_pcm_sframes_t snd_pcm_avail(snd_pcm_t *pcm)
* \return zero on success otherwise a negative error code
*
* The avail and delay values retuned are in sync.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_avail_delay(snd_pcm_t *pcm,
snd_pcm_sframes_t *availp,
@@ -2560,17 +2852,23 @@ int snd_pcm_avail_delay(snd_pcm_t *pcm,
SNDMSG("PCM not set up");
return -EIO;
}
+ snd_pcm_lock(pcm);
err = pcm->fast_ops->hwsync(pcm->fast_op_arg);
if (err < 0)
- return err;
+ goto unlock;
sf = pcm->fast_ops->avail_update(pcm->fast_op_arg);
- if (sf < 0)
- return (int)sf;
+ if (sf < 0) {
+ err = (int)sf;
+ goto unlock;
+ }
err = pcm->fast_ops->delay(pcm->fast_op_arg, delayp);
if (err < 0)
- return err;
+ goto unlock;
*availp = sf;
- return 0;
+ err = 0;
+ unlock:
+ snd_pcm_unlock(pcm);
+ return err;
}
/**
@@ -6680,12 +6978,27 @@ void snd_pcm_info_set_stream(snd_pcm_info_t *obj, snd_pcm_stream_t val)
*
* See the snd_pcm_mmap_commit() function to finish the frame processing in
* the direct areas.
+ *
+ * The function is thread-safe when built with the proper option.
*/
int snd_pcm_mmap_begin(snd_pcm_t *pcm,
const snd_pcm_channel_area_t **areas,
snd_pcm_uframes_t *offset,
snd_pcm_uframes_t *frames)
{
+ int err;
+
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_mmap_begin(pcm, areas, offset, frames);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
+#ifndef DOXYGEN
+/* locked version */
+int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas,
+ snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames)
+{
snd_pcm_uframes_t cont;
snd_pcm_uframes_t f;
snd_pcm_uframes_t avail;
@@ -6708,6 +7021,7 @@ int snd_pcm_mmap_begin(snd_pcm_t *pcm,
*frames = f;
return 0;
}
+#endif
/**
* \brief Application has completed the access to area requested with #snd_pcm_mmap_begin
@@ -6760,11 +7074,27 @@ int snd_pcm_mmap_begin(snd_pcm_t *pcm,
*
* Look to the \ref example_test_pcm "Sine-wave generator" example
* for more details about the generate_sine function.
+ *
+ * The function is thread-safe when built with the proper option.
*/
snd_pcm_sframes_t snd_pcm_mmap_commit(snd_pcm_t *pcm,
snd_pcm_uframes_t offset,
snd_pcm_uframes_t frames)
{
+ snd_pcm_sframes_t result;
+
+ snd_pcm_lock(pcm);
+ result = __snd_pcm_mmap_commit(pcm, offset, frames);
+ snd_pcm_unlock(pcm);
+ return result;
+}
+
+#ifndef DOXYGEN
+/* locked version*/
+snd_pcm_sframes_t __snd_pcm_mmap_commit(snd_pcm_t *pcm,
+ snd_pcm_uframes_t offset,
+ snd_pcm_uframes_t frames)
+{
assert(pcm);
if (CHECK_SANITY(offset != *pcm->appl.ptr % pcm->buffer_size)) {
SNDMSG("commit offset (%ld) doesn't match with appl_ptr (%ld) %% buf_size (%ld)",
@@ -6779,8 +7109,6 @@ snd_pcm_sframes_t snd_pcm_mmap_commit(snd_pcm_t *pcm,
return pcm->fast_ops->mmap_commit(pcm->fast_op_arg, offset, frames);
}
-#ifndef DOC_HIDDEN
-
int _snd_pcm_poll_descriptor(snd_pcm_t *pcm)
{
assert(pcm);
@@ -6791,24 +7119,32 @@ void snd_pcm_areas_from_buf(snd_pcm_t *pcm, snd_pcm_channel_area_t *areas,
void *buf)
{
unsigned int channel;
- unsigned int channels = pcm->channels;
+ unsigned int channels;
+
+ snd_pcm_lock(pcm);
+ channels = pcm->channels;
for (channel = 0; channel < channels; ++channel, ++areas) {
areas->addr = buf;
areas->first = channel * pcm->sample_bits;
areas->step = pcm->frame_bits;
}
+ snd_pcm_unlock(pcm);
}
void snd_pcm_areas_from_bufs(snd_pcm_t *pcm, snd_pcm_channel_area_t *areas,
void **bufs)
{
unsigned int channel;
- unsigned int channels = pcm->channels;
+ unsigned int channels;
+
+ snd_pcm_lock(pcm);
+ channels = pcm->channels;
for (channel = 0; channel < channels; ++channel, ++areas, ++bufs) {
areas->addr = *bufs;
areas->first = 0;
areas->step = pcm->sample_bits;
}
+ snd_pcm_unlock(pcm);
}
snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_t *areas,
@@ -6822,19 +7158,20 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
if (size == 0)
return 0;
+ snd_pcm_lock(pcm);
while (size > 0) {
snd_pcm_uframes_t frames;
snd_pcm_sframes_t avail;
_again:
- state = snd_pcm_state(pcm);
+ state = __snd_pcm_state(pcm);
switch (state) {
case SND_PCM_STATE_PREPARED:
- err = snd_pcm_start(pcm);
+ err = pcm_start(pcm);
if (err < 0)
goto _end;
break;
case SND_PCM_STATE_RUNNING:
- err = snd_pcm_hwsync(pcm);
+ err = pcm_hwsync(pcm);
if (err < 0)
goto _end;
break;
@@ -6854,7 +7191,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
err = -EBADFD;
goto _end;
}
- avail = snd_pcm_avail_update(pcm);
+ avail = __snd_pcm_avail_update(pcm);
if (avail < 0) {
err = avail;
goto _end;
@@ -6867,7 +7204,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
goto _end;
}
- err = snd_pcm_wait(pcm, -1);
+ err = __snd_pcm_wait_in_lock(pcm, -1);
if (err < 0)
break;
goto _again;
@@ -6887,6 +7224,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_
xfer += frames;
}
_end:
+ snd_pcm_unlock(pcm);
return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err);
}
@@ -6901,17 +7239,18 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
if (size == 0)
return 0;
+ snd_pcm_lock(pcm);
while (size > 0) {
snd_pcm_uframes_t frames;
snd_pcm_sframes_t avail;
_again:
- state = snd_pcm_state(pcm);
+ state = __snd_pcm_state(pcm);
switch (state) {
case SND_PCM_STATE_PREPARED:
case SND_PCM_STATE_PAUSED:
break;
case SND_PCM_STATE_RUNNING:
- err = snd_pcm_hwsync(pcm);
+ err = pcm_hwsync(pcm);
if (err < 0)
goto _end;
break;
@@ -6928,7 +7267,7 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
err = -EBADFD;
goto _end;
}
- avail = snd_pcm_avail_update(pcm);
+ avail = __snd_pcm_avail_update(pcm);
if (avail < 0) {
err = avail;
goto _end;
@@ -6959,10 +7298,10 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
snd_pcm_sframes_t hw_avail = pcm->buffer_size - avail;
hw_avail += frames;
/* some plugins might automatically start the stream */
- state = snd_pcm_state(pcm);
+ state = __snd_pcm_state(pcm);
if (state == SND_PCM_STATE_PREPARED &&
hw_avail >= (snd_pcm_sframes_t) pcm->start_threshold) {
- err = snd_pcm_start(pcm);
+ err = pcm_start(pcm);
if (err < 0)
goto _end;
}
@@ -6972,6 +7311,7 @@ snd_pcm_sframes_t snd_pcm_write_areas(snd_pcm_t *pcm, const snd_pcm_channel_area
xfer += frames;
}
_end:
+ snd_pcm_unlock(pcm);
return xfer > 0 ? (snd_pcm_sframes_t) xfer : snd_pcm_check_error(pcm, err);
}
diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 21f98e7a1779..ecc86db4831a 100644
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -559,7 +559,7 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
events = pfds[0].revents;
if (events & POLLIN) {
snd_pcm_uframes_t avail;
- snd_pcm_avail_update(pcm);
+ __snd_pcm_avail_update(pcm);
if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
events |= POLLOUT;
events &= ~POLLIN;
@@ -580,7 +580,7 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
snd_pcm_direct_clear_timer_queue(dmix);
events &= ~(POLLOUT|POLLIN);
/* additional check */
- switch (snd_pcm_state(pcm)) {
+ switch (__snd_pcm_state(pcm)) {
case SND_PCM_STATE_XRUN:
case SND_PCM_STATE_SUSPENDED:
case SND_PCM_STATE_SETUP:
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index 007d35664ce7..2714fb93c758 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -601,7 +601,8 @@ static int snd_pcm_dmix_drop(snd_pcm_t *pcm)
return 0;
}
-static int snd_pcm_dmix_drain(snd_pcm_t *pcm)
+/* locked version */
+static int __snd_pcm_dmix_drain(snd_pcm_t *pcm)
{
snd_pcm_direct_t *dmix = pcm->private_data;
snd_pcm_uframes_t stop_threshold;
@@ -659,6 +660,16 @@ static int snd_pcm_dmix_drain(snd_pcm_t *pcm)
return 0;
}
+static int snd_pcm_dmix_drain(snd_pcm_t *pcm)
+{
+ int err;
+
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_dmix_drain(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
static int snd_pcm_dmix_pause(snd_pcm_t *pcm ATTRIBUTE_UNUSED, int enable ATTRIBUTE_UNUSED)
{
return -EIO;
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index 05854dedf259..c5b3178a4990 100644
--- a/src/pcm/pcm_dshare.c
+++ b/src/pcm/pcm_dshare.c
@@ -354,7 +354,8 @@ static int snd_pcm_dshare_drop(snd_pcm_t *pcm)
return 0;
}
-static int snd_pcm_dshare_drain(snd_pcm_t *pcm)
+/* locked version */
+static int __snd_pcm_dshare_drain(snd_pcm_t *pcm)
{
snd_pcm_direct_t *dshare = pcm->private_data;
snd_pcm_uframes_t stop_threshold;
@@ -412,6 +413,16 @@ static int snd_pcm_dshare_drain(snd_pcm_t *pcm)
return 0;
}
+static int snd_pcm_dshare_drain(snd_pcm_t *pcm)
+{
+ int err;
+
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_dshare_drain(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
static int snd_pcm_dshare_pause(snd_pcm_t *pcm ATTRIBUTE_UNUSED, int enable ATTRIBUTE_UNUSED)
{
return -EIO;
diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
index 2d45171dda01..4efbc53d177e 100644
--- a/src/pcm/pcm_dsnoop.c
+++ b/src/pcm/pcm_dsnoop.c
@@ -297,7 +297,8 @@ static int snd_pcm_dsnoop_drop(snd_pcm_t *pcm)
return 0;
}
-static int snd_pcm_dsnoop_drain(snd_pcm_t *pcm)
+/* locked version */
+static int __snd_pcm_dsnoop_drain(snd_pcm_t *pcm)
{
snd_pcm_direct_t *dsnoop = pcm->private_data;
snd_pcm_uframes_t stop_threshold;
@@ -314,12 +315,22 @@ static int snd_pcm_dsnoop_drain(snd_pcm_t *pcm)
break;
if (pcm->mode & SND_PCM_NONBLOCK)
return -EAGAIN;
- snd_pcm_wait(pcm, -1);
+ __snd_pcm_wait_in_lock(pcm, -1);
}
pcm->stop_threshold = stop_threshold;
return snd_pcm_dsnoop_drop(pcm);
}
+static int snd_pcm_dsnoop_drain(snd_pcm_t *pcm)
+{
+ int err;
+
+ snd_pcm_lock(pcm);
+ err = __snd_pcm_dsnoop_drain(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
+}
+
static int snd_pcm_dsnoop_pause(snd_pcm_t *pcm ATTRIBUTE_UNUSED, int enable ATTRIBUTE_UNUSED)
{
return -EIO;
diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c
index 92eb0724c0e2..b1821f944240 100644
--- a/src/pcm/pcm_file.c
+++ b/src/pcm/pcm_file.c
@@ -440,13 +440,16 @@ static int snd_pcm_file_drop(snd_pcm_t *pcm)
return err;
}
+/* locking */
static int snd_pcm_file_drain(snd_pcm_t *pcm)
{
snd_pcm_file_t *file = pcm->private_data;
int err = snd_pcm_drain(file->gen.slave);
if (err >= 0) {
+ snd_pcm_lock(pcm);
snd_pcm_file_write_bytes(pcm, file->wbuf_used_bytes);
assert(file->wbuf_used_bytes == 0);
+ snd_pcm_unlock(pcm);
}
return err;
}
@@ -507,40 +510,49 @@ static snd_pcm_sframes_t snd_pcm_file_forward(snd_pcm_t *pcm, snd_pcm_uframes_t
return err;
}
+/* locking */
static snd_pcm_sframes_t snd_pcm_file_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size)
{
snd_pcm_file_t *file = pcm->private_data;
snd_pcm_channel_area_t areas[pcm->channels];
- snd_pcm_sframes_t n = snd_pcm_writei(file->gen.slave, buffer, size);
+ snd_pcm_sframes_t n = _snd_pcm_writei(file->gen.slave, buffer, size);
if (n > 0) {
snd_pcm_areas_from_buf(pcm, areas, (void*) buffer);
+ snd_pcm_lock(pcm);
snd_pcm_file_add_frames(pcm, areas, 0, n);
+ snd_pcm_unlock(pcm);
}
return n;
}
+/* locking */
static snd_pcm_sframes_t snd_pcm_file_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
snd_pcm_file_t *file = pcm->private_data;
snd_pcm_channel_area_t areas[pcm->channels];
- snd_pcm_sframes_t n = snd_pcm_writen(file->gen.slave, bufs, size);
+ snd_pcm_sframes_t n = _snd_pcm_writen(file->gen.slave, bufs, size);
if (n > 0) {
snd_pcm_areas_from_bufs(pcm, areas, bufs);
+ snd_pcm_lock(pcm);
snd_pcm_file_add_frames(pcm, areas, 0, n);
+ snd_pcm_unlock(pcm);
}
return n;
}
+/* locking */
static snd_pcm_sframes_t snd_pcm_file_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
{
snd_pcm_file_t *file = pcm->private_data;
snd_pcm_sframes_t n;
- n = snd_pcm_readi(file->gen.slave, buffer, size);
+ n = _snd_pcm_readi(file->gen.slave, buffer, size);
if (n <= 0)
return n;
if (file->ifd >= 0) {
+ snd_pcm_lock(pcm);
n = read(file->ifd, buffer, n * pcm->frame_bits / 8);
+ snd_pcm_unlock(pcm);
if (n < 0)
return n;
return n * 8 / pcm->frame_bits;
@@ -548,6 +560,7 @@ static snd_pcm_sframes_t snd_pcm_file_readi(snd_pcm_t *pcm, void *buffer, snd_pc
return n;
}
+/* locking */
static snd_pcm_sframes_t snd_pcm_file_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
snd_pcm_file_t *file = pcm->private_data;
@@ -558,7 +571,7 @@ static snd_pcm_sframes_t snd_pcm_file_readn(snd_pcm_t *pcm, void **bufs, snd_pcm
return 0; /* TODO: Noninterleaved read */
}
- n = snd_pcm_readn(file->gen.slave, bufs, size);
+ n = _snd_pcm_readn(file->gen.slave, bufs, size);
return n;
}
diff --git a/src/pcm/pcm_generic.c b/src/pcm/pcm_generic.c
index 4dbef08cc2c7..12102c40a372 100644
--- a/src/pcm/pcm_generic.c
+++ b/src/pcm/pcm_generic.c
@@ -235,25 +235,25 @@ int snd_pcm_generic_unlink(snd_pcm_t *pcm)
snd_pcm_sframes_t snd_pcm_generic_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size)
{
snd_pcm_generic_t *generic = pcm->private_data;
- return snd_pcm_writei(generic->slave, buffer, size);
+ return _snd_pcm_writei(generic->slave, buffer, size);
}
snd_pcm_sframes_t snd_pcm_generic_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
snd_pcm_generic_t *generic = pcm->private_data;
- return snd_pcm_writen(generic->slave, bufs, size);
+ return _snd_pcm_writen(generic->slave, bufs, size);
}
snd_pcm_sframes_t snd_pcm_generic_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
{
snd_pcm_generic_t *generic = pcm->private_data;
- return snd_pcm_readi(generic->slave, buffer, size);
+ return _snd_pcm_readi(generic->slave, buffer, size);
}
snd_pcm_sframes_t snd_pcm_generic_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
snd_pcm_generic_t *generic = pcm->private_data;
- return snd_pcm_readn(generic->slave, bufs, size);
+ return _snd_pcm_readn(generic->slave, bufs, size);
}
snd_pcm_sframes_t snd_pcm_generic_mmap_commit(snd_pcm_t *pcm,
@@ -287,7 +287,7 @@ int snd_pcm_generic_real_htimestamp(snd_pcm_t *pcm, snd_pcm_uframes_t *avail,
int ok = 0;
while (1) {
- avail1 = snd_pcm_avail_update(pcm);
+ avail1 = __snd_pcm_avail_update(pcm);
if (avail1 < 0)
return avail1;
if (ok && (snd_pcm_uframes_t)avail1 == *avail)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index 4f4b84b2d2bc..d4045f8efc4f 100644
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -1505,6 +1505,9 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name,
pcm->poll_fd = fd;
pcm->poll_events = info.stream == SND_PCM_STREAM_PLAYBACK ? POLLOUT : POLLIN;
pcm->tstamp_type = tstamp_type;
+#ifdef THREAD_SAFE_API
+ pcm->thread_safe = 1;
+#endif
ret = snd_pcm_hw_mmap_status(pcm);
if (ret < 0) {
diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c
index 43550c03875b..98daa8dea70d 100644
--- a/src/pcm/pcm_ioplug.c
+++ b/src/pcm/pcm_ioplug.c
@@ -469,15 +469,20 @@ static int snd_pcm_ioplug_drop(snd_pcm_t *pcm)
return 0;
}
+/* need locking */
static int snd_pcm_ioplug_drain(snd_pcm_t *pcm)
{
ioplug_priv_t *io = pcm->private_data;
+ int err;
if (io->data->state == SND_PCM_STATE_OPEN)
return -EBADFD;
if (io->data->callback->drain)
io->data->callback->drain(io->data);
- return snd_pcm_ioplug_drop(pcm);
+ snd_pcm_lock(pcm);
+ err = snd_pcm_ioplug_drop(pcm);
+ snd_pcm_unlock(pcm);
+ return err;
}
static int snd_pcm_ioplug_pause(snd_pcm_t *pcm, int enable)
@@ -609,7 +614,7 @@ static snd_pcm_sframes_t snd_pcm_ioplug_mmap_commit(snd_pcm_t *pcm,
const snd_pcm_channel_area_t *areas;
snd_pcm_uframes_t ofs, frames = size;
- snd_pcm_mmap_begin(pcm, &areas, &ofs, &frames);
+ __snd_pcm_mmap_begin(pcm, &areas, &ofs, &frames);
if (ofs != offset)
return -EIO;
return ioplug_priv_transfer_areas(pcm, areas, offset, frames);
@@ -635,7 +640,7 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm)
snd_pcm_uframes_t offset, size = UINT_MAX;
snd_pcm_sframes_t result;
- snd_pcm_mmap_begin(pcm, &areas, &offset, &size);
+ __snd_pcm_mmap_begin(pcm, &areas, &offset, &size);
result = io->data->callback->transfer(io->data, areas, offset, size);
if (result < 0)
return result;
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 326618ecd0c0..126e2b6ece8f 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -34,6 +34,10 @@
#include "local.h"
+#ifdef THREAD_SAFE_API
+#include <pthread.h>
+#endif
+
#define SND_INTERVAL_INLINE
#include "interval.h"
@@ -155,12 +159,12 @@ typedef struct {
int (*reset)(snd_pcm_t *pcm);
int (*start)(snd_pcm_t *pcm);
int (*drop)(snd_pcm_t *pcm);
- int (*drain)(snd_pcm_t *pcm);
+ int (*drain)(snd_pcm_t *pcm); /* need own locking */
int (*pause)(snd_pcm_t *pcm, int enable);
snd_pcm_state_t (*state)(snd_pcm_t *pcm);
int (*hwsync)(snd_pcm_t *pcm);
int (*delay)(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp);
- int (*resume)(snd_pcm_t *pcm);
+ int (*resume)(snd_pcm_t *pcm); /* need own locking */
int (*link)(snd_pcm_t *pcm1, snd_pcm_t *pcm2);
int (*link_slaves)(snd_pcm_t *pcm, snd_pcm_t *master);
int (*unlink)(snd_pcm_t *pcm);
@@ -168,10 +172,10 @@ typedef struct {
snd_pcm_sframes_t (*rewind)(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
snd_pcm_sframes_t (*forwardable)(snd_pcm_t *pcm);
snd_pcm_sframes_t (*forward)(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
- snd_pcm_sframes_t (*writei)(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size);
- snd_pcm_sframes_t (*writen)(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size);
- snd_pcm_sframes_t (*readi)(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size);
- snd_pcm_sframes_t (*readn)(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size);
+ snd_pcm_sframes_t (*writei)(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size); /* need own locking */
+ snd_pcm_sframes_t (*writen)(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size); /* need own locking */
+ snd_pcm_sframes_t (*readi)(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size); /* need own locking */
+ snd_pcm_sframes_t (*readn)(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size); /* need own locking */
snd_pcm_sframes_t (*avail_update)(snd_pcm_t *pcm);
snd_pcm_sframes_t (*mmap_commit)(snd_pcm_t *pcm, snd_pcm_uframes_t offset, snd_pcm_uframes_t size);
int (*htimestamp)(snd_pcm_t *pcm, snd_pcm_uframes_t *avail, snd_htimestamp_t *tstamp);
@@ -239,6 +243,10 @@ struct _snd_pcm {
snd_pcm_t *fast_op_arg;
void *private_data;
struct list_head async_handlers;
+#ifdef THREAD_SAFE_API
+ int thread_safe;
+ pthread_mutex_t lock;
+#endif
};
/* make local functions really local */
@@ -401,11 +409,24 @@ int _snd_pcm_poll_descriptor(snd_pcm_t *pcm);
#define _snd_pcm_link_descriptor _snd_pcm_poll_descriptor /* FIXME */
#define _snd_pcm_async_descriptor _snd_pcm_poll_descriptor /* FIXME */
+/* locked versions */
+snd_pcm_state_t __snd_pcm_state(snd_pcm_t *pcm);
+int __snd_pcm_poll_descriptors_count(snd_pcm_t *pcm);
+int __snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds,
+ unsigned int space);
+int __snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params);
+int __snd_pcm_mmap_begin(snd_pcm_t *pcm, const snd_pcm_channel_area_t **areas,
+ snd_pcm_uframes_t *offset, snd_pcm_uframes_t *frames);
+snd_pcm_sframes_t __snd_pcm_mmap_commit(snd_pcm_t *pcm,
+ snd_pcm_uframes_t offset,
+ snd_pcm_uframes_t frames);
+int __snd_pcm_wait_in_lock(snd_pcm_t *pcm, int timeout);
+
/* handle special error cases */
static inline int snd_pcm_check_error(snd_pcm_t *pcm, int err)
{
if (err == -EINTR) {
- switch (snd_pcm_state(pcm)) {
+ switch (__snd_pcm_state(pcm)) {
case SND_PCM_STATE_XRUN:
return -EPIPE;
case SND_PCM_STATE_SUSPENDED:
@@ -483,7 +504,7 @@ static inline snd_pcm_uframes_t snd_pcm_mmap_hw_rewindable(snd_pcm_t *pcm)
static inline const snd_pcm_channel_area_t *snd_pcm_mmap_areas(snd_pcm_t *pcm)
{
if (pcm->stopped_areas &&
- snd_pcm_state(pcm) != SND_PCM_STATE_RUNNING)
+ __snd_pcm_state(pcm) != SND_PCM_STATE_RUNNING)
return pcm->stopped_areas;
return pcm->running_areas;
}
@@ -533,21 +554,25 @@ static inline unsigned int snd_pcm_channel_area_step(const snd_pcm_channel_area_
static inline snd_pcm_sframes_t _snd_pcm_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size)
{
+ /* lock handled in the callback */
return pcm->fast_ops->writei(pcm->fast_op_arg, buffer, size);
}
static inline snd_pcm_sframes_t _snd_pcm_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
+ /* lock handled in the callback */
return pcm->fast_ops->writen(pcm->fast_op_arg, bufs, size);
}
static inline snd_pcm_sframes_t _snd_pcm_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
{
+ /* lock handled in the callback */
return pcm->fast_ops->readi(pcm->fast_op_arg, buffer, size);
}
static inline snd_pcm_sframes_t _snd_pcm_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
{
+ /* lock handled in the callback */
return pcm->fast_ops->readn(pcm->fast_op_arg, bufs, size);
}
@@ -1016,6 +1041,11 @@ _snd_pcm_parse_config_chmaps(snd_config_t *conf);
snd_pcm_chmap_t *
_snd_pcm_choose_fixed_chmap(snd_pcm_t *pcm, snd_pcm_chmap_query_t * const *maps);
+static inline snd_pcm_sframes_t __snd_pcm_avail_update(snd_pcm_t *pcm)
+{
+ return pcm->fast_ops->avail_update(pcm->fast_op_arg);
+}
+
/* return true if the PCM stream may wait to get avail_min space */
static inline int snd_pcm_may_wait_for_avail_min(snd_pcm_t *pcm, snd_pcm_uframes_t avail)
{
@@ -1038,3 +1068,19 @@ static inline void sw_set_period_event(snd_pcm_sw_params_t *params, int val)
}
#define PCMINABORT(pcm) (((pcm)->mode & SND_PCM_ABORT) != 0)
+
+#ifdef THREAD_SAFE_API
+static inline void snd_pcm_lock(snd_pcm_t *pcm)
+{
+ if (!pcm->thread_safe)
+ pthread_mutex_lock(&pcm->lock);
+}
+static inline void snd_pcm_unlock(snd_pcm_t *pcm)
+{
+ if (!pcm->thread_safe)
+ pthread_mutex_unlock(&pcm->lock);
+}
+#else /* THREAD_SAFE_API */
+#define snd_pcm_lock(pcm) do {} while (0)
+#define snd_pcm_unlock(pcm) do {} while (0)
+#endif /* THREAD_SAFE_API */
diff --git a/src/pcm/pcm_mmap.c b/src/pcm/pcm_mmap.c
index 5c4fbe1705eb..6c230a144559 100644
--- a/src/pcm/pcm_mmap.c
+++ b/src/pcm/pcm_mmap.c
@@ -82,12 +82,12 @@ static snd_pcm_sframes_t snd_pcm_mmap_write_areas(snd_pcm_t *pcm,
snd_pcm_uframes_t frames = size;
snd_pcm_sframes_t result;
- snd_pcm_mmap_begin(pcm, &pcm_areas, &pcm_offset, &frames);
+ __snd_pcm_mmap_begin(pcm, &pcm_areas, &pcm_offset, &frames);
snd_pcm_areas_copy(pcm_areas, pcm_offset,
areas, offset,
pcm->channels,
frames, pcm->format);
- result = snd_pcm_mmap_commit(pcm, pcm_offset, frames);
+ result = __snd_pcm_mmap_commit(pcm, pcm_offset, frames);
if (result < 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
offset += result;
@@ -114,12 +114,12 @@ static snd_pcm_sframes_t snd_pcm_mmap_read_areas(snd_pcm_t *pcm,
snd_pcm_uframes_t frames = size;
snd_pcm_sframes_t result;
- snd_pcm_mmap_begin(pcm, &pcm_areas, &pcm_offset, &frames);
+ __snd_pcm_mmap_begin(pcm, &pcm_areas, &pcm_offset, &frames);
snd_pcm_areas_copy(areas, offset,
pcm_areas, pcm_offset,
pcm->channels,
frames, pcm->format);
- result = snd_pcm_mmap_commit(pcm, pcm_offset, frames);
+ result = __snd_pcm_mmap_commit(pcm, pcm_offset, frames);
if (result < 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : result;
offset += result;
@@ -256,6 +256,7 @@ int snd_pcm_channel_info_shm(snd_pcm_t *pcm, snd_pcm_channel_info_t *info, int s
return 0;
}
+/* called in pcm lock */
int snd_pcm_mmap(snd_pcm_t *pcm)
{
int err;
@@ -435,6 +436,7 @@ int snd_pcm_mmap(snd_pcm_t *pcm)
return 0;
}
+/* called in pcm lock */
int snd_pcm_munmap(snd_pcm_t *pcm)
{
int err;
@@ -513,6 +515,7 @@ int snd_pcm_munmap(snd_pcm_t *pcm)
return 0;
}
+/* called in pcm lock */
snd_pcm_sframes_t snd_pcm_write_mmap(snd_pcm_t *pcm, snd_pcm_uframes_t offset,
snd_pcm_uframes_t size)
{
@@ -545,7 +548,9 @@ snd_pcm_sframes_t snd_pcm_write_mmap(snd_pcm_t *pcm, snd_pcm_uframes_t offset,
const snd_pcm_channel_area_t *a = &areas[c];
bufs[c] = snd_pcm_channel_area_addr(a, offset);
}
+ snd_pcm_unlock(pcm);
err = _snd_pcm_writen(pcm, bufs, frames);
+ snd_pcm_lock(pcm);
if (err >= 0)
frames = err;
break;
@@ -564,6 +569,7 @@ snd_pcm_sframes_t snd_pcm_write_mmap(snd_pcm_t *pcm, snd_pcm_uframes_t offset,
return err;
}
+/* called in pcm lock */
snd_pcm_sframes_t snd_pcm_read_mmap(snd_pcm_t *pcm, snd_pcm_uframes_t offset,
snd_pcm_uframes_t size)
{
@@ -596,7 +602,9 @@ snd_pcm_sframes_t snd_pcm_read_mmap(snd_pcm_t *pcm, snd_pcm_uframes_t offset,
const snd_pcm_channel_area_t *a = &areas[c];
bufs[c] = snd_pcm_channel_area_addr(a, offset);
}
+ snd_pcm_unlock(pcm);
err = _snd_pcm_readn(pcm->fast_op_arg, bufs, frames);
+ snd_pcm_lock(pcm);
if (err >= 0)
frames = err;
break;
diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c
index 60d99ad96167..33e04e38cb7f 100644
--- a/src/pcm/pcm_params.c
+++ b/src/pcm/pcm_params.c
@@ -2360,7 +2360,7 @@ int _snd_pcm_hw_params_internal(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
/* Default sw params */
memset(&sw, 0, sizeof(sw));
snd_pcm_sw_params_default(pcm, &sw);
- err = snd_pcm_sw_params(pcm, &sw);
+ err = __snd_pcm_sw_params(pcm, &sw);
if (err < 0)
return err;
diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 41bddac1a83f..0fe466647a55 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -1004,6 +1004,7 @@ static int snd_pcm_rate_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsign
return snd_pcm_poll_descriptors_revents(rate->gen.slave, pfds, nfds, revents);
}
+/* locking */
static int snd_pcm_rate_drain(snd_pcm_t *pcm)
{
snd_pcm_rate_t *rate = pcm->private_data;
@@ -1013,6 +1014,7 @@ static int snd_pcm_rate_drain(snd_pcm_t *pcm)
snd_pcm_uframes_t size, ofs, saved_avail_min;
snd_pcm_sw_params_t sw_params;
+ snd_pcm_lock(pcm);
/* temporarily set avail_min to one */
sw_params = rate->sw_params;
saved_avail_min = sw_params.avail_min;
@@ -1023,8 +1025,10 @@ static int snd_pcm_rate_drain(snd_pcm_t *pcm)
ofs = rate->last_commit_ptr % pcm->buffer_size;
while (size > 0) {
snd_pcm_uframes_t psize, spsize;
+ int err;
- if (snd_pcm_wait(rate->gen.slave, -1) < 0)
+ err = __snd_pcm_wait_in_lock(rate->gen.slave, -1);
+ if (err < 0)
break;
if (size > pcm->period_size) {
psize = pcm->period_size;
@@ -1042,6 +1046,7 @@ static int snd_pcm_rate_drain(snd_pcm_t *pcm)
}
sw_params.avail_min = saved_avail_min;
snd_pcm_sw_params(rate->gen.slave, &sw_params);
+ snd_pcm_unlock(pcm);
}
return snd_pcm_drain(rate->gen.slave);
}
diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c
index 361160302be8..508d5b0fc3d2 100644
--- a/src/pcm/pcm_route.c
+++ b/src/pcm/pcm_route.c
@@ -877,7 +877,7 @@ static int route_chmap_init(snd_pcm_t *pcm)
snd_pcm_route_t *route = pcm->private_data;
if (!route->chmap)
return 0;
- if (snd_pcm_state(pcm) != SND_PCM_STATE_PREPARED)
+ if (__snd_pcm_state(pcm) != SND_PCM_STATE_PREPARED)
return 0;
/* Check if we really need to set the chmap or not.
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC alsa-lib 2/5] test: Add pcm-multi-thread program
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 1/5] pcm: " Takashi Iwai
@ 2016-07-05 15:20 ` Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 3/5] Add pcm-multi-thread to .gitignore Takashi Iwai
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
A simple multi-thread stress test for PCM is added to test
subdirectory. It can perform various PCM update function in the
worker threads while reading/writing the data in the main thread.
It can help catching the unexpected error or blockage. For example,
running the capture test without thread-safety option will lead to the
assert due to the races.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
test/Makefile.am | 3 +-
test/pcm-multi-thread.c | 263 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 265 insertions(+), 1 deletion(-)
create mode 100644 test/pcm-multi-thread.c
diff --git a/test/Makefile.am b/test/Makefile.am
index cbf2ab4d58aa..970595ae432f 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -3,7 +3,7 @@ SUBDIRS=. lsb
check_PROGRAMS=control pcm pcm_min latency seq \
playmidi1 timer rawmidi midiloop \
oldapi queue_timer namehint client_event_filter \
- chmap audio_time user-ctl-element-set
+ chmap audio_time user-ctl-element-set pcm-multi-thread
control_LDADD=../src/libasound.la
pcm_LDADD=../src/libasound.la
@@ -23,6 +23,7 @@ client_event_filter_LDADD=../src/libasound.la
code_CFLAGS=-Wall -pipe -g -O2
chmap_LDADD=../src/libasound.la
audio_time_LDADD=../src/libasound.la
+pcm_multi_thread_LDADD=../src/libasound.la
user_ctl_element_set_LDADD=../src/libasound.la
user_ctl_element_set_CFLAGS=-Wall -g
diff --git a/test/pcm-multi-thread.c b/test/pcm-multi-thread.c
new file mode 100644
index 000000000000..da1b87c60b16
--- /dev/null
+++ b/test/pcm-multi-thread.c
@@ -0,0 +1,263 @@
+/*
+ * simple multi-thread stress test for PCM
+ *
+ * The main thread simply feeds or reads the sample data with the
+ * random size continuously. Meanwhile, the worker threads call some
+ * update function depending on the given mode, and show the thread
+ * number of the read value.
+ *
+ * The function for the worker thread is specified via -m option.
+ * When the random mode ('r') is set, the update function is chosen
+ * randomly in the loop.
+ *
+ * When the -v option is passed, this tries to show some obtained value
+ * from the function. Without -v, as default, it shows the thread number
+ * (0-9). In addition, it puts the mode suffix ('a' for avail, 'd' for
+ * delay, etc) for the random mode, as well as the suffix '!' indicating
+ * the error from the called function.
+ */
+
+#include <stdio.h>
+#include <pthread.h>
+#include <getopt.h>
+#include "../include/asoundlib.h"
+
+#define MAX_THREADS 10
+
+enum {
+ MODE_AVAIL_UPDATE,
+ MODE_STATUS,
+ MODE_HWSYNC,
+ MODE_TIMESTAMP,
+ MODE_DELAY,
+ MODE_RANDOM
+};
+
+static char mode_suffix[] = {
+ 'a', 's', 'h', 't', 'd', 'r'
+};
+
+static const char *devname = "default";
+static int stream = SND_PCM_STREAM_PLAYBACK;
+static int num_threads = 1;
+static int periodsize = 16 * 1024;
+static int bufsize = 16 * 1024 * 4;
+static int channels = 2;
+static int rate = 48000;
+static snd_pcm_format_t format = SND_PCM_FORMAT_S16_LE;
+
+static int running_mode = MODE_AVAIL_UPDATE;
+static int show_value = 0;
+static int quiet = 0;
+
+static pthread_t peeper_threads[MAX_THREADS];
+static int running = 1;
+static snd_pcm_t *pcm;
+
+static void *peeper(void *data)
+{
+ int thread_no = (long)data;
+ snd_pcm_sframes_t val;
+ snd_pcm_status_t *stat;
+ snd_htimestamp_t tstamp;
+ int mode = running_mode, err;
+
+ snd_pcm_status_alloca(&stat);
+
+ while (running) {
+ if (running_mode == MODE_RANDOM)
+ mode = rand() % MODE_RANDOM;
+ switch (mode) {
+ case MODE_AVAIL_UPDATE:
+ val = snd_pcm_avail_update(pcm);
+ err = 0;
+ break;
+ case MODE_STATUS:
+ err = snd_pcm_status(pcm, stat);
+ val = snd_pcm_status_get_avail(stat);
+ break;
+ case MODE_HWSYNC:
+ err = snd_pcm_hwsync(pcm);
+ break;
+ case MODE_TIMESTAMP:
+ err = snd_pcm_htimestamp(pcm, (snd_pcm_uframes_t *)&val,
+ &tstamp);
+ break;
+ default:
+ err = snd_pcm_delay(pcm, &val);
+ break;
+ }
+
+ if (quiet)
+ continue;
+ if (running_mode == MODE_RANDOM) {
+ fprintf(stderr, "%d%c%s", thread_no, mode_suffix[mode],
+ err ? "!" : "");
+ } else {
+ if (show_value && mode != MODE_HWSYNC)
+ fprintf(stderr, "\r%d ", (int)val);
+ else
+ fprintf(stderr, "%d%s", thread_no,
+ err ? "!" : "");
+ }
+ }
+ return NULL;
+}
+
+static void usage(void)
+{
+ fprintf(stderr, "usage: multi-thread [-options]\n");
+ fprintf(stderr, " -D str Set device name\n");
+ fprintf(stderr, " -r val Set sample rate\n");
+ fprintf(stderr, " -p val Set period size (in frame)\n");
+ fprintf(stderr, " -b val Set buffer size (in frame)\n");
+ fprintf(stderr, " -c val Set number of channels\n");
+ fprintf(stderr, " -f str Set PCM format\n");
+ fprintf(stderr, " -s str Set stream direction (playback or capture)\n");
+ fprintf(stderr, " -t val Set number of threads\n");
+ fprintf(stderr, " -m str Running mode (avail, status, hwsync, timestamp, delay, random)\n");
+ fprintf(stderr, " -v Show value\n");
+ fprintf(stderr, " -q Quiet mode\n");
+}
+
+static int parse_options(int argc, char **argv)
+{
+ int c, i;
+
+ while ((c = getopt(argc, argv, "D:r:f:p:b:s:t:m:vq")) >= 0) {
+ switch (c) {
+ case 'D':
+ devname = optarg;
+ break;
+ case 'r':
+ rate = atoi(optarg);
+ break;
+ case 'p':
+ periodsize = atoi(optarg);
+ break;
+ case 'b':
+ bufsize = atoi(optarg);
+ break;
+ case 'c':
+ channels = atoi(optarg);
+ break;
+ case 'f':
+ format = snd_pcm_format_value(optarg);
+ break;
+ case 's':
+ if (*optarg == 'p' || *optarg == 'P')
+ stream = SND_PCM_STREAM_PLAYBACK;
+ else if (*optarg == 'c' || *optarg == 'C')
+ stream = SND_PCM_STREAM_CAPTURE;
+ else {
+ fprintf(stderr, "invalid stream direction\n");
+ return 1;
+ }
+ break;
+ case 't':
+ num_threads = atoi(optarg);
+ if (num_threads < 1 || num_threads > MAX_THREADS) {
+ fprintf(stderr, "invalid number of threads\n");
+ return 1;
+ }
+ break;
+ case 'm':
+ for (i = 0; i <= MODE_RANDOM; i++)
+ if (mode_suffix[i] == *optarg)
+ break;
+ if (i > MODE_RANDOM) {
+ fprintf(stderr, "invalid mode type\n");
+ return 1;
+ }
+ running_mode = i;
+ break;
+ case 'v':
+ show_value = 1;
+ break;
+ case 'q':
+ quiet = 1;
+ break;
+ default:
+ usage();
+ return 1;
+ }
+ }
+ return 0;
+}
+
+static int setup_params(void)
+{
+ snd_pcm_hw_params_t *hw;
+
+ /* FIXME: more finer error checks */
+ snd_pcm_hw_params_alloca(&hw);
+ snd_pcm_hw_params_any(pcm, hw);
+ snd_pcm_hw_params_set_access(pcm, hw, SND_PCM_ACCESS_RW_INTERLEAVED);
+ snd_pcm_hw_params_set_format(pcm, hw, format);
+ snd_pcm_hw_params_set_channels(pcm, hw, channels);
+ snd_pcm_hw_params_set_rate(pcm, hw, rate, 0);
+ snd_pcm_hw_params_set_period_size(pcm, hw, periodsize, 0);
+ snd_pcm_hw_params_set_buffer_size(pcm, hw, bufsize);
+ if (snd_pcm_hw_params(pcm, hw) < 0) {
+ fprintf(stderr, "snd_pcm_hw_params error\n");
+ return 1;
+ }
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ char *buf;
+ int i, err;
+
+ if (parse_options(argc, argv))
+ return 1;
+
+ err = snd_pcm_open(&pcm, devname, stream, 0);
+ if (err < 0) {
+ fprintf(stderr, "cannot open pcm %s\n", devname);
+ return 1;
+ }
+
+ if (setup_params())
+ return 1;
+
+ buf = calloc(1, snd_pcm_format_size(format, bufsize) * channels);
+ if (!buf) {
+ fprintf(stderr, "cannot alloc buffer\n");
+ return 1;
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ if (pthread_create(&peeper_threads[i], NULL, peeper, (void *)(long)i)) {
+ fprintf(stderr, "pthread_create error\n");
+ return 1;
+ }
+ }
+
+ if (stream == SND_PCM_STREAM_CAPTURE)
+ snd_pcm_start(pcm);
+ for (;;) {
+ int size = rand() % (bufsize / 2);
+ if (stream == SND_PCM_STREAM_PLAYBACK)
+ err = snd_pcm_writei(pcm, buf, size);
+ else
+ err = snd_pcm_readi(pcm, buf, size);
+ if (err < 0) {
+ fprintf(stderr, "read/write error %d\n", err);
+ err = snd_pcm_recover(pcm, err, 0);
+ if (err < 0)
+ break;
+ if (stream == SND_PCM_STREAM_CAPTURE)
+ snd_pcm_start(pcm);
+ }
+ }
+
+ running = 0;
+ for (i = 0; i < num_threads; i++)
+ pthread_cancel(peeper_threads[i]);
+ for (i = 0; i < num_threads; i++)
+ pthread_join(peeper_threads[i], NULL);
+
+ return 1;
+}
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC alsa-lib 3/5] Add pcm-multi-thread to .gitignore
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 1/5] pcm: " Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 2/5] test: Add pcm-multi-thread program Takashi Iwai
@ 2016-07-05 15:20 ` Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 4/5] pcm: Remove superfluous rmb() from PCM meter plugin Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 5/5] pcm: Remove home brew atomic operations Takashi Iwai
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index f46157290d70..098de14dc332 100644
--- a/.gitignore
+++ b/.gitignore
@@ -58,6 +58,7 @@ test/oldapi
test/chmap
test/pcm
test/pcm_min
+test/pcm-multi-thread
test/playmidi1
test/queue_timer
test/rawmidi
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC alsa-lib 4/5] pcm: Remove superfluous rmb() from PCM meter plugin
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
` (2 preceding siblings ...)
2016-07-05 15:20 ` [PATCH RFC alsa-lib 3/5] Add pcm-multi-thread to .gitignore Takashi Iwai
@ 2016-07-05 15:20 ` Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 5/5] pcm: Remove home brew atomic operations Takashi Iwai
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
rmb() is still left in the code without any actual meaning there just
before the atomic operation. Let's clean it up.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
src/pcm/pcm_meter.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/pcm/pcm_meter.c b/src/pcm/pcm_meter.c
index 1b0ccb40b3dd..d71af7a683e0 100644
--- a/src/pcm/pcm_meter.c
+++ b/src/pcm/pcm_meter.c
@@ -136,7 +136,6 @@ static int snd_pcm_meter_update_scope(snd_pcm_t *pcm)
_again:
rptr = *pcm->hw.ptr;
old_rptr = meter->rptr;
- rmb();
if (atomic_read(&meter->reset)) {
reset = 1;
atomic_dec(&meter->reset);
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC alsa-lib 5/5] pcm: Remove home brew atomic operations
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
` (3 preceding siblings ...)
2016-07-05 15:20 ` [PATCH RFC alsa-lib 4/5] pcm: Remove superfluous rmb() from PCM meter plugin Takashi Iwai
@ 2016-07-05 15:20 ` Takashi Iwai
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 15:20 UTC (permalink / raw)
To: alsa-devel
We've had a few home brew atomic operations in a couple of places in
the PCM code. This was for supporting the concurrent accesses, but in
practice, it couldn't cover the race properly by itself alone.
Since we have a wider concurrency protection via mutex now, we can get
rid of these atomic codes, which worsens the portability
significantly.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
include/Makefile.am | 2 +-
include/iatomic.h | 170 ---------------------------------------------------
src/pcm/Makefile.am | 2 +-
src/pcm/atomic.c | 43 -------------
src/pcm/pcm_plugin.c | 72 +++++-----------------
src/pcm/pcm_plugin.h | 2 -
src/pcm/pcm_rate.c | 34 ++---------
7 files changed, 20 insertions(+), 305 deletions(-)
delete mode 100644 include/iatomic.h
delete mode 100644 src/pcm/atomic.c
diff --git a/include/Makefile.am b/include/Makefile.am
index 31a3f748dc46..67f32e36c911 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -5,7 +5,7 @@ alsaincludedir = ${includedir}/alsa
alsainclude_HEADERS = asoundlib.h asoundef.h \
version.h global.h input.h output.h error.h \
- conf.h control.h iatomic.h
+ conf.h control.h
if BUILD_CTL_PLUGIN_EXT
alsainclude_HEADERS += control_external.h
diff --git a/include/iatomic.h b/include/iatomic.h
deleted file mode 100644
index acdd3e29c13a..000000000000
--- a/include/iatomic.h
+++ /dev/null
@@ -1,170 +0,0 @@
-#ifndef __ALSA_IATOMIC_H
-#define __ALSA_IATOMIC_H
-
-#ifdef __i386__
-#define mb() __asm__ __volatile__ ("lock; addl $0,0(%%esp)": : :"memory")
-#define rmb() mb()
-#define wmb() __asm__ __volatile__ ("": : :"memory")
-#define IATOMIC_DEFINED 1
-#endif
-
-#ifdef __x86_64__
-#define mb() asm volatile("mfence":::"memory")
-#define rmb() asm volatile("lfence":::"memory")
-#define wmb() asm volatile("sfence":::"memory")
-#define IATOMIC_DEFINED 1
-#endif
-
-#ifdef __ia64__
-/*
- * Macros to force memory ordering. In these descriptions, "previous"
- * and "subsequent" refer to program order; "visible" means that all
- * architecturally visible effects of a memory access have occurred
- * (at a minimum, this means the memory has been read or written).
- *
- * wmb(): Guarantees that all preceding stores to memory-
- * like regions are visible before any subsequent
- * stores and that all following stores will be
- * visible only after all previous stores.
- * rmb(): Like wmb(), but for reads.
- * mb(): wmb()/rmb() combo, i.e., all previous memory
- * accesses are visible before all subsequent
- * accesses and vice versa. This is also known as
- * a "fence."
- *
- * Note: "mb()" and its variants cannot be used as a fence to order
- * accesses to memory mapped I/O registers. For that, mf.a needs to
- * be used. However, we don't want to always use mf.a because (a)
- * it's (presumably) much slower than mf and (b) mf.a is supported for
- * sequential memory pages only.
- */
-#define mb() __asm__ __volatile__ ("mf" ::: "memory")
-#define rmb() mb()
-#define wmb() mb()
-
-#define IATOMIC_DEFINED 1
-
-#endif /* __ia64__ */
-
-#ifdef __alpha__
-
-#define mb() \
-__asm__ __volatile__("mb": : :"memory")
-
-#define rmb() \
-__asm__ __volatile__("mb": : :"memory")
-
-#define wmb() \
-__asm__ __volatile__("wmb": : :"memory")
-
-#define IATOMIC_DEFINED 1
-
-#endif /* __alpha__ */
-
-#ifdef __powerpc__
-
-/*
- * Memory barrier.
- * The sync instruction guarantees that all memory accesses initiated
- * by this processor have been performed (with respect to all other
- * mechanisms that access memory). The eieio instruction is a barrier
- * providing an ordering (separately) for (a) cacheable stores and (b)
- * loads and stores to non-cacheable memory (e.g. I/O devices).
- *
- * mb() prevents loads and stores being reordered across this point.
- * rmb() prevents loads being reordered across this point.
- * wmb() prevents stores being reordered across this point.
- *
- * We can use the eieio instruction for wmb, but since it doesn't
- * give any ordering guarantees about loads, we have to use the
- * stronger but slower sync instruction for mb and rmb.
- */
-#define mb() __asm__ __volatile__ ("sync" : : : "memory")
-#define rmb() __asm__ __volatile__ ("sync" : : : "memory")
-#define wmb() __asm__ __volatile__ ("eieio" : : : "memory")
-
-#define IATOMIC_DEFINED 1
-
-#endif /* __powerpc__ */
-
-#ifndef IATOMIC_DEFINED
-
-/* Generic __sync_synchronize is available from gcc 4.1 */
-
-#define mb() __sync_synchronize()
-#define rmb() mb()
-#define wmb() mb()
-
-#define IATOMIC_DEFINED 1
-
-#endif /* IATOMIC_DEFINED */
-
-/*
- * Atomic read/write
- * Copyright (c) 2001 by Abramo Bagnara <abramo@alsa-project.org>
- */
-
-/* Max number of times we must spin on a spin-lock calling sched_yield().
- After MAX_SPIN_COUNT iterations, we put the calling thread to sleep. */
-
-#ifndef MAX_SPIN_COUNT
-#define MAX_SPIN_COUNT 50
-#endif
-
-/* Duration of sleep (in nanoseconds) when we can't acquire a spin-lock
- after MAX_SPIN_COUNT iterations of sched_yield().
- This MUST BE > 2ms.
- (Otherwise the kernel does busy-waiting for real-time threads,
- giving other threads no chance to run.) */
-
-#ifndef SPIN_SLEEP_DURATION
-#define SPIN_SLEEP_DURATION 2000001
-#endif
-
-typedef struct {
- unsigned int begin, end;
-} snd_atomic_write_t;
-
-typedef struct {
- volatile const snd_atomic_write_t *write;
- unsigned int end;
-} snd_atomic_read_t;
-
-void snd_atomic_read_wait(snd_atomic_read_t *t);
-
-static __inline__ void snd_atomic_write_init(snd_atomic_write_t *w)
-{
- w->begin = 0;
- w->end = 0;
-}
-
-static __inline__ void snd_atomic_write_begin(snd_atomic_write_t *w)
-{
- w->begin++;
- wmb();
-}
-
-static __inline__ void snd_atomic_write_end(snd_atomic_write_t *w)
-{
- wmb();
- w->end++;
-}
-
-static __inline__ void snd_atomic_read_init(snd_atomic_read_t *r, snd_atomic_write_t *w)
-{
- r->write = w;
-}
-
-static __inline__ void snd_atomic_read_begin(snd_atomic_read_t *r)
-{
- r->end = r->write->end;
- rmb();
-}
-
-static __inline__ int snd_atomic_read_ok(snd_atomic_read_t *r)
-{
- rmb();
- return r->end == r->write->begin;
-}
-
-#endif /* __ALSA_IATOMIC_H */
diff --git a/src/pcm/Makefile.am b/src/pcm/Makefile.am
index 81598f634bc3..8edbd0b5c719 100644
--- a/src/pcm/Makefile.am
+++ b/src/pcm/Makefile.am
@@ -3,7 +3,7 @@ DIST_SUBDIRS = scopes
EXTRA_LTLIBRARIES = libpcm.la
-libpcm_la_SOURCES = atomic.c mask.c interval.c \
+libpcm_la_SOURCES = mask.c interval.c \
pcm.c pcm_params.c pcm_simple.c \
pcm_hw.c pcm_misc.c pcm_mmap.c pcm_symbols.c
diff --git a/src/pcm/atomic.c b/src/pcm/atomic.c
deleted file mode 100644
index 75659457af76..000000000000
--- a/src/pcm/atomic.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Atomic read/write
- * Copyright (c) 2001 by Abramo Bagnara <abramo@alsa-project.org>
- *
- * This library is free software; you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as
- * published by the Free Software Foundation; either version 2.1 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- */
-
-#include <stdlib.h>
-#include <time.h>
-#include <sched.h>
-#include "iatomic.h"
-
-void snd_atomic_read_wait(snd_atomic_read_t *t)
-{
- volatile const snd_atomic_write_t *w = t->write;
- unsigned int loops = 0;
- struct timespec ts;
- while (w->begin != w->end) {
- if (loops < MAX_SPIN_COUNT) {
- sched_yield();
- loops++;
- continue;
- }
- loops = 0;
- ts.tv_sec = 0;
- ts.tv_nsec = SPIN_SLEEP_DURATION;
- nanosleep(&ts, NULL);
- }
-}
-
diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
index 8527783c3569..e53c5bb5568e 100644
--- a/src/pcm/pcm_plugin.c
+++ b/src/pcm/pcm_plugin.c
@@ -133,7 +133,6 @@ void snd_pcm_plugin_init(snd_pcm_plugin_t *plugin)
memset(plugin, 0, sizeof(snd_pcm_plugin_t));
plugin->undo_read = snd_pcm_plugin_undo_read;
plugin->undo_write = snd_pcm_plugin_undo_write;
- snd_atomic_write_init(&plugin->watom);
}
static int snd_pcm_plugin_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
@@ -157,15 +156,11 @@ static int snd_pcm_plugin_prepare(snd_pcm_t *pcm)
{
snd_pcm_plugin_t *plugin = pcm->private_data;
int err;
- snd_atomic_write_begin(&plugin->watom);
err = snd_pcm_prepare(plugin->gen.slave);
- if (err < 0) {
- snd_atomic_write_end(&plugin->watom);
+ if (err < 0)
return err;
- }
*pcm->hw.ptr = 0;
*pcm->appl.ptr = 0;
- snd_atomic_write_end(&plugin->watom);
if (plugin->init) {
err = plugin->init(pcm);
if (err < 0)
@@ -178,15 +173,11 @@ static int snd_pcm_plugin_reset(snd_pcm_t *pcm)
{
snd_pcm_plugin_t *plugin = pcm->private_data;
int err;
- snd_atomic_write_begin(&plugin->watom);
err = snd_pcm_reset(plugin->gen.slave);
- if (err < 0) {
- snd_atomic_write_end(&plugin->watom);
+ if (err < 0)
return err;
- }
*pcm->hw.ptr = 0;
*pcm->appl.ptr = 0;
- snd_atomic_write_end(&plugin->watom);
if (plugin->init) {
err = plugin->init(pcm);
if (err < 0)
@@ -212,14 +203,10 @@ snd_pcm_sframes_t snd_pcm_plugin_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames
return 0;
sframes = frames;
- snd_atomic_write_begin(&plugin->watom);
sframes = snd_pcm_rewind(plugin->gen.slave, sframes);
- if (sframes < 0) {
- snd_atomic_write_end(&plugin->watom);
+ if (sframes < 0)
return sframes;
- }
snd_pcm_mmap_appl_backward(pcm, (snd_pcm_uframes_t) sframes);
- snd_atomic_write_end(&plugin->watom);
return (snd_pcm_sframes_t) sframes;
}
@@ -240,14 +227,10 @@ snd_pcm_sframes_t snd_pcm_plugin_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frame
return 0;
sframes = frames;
- snd_atomic_write_begin(&plugin->watom);
sframes = INTERNAL(snd_pcm_forward)(plugin->gen.slave, sframes);
- if (sframes < 0) {
- snd_atomic_write_end(&plugin->watom);
+ if (sframes < 0)
return sframes;
- }
snd_pcm_mmap_appl_forward(pcm, (snd_pcm_uframes_t) frames);
- snd_atomic_write_end(&plugin->watom);
return (snd_pcm_sframes_t) frames;
}
@@ -279,31 +262,27 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm,
err = -EPIPE;
goto error;
}
- snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
if (res < 0) {
err = res;
- goto error_atomic;
+ goto error;
}
frames -= res;
}
if (result <= 0) {
err = result;
- goto error_atomic;
+ goto error;
}
snd_pcm_mmap_appl_forward(pcm, frames);
- snd_atomic_write_end(&plugin->watom);
offset += frames;
xfer += frames;
size -= frames;
}
return (snd_pcm_sframes_t)xfer;
- error_atomic:
- snd_atomic_write_end(&plugin->watom);
error:
return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
@@ -336,7 +315,6 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
err = -EPIPE;
goto error;
}
- snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
@@ -344,24 +322,21 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
res = plugin->undo_read(slave, areas, offset, frames, slave_frames - result);
if (res < 0) {
err = res;
- goto error_atomic;
+ goto error;
}
frames -= res;
}
if (result <= 0) {
err = result;
- goto error_atomic;
+ goto error;
}
snd_pcm_mmap_appl_forward(pcm, frames);
- snd_atomic_write_end(&plugin->watom);
offset += frames;
xfer += frames;
size -= frames;
}
return (snd_pcm_sframes_t)xfer;
- error_atomic:
- snd_atomic_write_end(&plugin->watom);
error:
return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
@@ -417,9 +392,7 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
int err;
if (pcm->stream == SND_PCM_STREAM_CAPTURE) {
- snd_atomic_write_begin(&plugin->watom);
snd_pcm_mmap_appl_forward(pcm, size);
- snd_atomic_write_end(&plugin->watom);
return size;
}
slave_size = snd_pcm_avail_update(slave);
@@ -443,7 +416,6 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
frames = cont;
frames = plugin->write(pcm, areas, appl_offset, frames,
slave_areas, slave_offset, &slave_frames);
- snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
@@ -451,16 +423,15 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result);
if (res < 0) {
err = res;
- goto error_atomic;
+ goto error;
}
frames -= res;
}
if (result <= 0) {
err = result;
- goto error_atomic;
+ goto error;
}
snd_pcm_mmap_appl_forward(pcm, frames);
- snd_atomic_write_end(&plugin->watom);
if (frames == cont)
appl_offset = 0;
else
@@ -475,8 +446,6 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
}
return xfer;
- error_atomic:
- snd_atomic_write_end(&plugin->watom);
error:
return xfer > 0 ? xfer : err;
}
@@ -519,7 +488,6 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
frames = cont;
frames = (plugin->read)(pcm, areas, hw_offset, frames,
slave_areas, slave_offset, &slave_frames);
- snd_atomic_write_begin(&plugin->watom);
result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames);
if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) {
snd_pcm_sframes_t res;
@@ -527,16 +495,15 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
res = plugin->undo_read(slave, areas, hw_offset, frames, slave_frames - result);
if (res < 0) {
err = res;
- goto error_atomic;
+ goto error;
}
frames -= res;
}
if (result <= 0) {
err = result;
- goto error_atomic;
+ goto error;
}
snd_pcm_mmap_hw_forward(pcm, frames);
- snd_atomic_write_end(&plugin->watom);
if (frames == cont)
hw_offset = 0;
else
@@ -547,8 +514,6 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
}
return (snd_pcm_sframes_t)xfer;
- error_atomic:
- snd_atomic_write_end(&plugin->watom);
error:
return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
}
@@ -558,24 +523,15 @@ static int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
{
snd_pcm_plugin_t *plugin = pcm->private_data;
snd_pcm_sframes_t err;
- snd_atomic_read_t ratom;
- snd_atomic_read_init(&ratom, &plugin->watom);
- _again:
- snd_atomic_read_begin(&ratom);
+
/* sync with the latest hw and appl ptrs */
snd_pcm_plugin_avail_update(pcm);
err = snd_pcm_status(plugin->gen.slave, status);
- if (err < 0) {
- snd_atomic_read_ok(&ratom);
+ if (err < 0)
return err;
- }
status->appl_ptr = *pcm->appl.ptr;
status->hw_ptr = *pcm->hw.ptr;
- if (!snd_atomic_read_ok(&ratom)) {
- snd_atomic_read_wait(&ratom);
- goto _again;
- }
return 0;
}
diff --git a/src/pcm/pcm_plugin.h b/src/pcm/pcm_plugin.h
index b0a3e1869ea1..217f0757ea59 100644
--- a/src/pcm/pcm_plugin.h
+++ b/src/pcm/pcm_plugin.h
@@ -19,7 +19,6 @@
*
*/
-#include "iatomic.h"
#include "pcm_generic.h"
typedef snd_pcm_uframes_t (*snd_pcm_slave_xfer_areas_func_t)
@@ -46,7 +45,6 @@ typedef struct {
snd_pcm_slave_xfer_areas_undo_func_t undo_write;
int (*init)(snd_pcm_t *pcm);
snd_pcm_uframes_t appl_ptr, hw_ptr;
- snd_atomic_write_t watom;
} snd_pcm_plugin_t;
/* make local functions really local */
diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 0fe466647a55..e44bd21be8c9 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -32,7 +32,6 @@
#include "pcm_local.h"
#include "pcm_plugin.h"
#include "pcm_rate.h"
-#include "iatomic.h"
#include "plugin_ops.h"
@@ -51,7 +50,6 @@ typedef struct _snd_pcm_rate snd_pcm_rate_t;
struct _snd_pcm_rate {
snd_pcm_generic_t gen;
- snd_atomic_write_t watom;
snd_pcm_uframes_t appl_ptr, hw_ptr;
snd_pcm_uframes_t last_commit_ptr;
snd_pcm_uframes_t orig_avail_min;
@@ -584,9 +582,7 @@ static int snd_pcm_rate_hwsync(snd_pcm_t *pcm)
int err = snd_pcm_hwsync(rate->gen.slave);
if (err < 0)
return err;
- snd_atomic_write_begin(&rate->watom);
snd_pcm_rate_sync_hwptr(pcm);
- snd_atomic_write_end(&rate->watom);
return 0;
}
@@ -602,15 +598,11 @@ static int snd_pcm_rate_prepare(snd_pcm_t *pcm)
snd_pcm_rate_t *rate = pcm->private_data;
int err;
- snd_atomic_write_begin(&rate->watom);
err = snd_pcm_prepare(rate->gen.slave);
- if (err < 0) {
- snd_atomic_write_end(&rate->watom);
+ if (err < 0)
return err;
- }
*pcm->hw.ptr = 0;
*pcm->appl.ptr = 0;
- snd_atomic_write_end(&rate->watom);
err = snd_pcm_rate_init(pcm);
if (err < 0)
return err;
@@ -621,15 +613,11 @@ static int snd_pcm_rate_reset(snd_pcm_t *pcm)
{
snd_pcm_rate_t *rate = pcm->private_data;
int err;
- snd_atomic_write_begin(&rate->watom);
err = snd_pcm_reset(rate->gen.slave);
- if (err < 0) {
- snd_atomic_write_end(&rate->watom);
+ if (err < 0)
return err;
- }
*pcm->hw.ptr = 0;
*pcm->appl.ptr = 0;
- snd_atomic_write_end(&rate->watom);
err = snd_pcm_rate_init(pcm);
if (err < 0)
return err;
@@ -923,9 +911,7 @@ static snd_pcm_sframes_t snd_pcm_rate_mmap_commit(snd_pcm_t *pcm,
if (err < 0)
return err;
}
- snd_atomic_write_begin(&rate->watom);
snd_pcm_mmap_appl_forward(pcm, size);
- snd_atomic_write_end(&rate->watom);
return size;
}
@@ -938,9 +924,7 @@ static snd_pcm_sframes_t snd_pcm_rate_avail_update(snd_pcm_t *pcm)
slave_size = snd_pcm_avail_update(slave);
if (pcm->stream == SND_PCM_STREAM_CAPTURE)
goto _capture;
- snd_atomic_write_begin(&rate->watom);
snd_pcm_rate_sync_hwptr(pcm);
- snd_atomic_write_end(&rate->watom);
snd_pcm_rate_sync_playback_area(pcm, rate->appl_ptr);
return snd_pcm_mmap_avail(pcm);
_capture: {
@@ -1090,15 +1074,10 @@ static int snd_pcm_rate_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
{
snd_pcm_rate_t *rate = pcm->private_data;
snd_pcm_sframes_t err;
- snd_atomic_read_t ratom;
- snd_atomic_read_init(&ratom, &rate->watom);
- _again:
- snd_atomic_read_begin(&ratom);
+
err = snd_pcm_status(rate->gen.slave, status);
- if (err < 0) {
- snd_atomic_read_ok(&ratom);
+ if (err < 0)
return err;
- }
if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
if (rate->start_pending)
status->state = SND_PCM_STATE_RUNNING;
@@ -1116,10 +1095,6 @@ static int snd_pcm_rate_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
status->avail = snd_pcm_mmap_capture_avail(pcm);
status->avail_max = rate->ops.output_frames(rate->obj, status->avail_max);
}
- if (!snd_atomic_read_ok(&ratom)) {
- snd_atomic_read_wait(&ratom);
- goto _again;
- }
return 0;
}
@@ -1309,7 +1284,6 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
rate->gen.close_slave = close_slave;
rate->srate = srate;
rate->sformat = sformat;
- snd_atomic_write_init(&rate->watom);
err = snd_pcm_new(&pcm, SND_PCM_TYPE_RATE, name, slave->stream, slave->mode);
if (err < 0) {
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RFC alsa-lib 1/5] pcm: Add thread-safety to PCM API
2016-07-05 15:20 ` [PATCH RFC alsa-lib 1/5] pcm: " Takashi Iwai
@ 2016-07-05 16:30 ` Clemens Ladisch
2016-07-05 16:36 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: Clemens Ladisch @ 2016-07-05 16:30 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel
Takashi Iwai wrote:
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -686,6 +686,8 @@ snd_pcm_stream_t snd_pcm_stream(snd_pcm_t *pcm)
> *
> * Closes the specified PCM handle and frees all associated
> * resources.
> + *
> + * The function is thread-safe when built with the proper option.
> */
> int snd_pcm_close(snd_pcm_t *pcm)
> {
> @@ -697,6 +699,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
> if (err < 0)
> res = err;
> }
> + snd_pcm_lock(pcm);
> if (pcm->mmap_channels)
> snd_pcm_munmap(pcm);
> while (!list_empty(&pcm->async_handlers)) {
> @@ -704,6 +707,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
> snd_async_del_handler(h);
> }
> err = pcm->ops->close(pcm->op_arg);
> + snd_pcm_unlock(pcm);
> if (err < 0)
> res = err;
> err = snd_pcm_free(pcm);
Thread safety does not really make sense for a destructor like
snd_pcm_close(). If any other code is accessing the device at the same
time, the freeing will make it crash anyway.
What would make sense would be to output a debug warning if the device
is locked, and/or to poison the device (maybe ->ops = NULL).
Regards,
Clemens
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC alsa-lib 1/5] pcm: Add thread-safety to PCM API
2016-07-05 16:30 ` Clemens Ladisch
@ 2016-07-05 16:36 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2016-07-05 16:36 UTC (permalink / raw)
To: Clemens Ladisch; +Cc: alsa-devel
On Tue, 05 Jul 2016 18:30:36 +0200,
Clemens Ladisch wrote:
>
> Takashi Iwai wrote:
> > --- a/src/pcm/pcm.c
> > +++ b/src/pcm/pcm.c
> > @@ -686,6 +686,8 @@ snd_pcm_stream_t snd_pcm_stream(snd_pcm_t *pcm)
> > *
> > * Closes the specified PCM handle and frees all associated
> > * resources.
> > + *
> > + * The function is thread-safe when built with the proper option.
> > */
> > int snd_pcm_close(snd_pcm_t *pcm)
> > {
> > @@ -697,6 +699,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
> > if (err < 0)
> > res = err;
> > }
> > + snd_pcm_lock(pcm);
> > if (pcm->mmap_channels)
> > snd_pcm_munmap(pcm);
> > while (!list_empty(&pcm->async_handlers)) {
> > @@ -704,6 +707,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
> > snd_async_del_handler(h);
> > }
> > err = pcm->ops->close(pcm->op_arg);
> > + snd_pcm_unlock(pcm);
> > if (err < 0)
> > res = err;
> > err = snd_pcm_free(pcm);
>
> Thread safety does not really make sense for a destructor like
> snd_pcm_close(). If any other code is accessing the device at the same
> time, the freeing will make it crash anyway.
That's true. OTOH, snd_pcm_close() does sync before closing, so I
thought it might be not too bad to be prepared for bad programmers.
But I'm fine to get rid of it.
> What would make sense would be to output a debug warning if the device
> is locked, and/or to poison the device (maybe ->ops = NULL).
Yes, more debugging option would be nicer. We can add it later.
thanks,
Takashi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-05 16:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-05 15:20 [PATCH RFC alsa-lib 0/5] Add thread-safety to PCM API Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 1/5] pcm: " Takashi Iwai
2016-07-05 16:30 ` Clemens Ladisch
2016-07-05 16:36 ` Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 2/5] test: Add pcm-multi-thread program Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 3/5] Add pcm-multi-thread to .gitignore Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 4/5] pcm: Remove superfluous rmb() from PCM meter plugin Takashi Iwai
2016-07-05 15:20 ` [PATCH RFC alsa-lib 5/5] pcm: Remove home brew atomic operations Takashi Iwai
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.