Duplicate wake-ups in pcm_lib.c

[pl bossart <bossart.nospam@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

Thanks to Takashi's advice, I managed to find out the reason why I was
seeing null events returned by poll(). This could explain why
PulseAudio doesn't seem to sleep much. It turns out that we have two
calls to wakeup() in pcm_lib.c, and a nice race condition it seems.
See the log below.

A wake-up is generated during the period interrupt, and a second
wake-up is generated during the write loop, after the application was
awaken but just before the pointers are updated. This second wake-up
shouldn't exist, since the write loop actually fills the ring buffer.
By the time the second wake-up is actually handled, there's really no
space left in the buffer and a null event is generated; it'll wake-up
the application a second time for nothing. Maybe we should move the
call to snd_pcm_update_hw_ptr() after the transfer took place?

Can anyone apply the attached patches (only adds printks) and confirm
this issue on their platforms? That would be very useful.
Happy holidays everyone
- Pierre


alsa-lib/test/pcm --verbose -Dhw -c2 -r48000 -f440  --method write_and_poll

[  371.369865] snd_intel8x0_interrupt
[  371.369876]  snd_intel8x0_update
[  371.369884] snd_pcm_period_elapsed
[  371.369890]  snd_pcm_update_hw_ptr_interrupt
[  371.369900]    snd_pcm_update_hw_ptr_post
[  371.369909]     wakeup in snd_pcm_update_hw_ptr_post
sound/core/pcm_lib.c: 214, avail 8194, avail_min 8192
[  371.369935] ALSA: poll event  POLLOUT|POLLWRNORM, avail 8194 avail_min 8192
[  371.375449] snd_pcm_lib_write1: in
[  371.375457] snd_pcm_lib_write1 loop
[  371.375462]  snd_pcm_update_hw_ptr
[  371.375472]    snd_pcm_update_hw_ptr_post
[  371.375481]     wakeup in snd_pcm_update_hw_ptr_post
sound/core/pcm_lib.c: 214, avail 8462, avail_min 8192

>>Between these two points, the appl_ptr is updated, and there's no space left in the buffer. The wake-up is generated too early.

[  371.375514] snd_pcm_lib_write1: out
[  371.375524] ALSA: null poll event, avail 270 avail_min 8192
[  371.540532] snd_intel8x0_interrupt
[  371.540542]  snd_intel8x0_update
[  371.540551] snd_pcm_period_elapsed
[  371.540556]  snd_pcm_update_hw_ptr_interrupt
[  371.540566]    snd_pcm_update_hw_ptr_post
[  371.540575]     wakeup in snd_pcm_update_hw_ptr_post
sound/core/pcm_lib.c: 214, avail 8194, avail_min 8192
[  371.540601] ALSA: poll event  POLLOUT|POLLWRNORM, avail 8194 avail_min 8192
[  371.543734] snd_pcm_lib_write1: in
[  371.543741] snd_pcm_lib_write1 loop
[  371.543746]  snd_pcm_update_hw_ptr
[  371.543755]    snd_pcm_update_hw_ptr_post
[  371.543764]     wakeup in snd_pcm_update_hw_ptr_post
sound/core/pcm_lib.c: 214, avail 8347, avail_min 8192
[  371.543797] snd_pcm_lib_write1: out
[  371.543807] ALSA: null poll event, avail 155 avail_min 8192

[-- Attachment #2: core_pcm.patch --]
[-- Type: text/x-patch, Size: 4359 bytes --]

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 30f4108..d6f831f 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -191,6 +191,8 @@ static int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream,
 {
 	snd_pcm_uframes_t avail;
 
+        printk(KERN_ERR "   snd_pcm_update_hw_ptr_post");
+        
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		avail = snd_pcm_playback_avail(runtime);
 	else
@@ -208,8 +210,10 @@ static int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream,
 			return -EPIPE;
 		}
 	}
-	if (avail >= runtime->control->avail_min)
+	if (avail >= runtime->control->avail_min) {
+                printk(KERN_ERR "    wakeup in snd_pcm_update_hw_ptr_post %s: %d, avail %d, avail_min %d",__FILE__,__LINE__,avail,runtime->control->avail_min );
 		wake_up(&runtime->sleep);
+        }
 	return 0;
 }
 
