All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Henningsson <david.henningsson@canonical.com>
To: Trent Piepho <tpiepho@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Krakora Robert-B42341 <B42341@freescale.com>
Subject: Re: Races in alsa-lib with threads
Date: Wed, 14 Nov 2012 09:02:05 +0100	[thread overview]
Message-ID: <50A34FFD.4040603@canonical.com> (raw)
In-Reply-To: <CA+7tXii5Lecb3GW4GPgMg6UWYe1jmpEWi_FHM24_nEB5Q7YT0w@mail.gmail.com>

On 11/13/2012 08:41 PM, Trent Piepho wrote:
> On Tue, Nov 13, 2012 at 8:50 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> At Tue, 13 Nov 2012 13:39:24 +0000,
>> Krakora Robert-B42341 wrote:
>>> 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.
>>
>> No, sorry, we don't want to introduce pthread_lock in alsa-lib PCM
>> stuff.
>>
>
> The problem with introduction serialization outside alsa-lib is that
> you must now serialize entire ALSA calls.  The locks must be held for
> far too long.
>
> Consider snd_pcm_writei(), most of the time is usually spent blocked
> waiting for a period to elapse.  It is perfectly ok to call
> snd_pcm_delay() during this time.  But if one isn't allowed to make
> any other pcm calls during snd_pcm_writei() then this can't be done.
>
> It's a pretty big problem.  Most code using blocking mode is going to
> write another period as soon as the writei call returns from the last
> write.  It will spend almost all its time inside snd_pcm_writei() and
> thus will always be holding the app's pcm stream lock.  As soon as it
> releases the lock it grabs it again for the next write.  Another
> thread trying to call snd_pcm_delay() will virtually NEVER get a
> chance.  Not only is it unnecessary stopped from getting the delay
> while another thread is blocked inside writei(), but it won't get a
> chance to call it between writei() calls either.
>
> But there doesn't need to be a conflict, since the actual critical
> section that needs locking is very small, far smaller than the time
> spent sleeping inside writei() or wait().  How can just the critical
> section be protected without placing the locking inside alsa-lib?

Hmm, but the great majority of programs are not interested in accessing 
alsa-lib from multiple threads in the way that's currently unsafe. That 
includes programs who are very dependent on low latency. I would not 
like to see all these programs to suffer from the overhead of adding 
locks here and there, even if those locks are small.

You claim that the Gstreamer ALSA sink plugin accesses alsa-lib from two 
threads simultaneously. Could you elaborate on how this can happen, 
maybe it is easy to fix?



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

  parent reply	other threads:[~2012-11-14  8:02 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-10  2:03 Races in alsa-lib with threads Trent Piepho
2012-11-10 13:53 ` Takashi Iwai
2012-11-11 16:35   ` Jaroslav Kysela
2012-11-12 19:40     ` Rob Krakora
2012-11-13  6:32       ` Takashi Iwai
2012-11-13  7:14         ` Trent Piepho
2012-11-13  7:30           ` Takashi Iwai
     [not found]             ` <5175C6FEA820754B902FF6873E9285B00C985240@039-SN2MPN1-021.039d.mgd.msft.net>
2012-11-13 13:47               ` Krakora Robert-B42341
2012-11-13 13:50               ` Takashi Iwai
2012-11-13 14:04                 ` Krakora Robert-B42341
2012-11-13 14:18                   ` Takashi Iwai
2012-11-13 14:08                 ` Jaroslav Kysela
2012-11-13 14:15                   ` Krakora Robert-B42341
2012-11-13 14:19                   ` Takashi Iwai
2012-11-13 14:26                 ` Krakora Robert-B42341
2012-11-13 19:41                 ` Trent Piepho
2012-11-13 20:23                   ` Clemens Ladisch
2012-11-13 20:36                     ` Trent Piepho
2012-11-13 20:29                   ` Takashi Iwai
2012-11-13 20:55                     ` Krakora Robert-B42341
2012-11-13 21:11                       ` Krakora Robert-B42341
2012-11-13 20:52                   ` Krakora Robert-B42341
2012-11-14  8:02                   ` David Henningsson [this message]
2012-11-14 12:55                     ` Krakora Robert-B42341
2012-11-14 19:49                     ` Trent Piepho
2012-11-14 20:03                       ` Rémi Denis-Courmont
2012-11-14 20:06                         ` Krakora Robert-B42341
2012-11-14 20:54                         ` Trent Piepho
2012-11-14 21:19                           ` Rémi Denis-Courmont
2012-11-15  1:52                             ` Krakora Robert-B42341
2012-11-15  7:39                       ` Takashi Iwai
2012-11-15 13:05                         ` Krakora Robert-B42341

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=50A34FFD.4040603@canonical.com \
    --to=david.henningsson@canonical.com \
    --cc=B42341@freescale.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=tiwai@suse.de \
    --cc=tpiepho@gmail.com \
    /path/to/YOUR_REPLY

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

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