November 2012 - Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] Races in alsa-lib with threads
by Krakora Robert-B42341 13 Nov '12

13 Nov '12

> From: Takashi Iwai [tiwai(a)suse.de] > Sent: Tuesday, November 13, 2012 2:30 AM > To: Trent Piepho > Cc: Krakora Robert-B42341; alsa-devel(a)alsa-project.org > Subject: Re: [alsa-devel] Races in alsa-lib with threads > > At Tue, 13 Nov 2012 02:14:08 -0500, > Trent Piepho wrote: > > > > On Tue, Nov 13, 2012 at 1:32 AM, Takashi Iwai <tiwai(a)suse.de> wrote: > > > At Mon, 12 Nov 2012 19:40:42 +0000 (UTC), > > > Rob Krakora wrote: > > >> Would you be able to point me the the ALSA documentation that indicates the > > >> stipulations on handle usage using multiple threads? I cannot find it. > > > > > > Think other way round: > > > The fact that it isn't documented means it's not safe to use in that > > > way :) > > > > > > The introduction on the alsa-project home page says, "SMP and > > thread-safe design. " Some people might misunderstand what > > thread-safe means. Maybe some clarification could be added. > > "Different streams are SMP and thread-safe (calls for the same stream > > must be serialized)." > > Yeah this should be corrected. SMP things are just about the kernel > side at that time. > > It's a Wiki, feel free to correct / improve the text. > > > Takashi Hi Takashi, The way that the GStreamer ALSA Sink Plugin is using ALSA Lib assumes that it is thread safe. The fix crafted by one of Trent's colleagues (attached) seems to be the way to go. However, I don't know how it would impact other functionality within ALSA Lib. Best Regards, Rob Krakora

1 0

[alsa-devel] [PATCH v4 2/2] ac3dec: Show usage if no parameter is passed
by Fabio Estevam 13 Nov '12

13 Nov '12

When ac3dec is called without any argument the application hangs forever. Instead of hanging, print the usage and exit. Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- Changes since v3: - Fix return value Changes since v2: - No changes Changes since v1: - Also check for isatty, so that 'ac3dec < file.wav' can still work ac3dec/ac3dec.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ac3dec/ac3dec.c b/ac3dec/ac3dec.c index 00c4f1a..ce6c507 100644 --- a/ac3dec/ac3dec.c +++ b/ac3dec/ac3dec.c @@ -130,6 +130,11 @@ int main(int argc,char *argv[]) out_config.spdif = SPDIF_NONE; out_config.hdmi = 0; + if (isatty(fileno(stdin)) && (argc == 1)) { + help(); + return 1; + } + morehelp = 0; while (1) { int c; -- 1.7.9.5

2 1

[alsa-devel] [PATCH v2 1/2] aplay: Show usage if no parameter is passed
by Fabio Estevam 13 Nov '12

13 Nov '12

When aplay/arecord are called without any argument the application hangs forever. Instead of hanging, print the usage and exit. Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- Changes since v1: - Also check for isatty, so that 'aplay < file.wav' can still work aplay/aplay.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/aplay/aplay.c b/aplay/aplay.c index 741979a..38459e6 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -519,6 +519,11 @@ int main(int argc, char *argv[]) return 1; } + if (isatty(fileno(stdin)) && (argc == 1)) { + usage(command); + return 0; + } + chunk_size = -1; rhwparams.format = DEFAULT_FORMAT; rhwparams.rate = DEFAULT_SPEED; -- 1.7.9.5

3 3

Re: [alsa-devel] Sound+USB: deadlock problem
by Takashi Iwai 13 Nov '12

13 Nov '12