@@ -231,6 +235,8 @@ static int snd_pcm_update_hw_ptr_interrupt(struct snd_pcm_substream *substream)
 	snd_pcm_sframes_t hdelta, delta;
 	unsigned long jdelta;
 
+        printk(KERN_ERR " snd_pcm_update_hw_ptr_interrupt");
+        
 	old_hw_ptr = runtime->status->hw_ptr;
 	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
 	if (pos == SNDRV_PCM_POS_XRUN) {
@@ -363,6 +369,8 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
 	snd_pcm_sframes_t delta;
 	unsigned long jdelta;
 
+        printk(KERN_ERR " snd_pcm_update_hw_ptr");
+        
 	old_hw_ptr = runtime->status->hw_ptr;
 	pos = snd_pcm_update_hw_ptr_pos(substream, runtime);
 	if (pos == SNDRV_PCM_POS_XRUN) {
@@ -1634,6 +1642,8 @@ void snd_pcm_period_elapsed(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime;
 	unsigned long flags;
 
+        printk(KERN_ERR "snd_pcm_period_elapsed");
+        
 	if (PCM_RUNTIME_CHECK(substream))
 		return;
 	runtime = substream->runtime;
@@ -1756,6 +1766,8 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream,
 	snd_pcm_uframes_t offset = 0;
 	int err = 0;
 
+        printk(KERN_ERR "snd_pcm_lib_write1: in");
+        
 	if (size == 0)
 		return 0;
 
@@ -1780,8 +1792,10 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream,
 		snd_pcm_uframes_t frames, appl_ptr, appl_ofs;
 		snd_pcm_uframes_t avail;
 		snd_pcm_uframes_t cont;
-		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING)
-			snd_pcm_update_hw_ptr(substream);
+		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) {
+                    printk(KERN_ERR "snd_pcm_lib_write1 loop");
+                    snd_pcm_update_hw_ptr(substream);
+                }
 		avail = snd_pcm_playback_avail(runtime);
 		if (!avail) {
 			if (nonblock) {
@@ -1836,6 +1850,7 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream,
  _end_unlock:
 	snd_pcm_stream_unlock_irq(substream);
  _end:
+        printk(KERN_ERR "snd_pcm_lib_write1: out");
 	return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
 }
 
@@ -1998,7 +2013,10 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream,
 		snd_pcm_uframes_t avail;
 		snd_pcm_uframes_t cont;
 		if (runtime->status->state == SNDRV_PCM_STATE_RUNNING)
-			snd_pcm_update_hw_ptr(substream);
+                {
+                    printk(KERN_ERR "snd_pcm_lib_read1 loop");
+                    snd_pcm_update_hw_ptr(substream);
+                }
 		avail = snd_pcm_capture_avail(runtime);
 		if (!avail) {
 			if (runtime->status->state ==
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 29ab46a..048052a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2903,13 +2903,16 @@ static unsigned int snd_pcm_playback_poll(struct file *file, poll_table * wait)
 	case SNDRV_PCM_STATE_PAUSED:
 		if (avail >= runtime->control->avail_min) {
 			mask = POLLOUT | POLLWRNORM;
+                        printk(KERN_ERR "ALSA: poll event  POLLOUT|POLLWRNORM, avail %d avail_min %d",avail,runtime->control->avail_min );
 			break;
 		}
+                printk(KERN_ERR "ALSA: null poll event, avail %d avail_min %d",avail,runtime->control->avail_min );
 		/* Fall through */
 	case SNDRV_PCM_STATE_DRAINING:
 		mask = 0;
 		break;
 	default:
+                printk(KERN_ERR "ALSA: default, avail %d avail_min %d",avail,runtime->control->avail_min );
 		mask = POLLOUT | POLLWRNORM | POLLERR;
 		break;
 	}

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel