Fix more CPU hang-up bugs in dmix

- Fix CPU hang-up during snd_pcm_drain()
  A new internal function snd_pcm_wait_nocheck() to force to call poll().

- Clean up, fix status() callback of dmix

src/pcm/pcm.c

@@ -2188,10 +2188,6 @@ int snd_pcm_open_slave(snd_pcm_t **pcmp, snd_config_t *root,
  */
 int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
 {
-	struct pollfd pfd;
-	unsigned short revents;
-	int err, err_poll;
-	
 	if (snd_pcm_mmap_avail(pcm) >= pcm->avail_min) {
 		/* check more precisely */
 		switch (snd_pcm_state(pcm)) {
@@ -2205,6 +2201,21 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
 			return 1;
 		}
 	}
+	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
+ */
+int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
+{
+	struct pollfd pfd;
+	unsigned short revents;
+	int err, err_poll;
+	
 	err = snd_pcm_poll_descriptors(pcm, &pfd, 1);
 	if (err < 0)
 		return err;
@@ -2247,6 +2258,7 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
 #endif
 	return err_poll > 0 ? 1 : 0;
 }
+#endif
 
 /**
  * \brief Return number of frames ready to be read/written

src/pcm/pcm_direct.c

@@ -416,7 +416,7 @@ int snd_pcm_direct_async(snd_pcm_t *pcm, int sig, pid_t pid)
 }
 
 /* empty the timer read queue */
-static void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
+void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
 {
 	/* rbuf might be overwriten by multiple plugins */
 	/* we don't need the value */
@@ -440,15 +440,17 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
 	assert(pfds && nfds == 1 && revents);
 	events = pfds[0].revents;
 	if (events & POLLIN) {
+		snd_pcm_uframes_t avail;
 		int empty = 0;
 		snd_pcm_avail_update(pcm);
 		if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
 			events |= POLLOUT;
 			events &= ~POLLIN;
-			empty = snd_pcm_mmap_playback_avail(pcm) < pcm->avail_min;
+			avail = snd_pcm_mmap_playback_avail(pcm);
 		} else {
-			empty = snd_pcm_mmap_capture_avail(pcm) < pcm->avail_min;
+			avail = snd_pcm_mmap_capture_avail(pcm);
 		}
+		empty = avail < pcm->avail_min;
 		if (empty)
 			snd_pcm_direct_clear_timer_queue(dmix);
 		if (empty)

src/pcm/pcm_direct.h

@@ -145,6 +145,7 @@ int snd_pcm_direct_channel_info(snd_pcm_t *pcm, snd_pcm_channel_info_t * info);
 int snd_pcm_direct_mmap(snd_pcm_t *pcm);
 int snd_pcm_direct_munmap(snd_pcm_t *pcm);
 int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix);
+void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix);
 
 int snd_timer_async(snd_timer_t *timer, int sig, pid_t pid);
 struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm);

src/pcm/pcm_dmix.c

@@ -204,6 +204,18 @@ static void mix_areas(snd_pcm_direct_t *dmix,
 	}
 }
 
+/*
+ * if no concurrent access is allowed in the mixing routines, we need to protect
+ * the area via semaphore
+ */
+#ifdef NO_CONCURRENT_ACCESS
+#define dmix_down_sem(dmix) snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT)
+#define dmix_up_sem(dmix) snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT)
+#else
+#define dmix_down_sem(dmix)
+#define dmix_up_sem(dmix)
+#endif
+
 /*
  *  synchronize shm ring buffer with hardware
  */
@@ -224,27 +236,23 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm, snd_pcm_uframes_t size)
 	slave_appl_ptr = dmix->slave_appl_ptr % dmix->shmptr->s.buffer_size;
 	dmix->slave_appl_ptr += size;
 	dmix->slave_appl_ptr %= dmix->shmptr->s.boundary;
-	while (size > 0) {
+	dmix_down_sem(dmix);
+	for (;;) {
 		transfer = appl_ptr + size > pcm->buffer_size ? pcm->buffer_size - appl_ptr : size;
 		if (slave_appl_ptr + transfer > dmix->shmptr->s.buffer_size)
 			transfer = dmix->shmptr->s.buffer_size - slave_appl_ptr;
-		if (transfer) {
+		if (! transfer)
-#ifdef NO_CONCURRENT_ACCESS
+			break;
-			snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
+		mix_areas(dmix, src_areas, dst_areas, appl_ptr, slave_appl_ptr, transfer);
-#endif
-			mix_areas(dmix, src_areas, dst_areas, appl_ptr, slave_appl_ptr, transfer);
-#ifdef NO_CONCURRENT_ACCESS
-			snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
-#endif
-		}
 		if (transfer >= size)
-			return;
+			break;
 		size -= transfer;
 		slave_appl_ptr += transfer;
 		slave_appl_ptr %= dmix->shmptr->s.buffer_size;
 		appl_ptr += transfer;
 		appl_ptr %= pcm->buffer_size;
 	}
+	dmix_up_sem(dmix);
 }
 
 /*
@@ -270,8 +278,8 @@ static int _snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm, int do_slave_sync)
 	diff = slave_hw_ptr - old_slave_hw_ptr;
 	if (diff == 0)		/* fast path */
 		return 0;