At Tue, 13 Nov 2012 10:39:14 +0100, Jiri Slaby wrote: > > On 11/13/2012 09:45 AM, Takashi Iwai wrote: > > At Tue, 13 Nov 2012 09:30:10 +0100, > > Jiri Slaby wrote: > >> > >> On 11/13/2012 09:28 AM, Takashi Iwai wrote: > >>> At Tue, 13 Nov 2012 09:26:17 +0100, > >>> Jiri Slaby wrote: > >>>> > >>>> Hello, > >>>> > >>>> I've just plugged+unplugged+plugged my USB audio card and the audio > >>>> subsystem got stuck: > >>> > >>> Which kernel? If it's older than 3.7-rc5, could you try 3.7-rc5? > >> > >> Oh, forgot to mention: > >> 3.7.0-rc5-next-20121112_64+ > > > > Through a quick glance, one of mutex locks seems stuck. > > Just a blind short -- does the patch below cure the problem? > > Yes, this helps. > > Adding a LOCKDEP splash _before_ applying the patch: Thanks, this helps much for understanding the cause. It's a new rwsem in the usb-audio driver that deadlocks. Although the previous patch fixes the deadlock it opens the race again. Could you try the patch below instead? The rwsem above doesn't have to be taken for the whole operation, but just needs to protect for turning on the flag. The patch below should suffice for protecting races but avoids the deadlock. Takashi --- diff --git a/sound/usb/card.c b/sound/usb/card.c index 282f0fc..dbf7999 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -559,9 +559,11 @@ static void snd_usb_audio_disconnect(struct usb_device *dev, return; card = chip->card; - mutex_lock(&register_mutex); down_write(&chip->shutdown_rwsem); chip->shutdown = 1; + up_write(&chip->shutdown_rwsem); + + mutex_lock(&register_mutex); chip->num_interfaces--; if (chip->num_interfaces <= 0) { snd_card_disconnect(card); @@ -582,11 +584,9 @@ static void snd_usb_audio_disconnect(struct usb_device *dev, snd_usb_mixer_disconnect(p); } usb_chip[chip->index] = NULL; - up_write(&chip->shutdown_rwsem); mutex_unlock(&register_mutex); snd_card_free_when_closed(card); } else { - up_write(&chip->shutdown_rwsem); mutex_unlock(&register_mutex); } }

1 0

Re: [alsa-devel] Sound+USB: deadlock problem
by Takashi Iwai 13 Nov '12

13 Nov '12

At Tue, 13 Nov 2012 09:30:10 +0100, Jiri Slaby wrote: > > On 11/13/2012 09:28 AM, Takashi Iwai wrote: > > At Tue, 13 Nov 2012 09:26:17 +0100, > > Jiri Slaby wrote: > >> > >> Hello, > >> > >> I've just plugged+unplugged+plugged my USB audio card and the audio > >> subsystem got stuck: > > > > Which kernel? If it's older than 3.7-rc5, could you try 3.7-rc5? > > Oh, forgot to mention: > 3.7.0-rc5-next-20121112_64+ Through a quick glance, one of mutex locks seems stuck. Just a blind short -- does the patch below cure the problem? thanks, Takashi --- diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 030102c..a66a7ca 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1086,7 +1086,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) if (list_empty(&pcm->list)) goto unlock; - mutex_lock(&pcm->open_mutex); + /* mutex_lock(&pcm->open_mutex); */ wake_up(&pcm->open_wait); list_del_init(&pcm->list); for (cidx = 0; cidx < 2; cidx++) @@ -1118,7 +1118,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) pcm->streams[cidx].chmap_kctl = NULL; } } - mutex_unlock(&pcm->open_mutex); + /* mutex_unlock(&pcm->open_mutex); */ unlock: mutex_unlock(&register_mutex); return 0;

1 0

Re: [alsa-devel] Sound+USB: deadlock problem
by Takashi Iwai 13 Nov '12

13 Nov '12