-	if (dmix->state != SNDRV_PCM_STATE_RUNNING &&
+	if (dmix->state != SND_PCM_STATE_RUNNING &&
-	    dmix->state != SNDRV_PCM_STATE_DRAINING)
+	    dmix->state != SND_PCM_STATE_DRAINING)
 		/* not really started yet - don't update hw_ptr */
 		return 0;
 	if (diff < 0) {
@@ -282,18 +290,21 @@ static int _snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm, int do_slave_sync)
 	dmix->hw_ptr %= pcm->boundary;
 	if (pcm->stop_threshold >= pcm->boundary)	/* don't care */
 		return 0;
-	if ((avail = snd_pcm_mmap_playback_avail(pcm)) >= pcm->stop_threshold) {
+	avail = snd_pcm_mmap_playback_avail(pcm);
+	if (avail > dmix->avail_max)
+		dmix->avail_max = avail;
+	if (avail >= pcm->stop_threshold) {
 		struct timeval tv;
 		snd_pcm_direct_timer_stop(dmix);
 		gettimeofday(&tv, 0);
 		dmix->trigger_tstamp.tv_sec = tv.tv_sec;
 		dmix->trigger_tstamp.tv_nsec = tv.tv_usec * 1000L;
-		dmix->state = SND_PCM_STATE_XRUN;
+		if (dmix->state == SND_PCM_STATE_RUNNING) {
-		dmix->avail_max = avail;
+			dmix->state = SND_PCM_STATE_XRUN;
-		return -EPIPE;
+			return -EPIPE;
+		}
+		dmix->state = SND_PCM_STATE_SETUP;
 	}
-	if (avail > dmix->avail_max)
-		dmix->avail_max = avail;
 	return 0;
 }
 
@@ -307,10 +318,27 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm)
  *  plugin implementation
  */
 
-static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
+static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm)
 {
 	snd_pcm_direct_t *dmix = pcm->private_data;
 	snd_pcm_state_t state;
+	state = snd_pcm_state(dmix->spcm);
+	switch (state) {
+	case SND_PCM_STATE_SUSPENDED:
+		return state;
+	case SND_PCM_STATE_DISCONNECTED:
+		return state;
+	default:
+		break;
+	}
+	if (dmix->state == STATE_RUN_PENDING)
+		return SNDRV_PCM_STATE_RUNNING;
+	return dmix->state;
+}
+
+static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
+{
+	snd_pcm_direct_t *dmix = pcm->private_data;
 
 	switch (dmix->state) {
 	case SNDRV_PCM_STATE_DRAINING:
@@ -321,15 +349,7 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
 		break;
 	}
 	memset(status, 0, sizeof(*status));
-	if (dmix->state == STATE_RUN_PENDING)
+	status->state = snd_pcm_dmix_state(pcm);
-		status->state = SNDRV_PCM_STATE_RUNNING;
-	else {
-		state = snd_pcm_state(dmix->spcm);
-		if (state == SNDRV_PCM_STATE_RUNNING)
-			status->state = dmix->state;
-		else
-			status->state = state;
-	}
 	status->trigger_tstamp = dmix->trigger_tstamp;
 	status->tstamp = snd_pcm_hw_fast_tstamp(dmix->spcm);
 	status->avail = snd_pcm_mmap_playback_avail(pcm);
@@ -338,23 +358,6 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
 	return 0;
 }
 