At Tue, 13 Nov 2012 09:26:17 +0100, Jiri Slaby wrote: > > Hello, > > I've just plugged+unplugged+plugged my USB audio card and the audio > subsystem got stuck: Which kernel? If it's older than 3.7-rc5, could you try 3.7-rc5? Takashi > > INFO: task khubd:440 blocked for more than 120 seconds. > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > khubd D ffff8801cbc92400 0 440 2 0x00000000 > ffff8801c4ae3a38 0000000000000046 ffff8801c49be270 ffff8801c4ae3fd8 > ffff8801c4ae3fd8 ffff8801c4ae3fd8 ffff8801c4284830 ffff8801c49be270 > ffff8801c4ae3a28 ffff8801a7198758 ffff8801a719875c ffff8801c49be270 > Call Trace: > [<ffffffff816ae109>] schedule+0x29/0x70 > [<ffffffff816ae38e>] schedule_preempt_disabled+0xe/0x10 > [<ffffffff816ad02a>] __mutex_lock_slowpath+0xca/0x140 > [<ffffffff816acb5a>] mutex_lock+0x2a/0x50 > [<ffffffff8157c8e6>] snd_pcm_dev_disconnect+0x46/0x1f0 > [<ffffffff81576512>] snd_device_disconnect+0x62/0xa0 > [<ffffffff815765f4>] snd_device_disconnect_all+0x44/0x70 > [<ffffffff81570d83>] snd_card_disconnect+0x153/0x1d0 > [<ffffffff816ad3a8>] ? down_write+0x38/0x40 > [<ffffffff815bf038>] usb_audio_disconnect+0x88/0x1c0 > [<ffffffff814df460>] usb_unbind_interface+0x60/0x1a0 > [<ffffffff81436e4c>] __device_release_driver+0x7c/0xe0 > [<ffffffff814370bc>] device_release_driver+0x2c/0x40 > [<ffffffff8143681e>] bus_remove_device+0xee/0x160 > [<ffffffff81433c2a>] device_del+0x11a/0x1b0 > [<ffffffff814dce40>] usb_disable_device+0xb0/0x270 > [<ffffffff814d38ed>] usb_disconnect+0xad/0x160 > [<ffffffff814d613e>] hub_thread+0x6fe/0x1700 > [<ffffffff810b643e>] ? dequeue_task_fair+0xee/0x100 > [<ffffffff810a2c70>] ? finish_wait+0x80/0x80 > [<ffffffff814d5a40>] ? usb_remote_wakeup+0x70/0x70 > [<ffffffff810a2620>] kthread+0xc0/0xd0 > [<ffffffff810a2560>] ? kthread_create_on_node+0x130/0x130 > [<ffffffff816afb5c>] ret_from_fork+0x7c/0xb0 > [<ffffffff810a2560>] ? kthread_create_on_node+0x130/0x130 > INFO: task pulseaudio:4642 blocked for more than 120 seconds. > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > pulseaudio D ffff8801cbc12400 0 4642 1 0x00000000 > ffff8801c1c39cf0 0000000000000082 ffff8801c0d7d550 ffff8801c1c39fd8 > ffff8801c1c39fd8 ffff8801c1c39fd8 ffff8801a7203480 ffff8801c0d7d550 > ffff8801aa37b530 ffff8801c0d7d550 ffff8800ab118a48 fffffffeffffffff > Call Trace: > [<ffffffff816ae109>] schedule+0x29/0x70 > [<ffffffff816aeafd>] rwsem_down_failed_common+0xbd/0x150 > [<ffffffff816aebc5>] rwsem_down_read_failed+0x15/0x17 > [<ffffffff81302c04>] call_rwsem_down_read_failed+0x14/0x30 > [<ffffffff816ad3d4>] ? down_read+0x24/0x2b > [<ffffffff815c84b2>] snd_usb_hw_free+0x42/0x90 > [<ffffffff815809ab>] snd_pcm_release_substream+0x5b/0xc0 > [<ffffffff81580a58>] snd_pcm_release+0x48/0xb0 > [<ffffffff815707d7>] snd_disconnect_release+0x97/0xe0 > [<ffffffff8116ed8c>] __fput+0xec/0x230 > [<ffffffff8116eede>] ____fput+0xe/0x10 > [<ffffffff8109ef37>] task_work_run+0xa7/0xe0 > [<ffffffff81045959>] do_notify_resume+0x59/0x80 > [<ffffffff816afe08>] int_signal+0x12/0x17 > INFO: task khubd:440 blocked for more than 120 seconds. > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > khubd D ffff8801cbc92400 0 440 2 0x00000000 > ffff8801c4ae3a38 0000000000000046 ffff8801c49be270 ffff8801c4ae3fd8 > ffff8801c4ae3fd8 ffff8801c4ae3fd8 ffff8801c4284830 ffff8801c49be270 > ffff8801c4ae3a28 ffff8801a7198758 ffff8801a719875c ffff8801c49be270 > Call Trace: > [<ffffffff816ae109>] schedule+0x29/0x70 > [<ffffffff816ae38e>] schedule_preempt_disabled+0xe/0x10 > [<ffffffff816ad02a>] __mutex_lock_slowpath+0xca/0x140 > [<ffffffff816acb5a>] mutex_lock+0x2a/0x50 > [<ffffffff8157c8e6>] snd_pcm_dev_disconnect+0x46/0x1f0 > [<ffffffff81576512>] snd_device_disconnect+0x62/0xa0 > [<ffffffff815765f4>] snd_device_disconnect_all+0x44/0x70 > [<ffffffff81570d83>] snd_card_disconnect+0x153/0x1d0 > [<ffffffff816ad3a8>] ? down_write+0x38/0x40 > [<ffffffff815bf038>] usb_audio_disconnect+0x88/0x1c0 > [<ffffffff814df460>] usb_unbind_interface+0x60/0x1a0 > [<ffffffff81436e4c>] __device_release_driver+0x7c/0xe0 > [<ffffffff814370bc>] device_release_driver+0x2c/0x40 > [<ffffffff8143681e>] bus_remove_device+0xee/0x160 > [<ffffffff81433c2a>] device_del+0x11a/0x1b0 > [<ffffffff814dce40>] usb_disable_device+0xb0/0x270 > [<ffffffff814d38ed>] usb_disconnect+0xad/0x160 > [<ffffffff814d613e>] hub_thread+0x6fe/0x1700 > [<ffffffff810b643e>] ? dequeue_task_fair+0xee/0x100 > [<ffffffff810a2c70>] ? finish_wait+0x80/0x80 > [<ffffffff814d5a40>] ? usb_remote_wakeup+0x70/0x70 > [<ffffffff810a2620>] kthread+0xc0/0xd0 > [<ffffffff810a2560>] ? kthread_create_on_node+0x130/0x130 > [<ffffffff816afb5c>] ret_from_fork+0x7c/0xb0 > [<ffffffff810a2560>] ? kthread_create_on_node+0x130/0x130 > sadc D ffff8801cbc92400 0 7883 7870 0x00000004 > ffff8801b9ab5d98 0000000000000082 ffff8801087b0d20 ffff8801b9ab5fd8 > ffff8801b9ab5fd8 ffff8801b9ab5fd8 ffff8801c487c830 ffff8801087b0d20 > ffff8801c0ca5380 ffff8801b99b08e8 ffff8801b99b08ec ffff8801087b0d20 > Call Trace: > [<ffffffff816ae109>] schedule+0x29/0x70 > [<ffffffff816ae38e>] schedule_preempt_disabled+0xe/0x10 > [<ffffffff816ad02a>] __mutex_lock_slowpath+0xca/0x140 > [<ffffffff816acb5a>] mutex_lock+0x2a/0x50 > [<ffffffff814e312b>] show_manufacturer+0x2b/0x60 > [<ffffffff81433910>] dev_attr_show+0x20/0x60 > [<ffffffff81129707>] ? __get_free_pages+0x17/0x50 > [<ffffffff811d6fc5>] sysfs_read_file+0xa5/0x1c0 > [<ffffffff8116dcb9>] vfs_read+0xa9/0x180 > [<ffffffff8116dde2>] sys_read+0x52/0xa0 > [<ffffffff816afc02>] system_call_fastpath+0x16/0x1b > > > thanks, > -- > js > suse labs >

1 0

Re: [alsa-devel] [RFC PATCH v4 2/2] ASoC: sam9g20-wm8731: convert to dt support
by Mark Brown 13 Nov '12

13 Nov '12

On Thu, Nov 08, 2012 at 04:24:31PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 15:57 Thu 08 Nov , Bo Shen wrote: > > Signed-off-by: Bo Shen <voice.shen(a)atmel.com> > > --- > > Change since v3 RFC: > > Remove dai node, remap ssc in place > > without pinctrl added, so don't modify the dtsi file, will be added > > soon > until I see the pinctrl to test this no > This code is untested I see a version adding the DT pinmux stuff has now been posted but this does seem a bit unfair, the code should be fine on a current ASoC branch - it's only going to break after merging with the pinctrl support.

1 0

[alsa-devel] Races in alsa-lib with threads
by Trent Piepho 13 Nov '12

13 Nov '12

We've found a race with alsa-lib functions are called from multiple threads. I was under the impression that alsa-lib was supposed to be thread safe. Is this not the case and all alsa calls should be done one from thread or protected by a mutex? The race we found was in sync_ptr1() in pcm_hw.c. This issues the ioctl() to get the current hw ptr from the kernel and send the app ptr to the kernel. The communication is done by passing a pointer to hw->sync_ptr, a field in the snd_pcm_hw_t structure for the pcm. Any thread calling sync_ptr1 will pass the same structure to the kernel. This should be a red flag for a race, having two threads write to the same data structure at the same time. This function gets called by snd_pcm_writei(), as part of that function getting the current hw pointer so it can calculate the avail value. And again when it actually writes data to commit the new app pointer to the kernel. It also gets called by snd_pcm_delay() if a rate plugin is used. snd_pcm_rate_delay() will end up calling snd_pcm_hw_hwsync() and that does a sync_ptr1() call. So imaging two threads, one writing data and calling snd_pcm_writei() and another calling snd_pcm_delay() as part of trying to sync audio playback. gstreamer does this! The ioctl is handled by snd_pcm_sync_ptr() in pcm_native.c. It gets the stream lock while it reads/writes the various fields a sync struct on the stack, then releases the lock and calls copy_to_user to write the sync struct from its stack to userspace. Here's how the race works. Thread 1 calls snd_pcm_delay(). sync_ptr1 is called snd_pcm_sync_ptr gets the lock, fills a struct with the current data, releases the lock. Does not call copy_to_user yet! Thread 2 runs! It calls snd_pcm_writei(), actually writes data too It calls sync_ptr1() to update the app pointer with the new data and returns The snd_pcm_writei() call returns Thread 1 runs again, calls copy_to_user The copy_to_user writes the OLD struct that was prepared back on the 3rd line into userspace The hw and app pointer values, as far as the userspace copy in alsa-lib is concerned, have regressed to what they were before the call to snd_pcm_writei()! You get an audio stutter as the data written is overwritten by the next write. The race turns out to not be that unlikely. The stream lock is held and then released before calling copy_to_user. When it's released irqs are enabled. If a period has elapsed while the lock was held, the irq handle runs now, the elapsing period will wake anything blocked in poll() waiting for space, which then probably gets to run immediately since the schedule likes tasks that have been sleeping and just woke, and the new task was probably blocked waiting to write data so that's what it does as soon as it runs.