-static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm)
-{
-	snd_pcm_direct_t *dmix = pcm->private_data;
-	switch (snd_pcm_state(dmix->spcm)) {
-	case SND_PCM_STATE_SUSPENDED:
-		return SND_PCM_STATE_SUSPENDED;
-	case SND_PCM_STATE_DISCONNECTED:
-		dmix->state = SND_PCM_STATE_DISCONNECTED;
-		return -ENOTTY;
-	default:
-		break;
-	}
-	if (dmix->state == STATE_RUN_PENDING)
-		return SNDRV_PCM_STATE_RUNNING;
-	return dmix->state;
-}
-
 static int snd_pcm_dmix_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
 {
 	snd_pcm_direct_t *dmix = pcm->private_data;
@@ -366,6 +369,7 @@ static int snd_pcm_dmix_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
 		err = snd_pcm_dmix_sync_ptr(pcm);
 		if (err < 0)
 			return err;
+		/* fallthru */
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_SUSPENDED:
 	case STATE_RUN_PENDING:
@@ -481,22 +485,28 @@ static int snd_pcm_dmix_drain(snd_pcm_t *pcm)
 
 	if (dmix->state == SND_PCM_STATE_OPEN)
 		return -EBADFD;
-	stop_threshold = pcm->stop_threshold;
+	if (pcm->mode & SND_PCM_NONBLOCK)
-	if (pcm->stop_threshold > pcm->buffer_size)
+		return -EAGAIN;
-		pcm->stop_threshold = pcm->buffer_size;
 	if (dmix->state == SND_PCM_STATE_PREPARED &&
 	    snd_pcm_mmap_playback_hw_avail(pcm) > 0)
 		snd_pcm_dmix_start(pcm);
-	while (dmix->state == SND_PCM_STATE_RUNNING) {
+	stop_threshold = pcm->stop_threshold;
+	if (pcm->stop_threshold > pcm->buffer_size)
+		pcm->stop_threshold = pcm->buffer_size;
+	dmix->state = SND_PCM_STATE_DRAINING;
+	do {
 		err = snd_pcm_dmix_sync_ptr(pcm);
-		if (err < 0)
+		if (err < 0) {
-			break;
+			snd_pcm_dmix_drop(pcm);
-		if (pcm->mode & SND_PCM_NONBLOCK)
+			return err;
-			return -EAGAIN;
+		}
-		snd_pcm_wait(pcm, -1);
+		if (dmix->state == SND_PCM_STATE_DRAINING) {
-	}
+			snd_pcm_wait_nocheck(pcm, -1);
+			snd_pcm_direct_clear_timer_queue(dmix); /* force poll to wait */
+		}
+	} while (dmix->state == SND_PCM_STATE_DRAINING);
 	pcm->stop_threshold = stop_threshold;
-	return snd_pcm_dmix_drop(pcm);
+	return 0;
 }
 
 static int snd_pcm_dmix_pause(snd_pcm_t *pcm, int enable)
@@ -603,15 +613,17 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm,
 	default:
 		break;
 	}
+	if (! size)
+		return 0;
+	snd_pcm_mmap_appl_forward(pcm, size);
 	if (dmix->state == STATE_RUN_PENDING) {
 		if ((err = snd_pcm_dmix_start_timer(dmix)) < 0)
 			return err;
-	}
+	} else if (dmix->state == SND_PCM_STATE_RUNNING ||
-	snd_pcm_mmap_appl_forward(pcm, size);
+		   dmix->state == SND_PCM_STATE_DRAINING)
-	if (dmix->state == SND_PCM_STATE_RUNNING) {
+		_snd_pcm_dmix_sync_ptr(pcm, 1);
-		err = snd_pcm_dmix_sync_ptr(pcm);
+	if (dmix->state == SND_PCM_STATE_RUNNING ||
-		if (err < 0)
+	    dmix->state == SND_PCM_STATE_DRAINING) {
-			return err;
 		/* ok, we commit the changes after the validation of area */
 		/* it's intended, although the result might be crappy */
 		snd_pcm_dmix_sync_area(pcm, size);
@@ -622,13 +634,10 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm,
 static snd_pcm_sframes_t snd_pcm_dmix_avail_update(snd_pcm_t *pcm ATTRIBUTE_UNUSED)
 {
 	snd_pcm_direct_t *dmix = pcm->private_data;
-	int err;
 	
-	if (dmix->state == SND_PCM_STATE_RUNNING) {
+	if (dmix->state == SND_PCM_STATE_RUNNING ||
-		err = snd_pcm_dmix_sync_ptr(pcm);
+	    dmix->state == SND_PCM_STATE_DRAINING)
-		if (err < 0)
+		snd_pcm_dmix_sync_ptr(pcm);
-			return err;
-	}
 	return snd_pcm_mmap_playback_avail(pcm);
 }
 

src/pcm/pcm_local.h

@@ -744,6 +744,8 @@ int snd_pcm_conf_generic_id(const char *id);
 
 int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, int mmap_emulation, int sync_ptr_ioctl);
 
+int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout);
+
 #define SND_PCM_HW_PARBIT_ACCESS	(1U << SND_PCM_HW_PARAM_ACCESS)
 #define SND_PCM_HW_PARBIT_FORMAT	(1U << SND_PCM_HW_PARAM_FORMAT)
 #define SND_PCM_HW_PARBIT_SUBFORMAT	(1U << SND_PCM_HW_PARAM_SUBFORMAT)