4 6

[alsa-devel] ASoC updates for 3.7
by Mark Brown 13 Nov '12

13 Nov '12

The following changes since commit 77b67063bb6bce6d475e910d3b886a606d0d91f7: Linux 3.7-rc5 (2012-11-11 13:44:33 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tags/asoc-3.7 for you to fetch changes up to ba027da8eb309252cb611f23808da51487f68e12: Merge branches 'fix/arizona', 'fix/core', 'fix/cs42l52', 'fix/mxs', 'fix/samsung' and 'fix/wm8978' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into tmp (2012-11-13 15:13:29 +0900) ---------------------------------------------------------------- ASoC: Fixes for v3.7 A few small fixes plus a large but simple change for WM5102 which writes out a bunch of register updates to the device when we enable the clock as recommended following chip evaluation. ---------------------------------------------------------------- Charles Keepax (1): ASoC: bells: Correct type in sub speaker DAI name for WM5102 Dimitris Papastamos (2): ASoC: bells: Add missing select of WM0010 ASoC: bells: Select WM1250-EV1 Springbank audio I/O module Eric Millbrandt (1): ASoC: wm8978: pll incorrectly configured when codec is master Fabio Estevam (2): ASoC: mxs-saif: Add MODULE_ALIAS ASoC: mxs-saif: Fix channel swap for 24-bit format Mark Brown (2): ASoC: wm5102: Write register value corrections after SYSCLK is enabled Merge branches 'fix/arizona', 'fix/core', 'fix/cs42l52', 'fix/mxs', 'fix/samsung' and 'fix/wm8978' of git://git.kernel.org/.../broonie/sound into tmp Misael Lopez Cruz (1): ASoC: dapm: Use card_list during DAPM shutdown Mukund Navada (1): ASoC: core: Double control update err for snd_soc_put_volsw_sx Wei Yongjun (1): ASoC: cs42l52: fix the return value of cs42l52_set_fmt() sound/soc/codecs/cs42l52.c | 3 +- sound/soc/codecs/wm5102.c | 552 +++++++++++++++++++++++++++++++++++++++++++- sound/soc/codecs/wm8978.c | 2 +- sound/soc/mxs/mxs-saif.c | 17 +- sound/soc/samsung/Kconfig | 2 + sound/soc/samsung/bells.c | 2 +- sound/soc/soc-core.c | 5 +- sound/soc/soc-dapm.c | 2 +- 8 files changed, 573 insertions(+), 12 deletions(-)

2 1

[alsa-devel] [PATCH] ALSA: ASoC: cs4271: free allocated GPIO
by Daniel Mack 13 Nov '12

13 Nov '12

In case of probe deferral, the allocated GPIO line is not freed, which prevents it from being claimed and properly asserted in later attempts. Fix this by using devm_gpio_request(). Signed-off-by: Daniel Mack <zonque(a)gmail.com> Reported-by: Michael Hirsch <hirsch(a)teufel.de> Cc: Alexander Sverdlin <subaparts(a)yandex.ru> --- sound/soc/codecs/cs4271.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c index f994af3..e3f0a7f 100644 --- a/sound/soc/codecs/cs4271.c +++ b/sound/soc/codecs/cs4271.c @@ -485,7 +485,7 @@ static int cs4271_probe(struct snd_soc_codec *codec) gpio_nreset = cs4271plat->gpio_nreset; if (gpio_nreset >= 0) - if (gpio_request(gpio_nreset, "CS4271 Reset")) + if (devm_gpio_request(codec->dev, gpio_nreset, "CS4271 Reset")) gpio_nreset = -EINVAL; if (gpio_nreset >= 0) { /* Reset codec */ @@ -535,15 +535,10 @@ static int cs4271_probe(struct snd_soc_codec *codec) static int cs4271_remove(struct snd_soc_codec *codec) { struct cs4271_private *cs4271 = snd_soc_codec_get_drvdata(codec); - int gpio_nreset; - gpio_nreset = cs4271->gpio_nreset; - - if (gpio_is_valid(gpio_nreset)) { + if (gpio_is_valid(cs4271->gpio_nreset)) /* Set codec to the reset state */ - gpio_set_value(gpio_nreset, 0); - gpio_free(gpio_nreset); - } + gpio_set_value(cs4271->gpio_nreset, 0); return 0; }; -- 1.7.11.7

2 1