From patchwork Thu Mar 10 09:14:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 550529 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B8005C433F5 for ; Thu, 10 Mar 2022 09:16:13 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 060A417EF; Thu, 10 Mar 2022 10:15:22 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 060A417EF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646903772; bh=kCzNPSwBFUuFadK9QRN3ClPIwgropEinjgLt3i9UEWI=; h=From:To:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From; b=dwthv4ArwsNZXw8/DpIJy7KxF1qWmvtLHd0DL6fZhiWSEHZkArgGXCaqZKlYrQU+3 JD4rJnV3ql/0pjlngs2WK76bMnymMagB4/4yw2+9XVJ4qT9O5FIQJ6SjlGmGqmvLdN gdAIrmLBPSyzWfsn+nARBKGc1QzOkHdjZ2IeevrA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 78D9FF80518; Thu, 10 Mar 2022 10:14:31 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id C7F1DF80137; Thu, 10 Mar 2022 10:14:29 +0100 (CET) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 33C8CF80137 for ; Thu, 10 Mar 2022 10:14:21 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 33C8CF80137 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="ej1+AmsG"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="6UbrX8//" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id E3F951F381 for ; Thu, 10 Mar 2022 09:14:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646903660; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=+BVXA2J1VQNo7/arswMX9Tfy8uNymZwrQnW7iDEiUOk=; b=ej1+AmsGkV9+ZZHlB6HULKtVelM4L4n1EpAiwXk0bYZzzDDs9Hx2HL/XqHH+adVknVXjmr 72j2BQK/OqiudenkyDwAK4dJyYFFuTq4Rxt0+tsAR8BTpGtvyBGolInp26tLvbSF4J1jpX lwMHUgjifJwQIV+SiaMGR16/x3c9pMQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646903660; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=+BVXA2J1VQNo7/arswMX9Tfy8uNymZwrQnW7iDEiUOk=; b=6UbrX8//PZrKKdTH43g6+vLadpJ1JpWbCTi2pwVVRFSrZ92xmZBctprzJTSq4B/9jAetrQ DeKAsdEoOmAUPyCw== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id D1B12A3B81; Thu, 10 Mar 2022 09:14:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib 1/3] pcm: direct: Improved suspend/resume support Date: Thu, 10 Mar 2022 10:14:16 +0100 Message-Id: <20220310091418.1150-1-tiwai@suse.de> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" The current resume handling in PCM direct plugins don't treat multiple clients properly: once after the slave PCM gets resumed by one client, the access from others at a later point is seen as already running although the internal state isn't updated and becomes inconsistent. This may end up a negative size, which eventually hangs up. This patch is an attempt to improve the handling for resume. Now the suspended state is treated similarly like XRUN; namely, we keep the slave PCM "recoveries" count that is modified at each time the slave PCM XRUN happens, so that we can check the inconsistency against the client's state. As a differentiation to XRUN, we set the highest bit of recoveries count when the slave stream hits SUSPENDED state. This bit is referred at comparing with clients, and the client's state is updated to either XRUN or SUSPENDED depending on this bit. Along with this change, the actual resume is done in snd_pcm_direct_slave_recover(), and snd_pcm_direct_resume() rather calls this internally. Signed-off-by: Takashi Iwai --- src/pcm/pcm_direct.c | 72 ++++++++++++++++++++++++++------------------ src/pcm/pcm_dmix.c | 6 ++-- src/pcm/pcm_dshare.c | 6 ++-- src/pcm/pcm_dsnoop.c | 6 ++-- 4 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 193dc3e76d49..1ca3b76306b1 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -560,8 +560,11 @@ int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix) return 0; } +#define RECOVERIES_FLAG_SUSPENDED (1U << 31) +#define RECOVERIES_MASK ((1U << 31) - 1) + /* - * Recover slave on XRUN. + * Recover slave on XRUN or SUSPENDED. * Even if direct plugins disable xrun detection, there might be an xrun * raised directly by some drivers. * The first client recovers slave pcm. @@ -569,6 +572,8 @@ int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix) */ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) { + unsigned int recoveries; + int state; int ret; int semerr; @@ -579,7 +584,8 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) return semerr; } - if (snd_pcm_state(direct->spcm) != SND_PCM_STATE_XRUN) { + state = snd_pcm_state(direct->spcm); + if (state != SND_PCM_STATE_XRUN && state != SND_PCM_STATE_SUSPENDED) { /* ignore... someone else already did recovery */ semerr = snd_pcm_direct_semaphore_up(direct, DIRECT_IPC_SEM_CLIENT); @@ -590,6 +596,24 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) return 0; } + recoveries = direct->shmptr->s.recoveries; + recoveries = (recoveries + 1) & RECOVERIES_MASK; + if (state == SND_PCM_STATE_SUSPENDED) + recoveries |= RECOVERIES_FLAG_SUSPENDED; + direct->shmptr->s.recoveries = recoveries; + + /* some buggy drivers require the device resumed before prepared; + * when a device has RESUME flag and is in SUSPENDED state, resume + * here but immediately drop to bring it to a sane active state. + */ + if (state == SND_PCM_STATE_SUSPENDED && + (direct->spcm->info & SND_PCM_INFO_RESUME)) { + snd_pcm_resume(direct->spcm); + snd_pcm_drop(direct->spcm); + snd_pcm_direct_timer_stop(direct); + snd_pcm_direct_clear_timer_queue(direct); + } + ret = snd_pcm_prepare(direct->spcm); if (ret < 0) { SNDERR("recover: unable to prepare slave"); @@ -621,7 +645,6 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) } return ret; } - direct->shmptr->s.recoveries++; semerr = snd_pcm_direct_semaphore_up(direct, DIRECT_IPC_SEM_CLIENT); if (semerr < 0) { @@ -632,11 +655,15 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) } /* - * enter xrun state, if slave xrun occurred - * @return: 0 for no xrun or a negative error code for xrun + * enter xrun or suspended state, if slave xrun occurred or suspended + * @return: 0 for no xrun/suspend or a negative error code for xrun/suspend */ int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm) { + if (direct->state == SND_PCM_STATE_XRUN) + return -EPIPE; + else if (direct->state == SND_PCM_STATE_SUSPENDED) + return -ESTRPIPE; if (direct->shmptr->s.recoveries != direct->recoveries) { /* no matter how many xruns we missed - * so don't increment but just update to actual counter @@ -649,10 +676,14 @@ int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm) * if slave already entered xrun again the event is lost. * snd_pcm_direct_clear_timer_queue(direct); */ - direct->state = SND_PCM_STATE_XRUN; + if (direct->recoveries & RECOVERIES_FLAG_SUSPENDED) { + direct->state = SND_PCM_STATE_SUSPENDED; + return -ESTRPIPE; + } else { + direct->state = SND_PCM_STATE_XRUN; + return -EPIPE; + } } - if (direct->state == SND_PCM_STATE_XRUN) - return -EPIPE; return 0; } @@ -721,13 +752,13 @@ timer_changed: } switch (snd_pcm_state(dmix->spcm)) { case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: /* recover slave and update client state to xrun * before returning POLLERR */ snd_pcm_direct_slave_recover(dmix); snd_pcm_direct_client_chk_xrun(dmix, pcm); /* fallthrough */ - case SND_PCM_STATE_SUSPENDED: case SND_PCM_STATE_SETUP: events |= POLLERR; break; @@ -1074,27 +1105,10 @@ int snd_pcm_direct_prepare(snd_pcm_t *pcm) int snd_pcm_direct_resume(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data; - snd_pcm_t *spcm = dmix->spcm; + int err; - snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT); - /* some buggy drivers require the device resumed before prepared; - * when a device has RESUME flag and is in SUSPENDED state, resume - * here but immediately drop to bring it to a sane active state. - */ - if ((spcm->info & SND_PCM_INFO_RESUME) && - snd_pcm_state(spcm) == SND_PCM_STATE_SUSPENDED) { - snd_pcm_resume(spcm); - snd_pcm_drop(spcm); - snd_pcm_direct_timer_stop(dmix); - snd_pcm_direct_clear_timer_queue(dmix); - snd_pcm_areas_silence(snd_pcm_mmap_areas(spcm), 0, - spcm->channels, spcm->buffer_size, - spcm->format); - snd_pcm_prepare(spcm); - snd_pcm_start(spcm); - } - snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT); - return -ENOSYS; + err = snd_pcm_direct_slave_recover(dmix); + return err < 0 ? err : -ENOSYS; } #define COPY_SLAVE(field) (dmix->shmptr->s.field = spcm->field) diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index dfff18b992c5..e3bf49269f0c 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -431,6 +431,7 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) dmix->state = SND_PCM_STATE_DISCONNECTED; return -ENODEV; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) return err; break; @@ -457,11 +458,11 @@ static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm) snd_pcm_state_t state; state = snd_pcm_state(dmix->spcm); switch (state) { - case SND_PCM_STATE_SUSPENDED: case SND_PCM_STATE_DISCONNECTED: dmix->state = state; return state; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) return err; break; @@ -833,11 +834,10 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm, switch (snd_pcm_state(dmix->spcm)) { case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) return err; break; - case SND_PCM_STATE_SUSPENDED: - return -ESTRPIPE; default: break; } diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index d63c86bd4044..5a52ae9173be 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -206,6 +206,7 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) dshare->state = SNDRV_PCM_STATE_DISCONNECTED; return -ENODEV; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) return err; break; @@ -260,11 +261,11 @@ static snd_pcm_state_t snd_pcm_dshare_state(snd_pcm_t *pcm) snd_pcm_state_t state; state = snd_pcm_state(dshare->spcm); switch (state) { - case SND_PCM_STATE_SUSPENDED: case SND_PCM_STATE_DISCONNECTED: dshare->state = state; return state; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) return err; break; @@ -532,11 +533,10 @@ static snd_pcm_sframes_t snd_pcm_dshare_mmap_commit(snd_pcm_t *pcm, switch (snd_pcm_state(dshare->spcm)) { case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) return err; break; - case SND_PCM_STATE_SUSPENDED: - return -ESTRPIPE; default: break; } diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 77ffbada931f..1653f6fba86c 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -139,6 +139,7 @@ static int snd_pcm_dsnoop_sync_ptr(snd_pcm_t *pcm) dsnoop->state = SNDRV_PCM_STATE_DISCONNECTED; return -ENODEV; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) return err; break; @@ -211,11 +212,11 @@ static snd_pcm_state_t snd_pcm_dsnoop_state(snd_pcm_t *pcm) snd_pcm_state_t state; state = snd_pcm_state(dsnoop->spcm); switch (state) { - case SND_PCM_STATE_SUSPENDED: case SND_PCM_STATE_DISCONNECTED: dsnoop->state = state; return state; case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) return err; break; @@ -423,11 +424,10 @@ static snd_pcm_sframes_t snd_pcm_dsnoop_mmap_commit(snd_pcm_t *pcm, switch (snd_pcm_state(dsnoop->spcm)) { case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) return err; break; - case SND_PCM_STATE_SUSPENDED: - return -ESTRPIPE; default: break; } From patchwork Thu Mar 10 09:14:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 550530 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C819EC433EF for ; Thu, 10 Mar 2022 09:15:23 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 3292A17D6; Thu, 10 Mar 2022 10:14:31 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 3292A17D6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646903721; bh=oRaMzcOh9rJ4WgQEyP7jCP/sWr9gsy7yrax4Ew44fYY=; h=From:To:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=FW2U2GzZHDPWUzRB2nGWh9N+ibtctVBIpMRWM1jYPlZb4keQwskXFMXGFxbwB8NFJ 5ceugdP0eLMFpAR2WihJMiH53fFpnJnXqupilrOX6kXSkQDYXuVZCJTjgK4uPkjqZy SQh5g2g2cxhJNEFLMb9icRxHZk5DzyzY8ax0BNuw= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id AAFB3F8012F; Thu, 10 Mar 2022 10:14:29 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 29D98F8015B; Thu, 10 Mar 2022 10:14:28 +0100 (CET) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 1DF1DF800D2 for ; Thu, 10 Mar 2022 10:14:21 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 1DF1DF800D2 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="0wA7XufR"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="03xRZO5U" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id F35B61F442 for ; Thu, 10 Mar 2022 09:14:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646903661; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=IILEMp/lj25UrsVwunNUjmzD09rDI/aZR1suYMneNk0=; b=0wA7XufR78iJ+pWgSiWSaC2TPgJD2eAdXEKWzWbJLv3i7qQmK0Uboq0el8IkjH7+rBei2e qkzxBuuAwQ26+yIHADlop1HVhwPcNOb6VZyvYupPtkj26oPQGirq4uPK3HtvWgaad3/wKz oyvgMdkBV80fTkvTWZbyeZuh46UPRSI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646903661; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=IILEMp/lj25UrsVwunNUjmzD09rDI/aZR1suYMneNk0=; b=03xRZO5Uu3x5x5fJFig7R+00W+Z7gbfHmRBl2YuPow0LxIPqDiklUc8UGEM2NJDrwPG1ho Ik+triHrKUcj0ZDA== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id E2A41A3B83; Thu, 10 Mar 2022 09:14:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib 2/3] pcm: direct: Move slave PCM state checks into XRUN check helper Date: Thu, 10 Mar 2022 10:14:17 +0100 Message-Id: <20220310091418.1150-2-tiwai@suse.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20220310091418.1150-1-tiwai@suse.de> References: <20220310091418.1150-1-tiwai@suse.de> MIME-Version: 1.0 X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" The check of slave PCM state is always done before the client's recoveries count check, so let's merge them to the common helper. Also rename the helper function to snd_pcm_direct_check_xrun() as it's checking both slave and client states now. Signed-off-by: Takashi Iwai --- src/pcm/pcm_direct.c | 34 ++++++++++++++++++++-------------- src/pcm/pcm_direct.h | 2 +- src/pcm/pcm_dmix.c | 43 ++++--------------------------------------- src/pcm/pcm_dshare.c | 43 ++++--------------------------------------- src/pcm/pcm_dsnoop.c | 43 ++++--------------------------------------- 5 files changed, 33 insertions(+), 132 deletions(-) diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 1ca3b76306b1..060bcd5a0f7b 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -658,8 +658,23 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) * enter xrun or suspended state, if slave xrun occurred or suspended * @return: 0 for no xrun/suspend or a negative error code for xrun/suspend */ -int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm) +int snd_pcm_direct_check_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm) { + int err; + + switch (snd_pcm_state(direct->spcm)) { + case SND_PCM_STATE_DISCONNECTED: + direct->state = SNDRV_PCM_STATE_DISCONNECTED; + return -ENODEV; + case SND_PCM_STATE_XRUN: + case SND_PCM_STATE_SUSPENDED: + if ((err = snd_pcm_direct_slave_recover(direct)) < 0) + return err; + break; + default: + break; + } + if (direct->state == SND_PCM_STATE_XRUN) return -EPIPE; else if (direct->state == SND_PCM_STATE_SUSPENDED) @@ -750,19 +765,11 @@ timer_changed: } empty = avail < pcm->avail_min; } - switch (snd_pcm_state(dmix->spcm)) { - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - /* recover slave and update client state to xrun - * before returning POLLERR - */ - snd_pcm_direct_slave_recover(dmix); - snd_pcm_direct_client_chk_xrun(dmix, pcm); - /* fallthrough */ - case SND_PCM_STATE_SETUP: + + if (snd_pcm_direct_check_xrun(dmix, pcm) < 0 || + snd_pcm_state(dmix->spcm) == SND_PCM_STATE_SETUP) { events |= POLLERR; - break; - default: + } else { if (empty) { /* here we have a race condition: * if period event arrived after the avail_update call @@ -786,7 +793,6 @@ timer_changed: break; } } - break; } *revents = events; return 0; diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h index fb013a6666c2..3e0c8bfcc9bc 100644 --- a/src/pcm/pcm_direct.h +++ b/src/pcm/pcm_direct.h @@ -345,7 +345,7 @@ snd_pcm_chmap_query_t **snd_pcm_direct_query_chmaps(snd_pcm_t *pcm); snd_pcm_chmap_t *snd_pcm_direct_get_chmap(snd_pcm_t *pcm); int snd_pcm_direct_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map); int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct); -int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm); +int snd_pcm_direct_check_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm); int snd_timer_async(snd_timer_t *timer, int sig, pid_t pid); struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm); void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix); diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index e3bf49269f0c..d00d53bef604 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -426,19 +426,7 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dmix = pcm->private_data; int err; - switch (snd_pcm_state(dmix->spcm)) { - case SND_PCM_STATE_DISCONNECTED: - dmix->state = SND_PCM_STATE_DISCONNECTED; - return -ENODEV; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dmix, pcm); + err = snd_pcm_direct_check_xrun(dmix, pcm); if (err < 0) return err; if (dmix->slowptr) @@ -454,22 +442,8 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data; - int err; - snd_pcm_state_t state; - state = snd_pcm_state(dmix->spcm); - switch (state) { - case SND_PCM_STATE_DISCONNECTED: - dmix->state = state; - return state; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) - return err; - break; - default: - break; - } - snd_pcm_direct_client_chk_xrun(dmix, pcm); + + snd_pcm_direct_check_xrun(dmix, pcm); if (dmix->state == STATE_RUN_PENDING) return SNDRV_PCM_STATE_RUNNING; return dmix->state; @@ -832,16 +806,7 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm, snd_pcm_direct_t *dmix = pcm->private_data; int err; - switch (snd_pcm_state(dmix->spcm)) { - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dmix)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dmix, pcm); + err = snd_pcm_direct_check_xrun(dmix, pcm); if (err < 0) return err; if (! size) diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index 5a52ae9173be..0ff43a90d270 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -201,19 +201,7 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; int err; - switch (snd_pcm_state(dshare->spcm)) { - case SND_PCM_STATE_DISCONNECTED: - dshare->state = SNDRV_PCM_STATE_DISCONNECTED; - return -ENODEV; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dshare, pcm); + err = snd_pcm_direct_check_xrun(dshare, pcm); if (err < 0) return err; if (dshare->slowptr) @@ -257,22 +245,8 @@ static int snd_pcm_dshare_status(snd_pcm_t *pcm, snd_pcm_status_t * status) static snd_pcm_state_t snd_pcm_dshare_state(snd_pcm_t *pcm) { snd_pcm_direct_t *dshare = pcm->private_data; - int err; - snd_pcm_state_t state; - state = snd_pcm_state(dshare->spcm); - switch (state) { - case SND_PCM_STATE_DISCONNECTED: - dshare->state = state; - return state; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) - return err; - break; - default: - break; - } - snd_pcm_direct_client_chk_xrun(dshare, pcm); + + snd_pcm_direct_check_xrun(dshare, pcm); if (dshare->state == STATE_RUN_PENDING) return SNDRV_PCM_STATE_RUNNING; return dshare->state; @@ -531,16 +505,7 @@ static snd_pcm_sframes_t snd_pcm_dshare_mmap_commit(snd_pcm_t *pcm, snd_pcm_direct_t *dshare = pcm->private_data; int err; - switch (snd_pcm_state(dshare->spcm)) { - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dshare)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dshare, pcm); + err = snd_pcm_direct_check_xrun(dshare, pcm); if (err < 0) return err; if (! size) diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 1653f6fba86c..729ff447b41f 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -134,19 +134,7 @@ static int snd_pcm_dsnoop_sync_ptr(snd_pcm_t *pcm) snd_pcm_sframes_t diff; int err; - switch (snd_pcm_state(dsnoop->spcm)) { - case SND_PCM_STATE_DISCONNECTED: - dsnoop->state = SNDRV_PCM_STATE_DISCONNECTED; - return -ENODEV; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dsnoop, pcm); + err = snd_pcm_direct_check_xrun(dsnoop, pcm); if (err < 0) return err; if (dsnoop->slowptr) @@ -208,22 +196,8 @@ static int snd_pcm_dsnoop_status(snd_pcm_t *pcm, snd_pcm_status_t * status) static snd_pcm_state_t snd_pcm_dsnoop_state(snd_pcm_t *pcm) { snd_pcm_direct_t *dsnoop = pcm->private_data; - int err; - snd_pcm_state_t state; - state = snd_pcm_state(dsnoop->spcm); - switch (state) { - case SND_PCM_STATE_DISCONNECTED: - dsnoop->state = state; - return state; - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) - return err; - break; - default: - break; - } - snd_pcm_direct_client_chk_xrun(dsnoop, pcm); + + snd_pcm_direct_check_xrun(dsnoop, pcm); return dsnoop->state; } @@ -422,16 +396,7 @@ static snd_pcm_sframes_t snd_pcm_dsnoop_mmap_commit(snd_pcm_t *pcm, snd_pcm_direct_t *dsnoop = pcm->private_data; int err; - switch (snd_pcm_state(dsnoop->spcm)) { - case SND_PCM_STATE_XRUN: - case SND_PCM_STATE_SUSPENDED: - if ((err = snd_pcm_direct_slave_recover(dsnoop)) < 0) - return err; - break; - default: - break; - } - err = snd_pcm_direct_client_chk_xrun(dsnoop, pcm); + err = snd_pcm_direct_check_xrun(dsnoop, pcm); if (err < 0) return err; if (dsnoop->state == SND_PCM_STATE_RUNNING) { From patchwork Thu Mar 10 09:14:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 550152 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 92DB9C433F5 for ; Thu, 10 Mar 2022 09:15:56 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 88E411813; Thu, 10 Mar 2022 10:15:04 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 88E411813 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646903754; bh=NDosigngH8CExy3wWkEoTd7t3GJDvvMjC+otns5QXgo=; h=From:To:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=S6Hsixo1AKa315J46B6c/NAQ742JvSs531+0oM5JBhSKLq/8cSWWTH7zjO3vWFTvu QdP+fKnTDcwnniyD1NiIEY85jGSQTRquHBemIv9MqKl7LQZ5+zjeIPH0y//Us50URp 6EJuBRR0+VWKGYdIeKMJWjMig1Q90By3TUWLFp1w= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 8113AF8016C; Thu, 10 Mar 2022 10:14:30 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id D0947F8013C; Thu, 10 Mar 2022 10:14:28 +0100 (CET) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id DF231F8012C for ; Thu, 10 Mar 2022 10:14:21 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz DF231F8012C Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="p8T+h0b2"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="7VaHxRlf" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 0DBB71F443 for ; Thu, 10 Mar 2022 09:14:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646903661; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8t0iQNIPy162m2SrP1PBxOSISuiTqFdtuLkgwscLzBU=; b=p8T+h0b2gKXseqgYEjc0yKyy2iNDsma698kG+brjh7weg9g7JQLwSM0jlowVnWE+ajgxzn ITVA6cQiQkUmiOqm6ssiycqw1z+xGihbwnmXR6hMzLmcJzrnZGZ49V+EPEFW96+u67+8vM Kee7VtydSb312qiGOzy+o3Lqtut0hnQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646903661; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8t0iQNIPy162m2SrP1PBxOSISuiTqFdtuLkgwscLzBU=; b=7VaHxRlfwaj6VkGDCRt8bCrPzvKyXXoybQkt6K/cGHqOCx/a0q5xSInV2SO7OSiO8cfgkU 3yqubxHrCP8NKtAQ== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id F20C6A3B81; Thu, 10 Mar 2022 09:14:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib 3/3] pcm: direct: Check xrun/suspend before the slave hwptr update Date: Thu, 10 Mar 2022 10:14:18 +0100 Message-Id: <20220310091418.1150-3-tiwai@suse.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20220310091418.1150-1-tiwai@suse.de> References: <20220310091418.1150-1-tiwai@suse.de> MIME-Version: 1.0 X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" The xrun/suspend may happen at any time and we should check it right before the slave hwptr update. Otherwise the hwptr value may be screwed and get unexpected large read/write. Reported-by: S.J. Wang Signed-off-by: Takashi Iwai --- src/pcm/pcm_dmix.c | 4 ++-- src/pcm/pcm_dshare.c | 4 ++-- src/pcm/pcm_dsnoop.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index d00d53bef604..111fea157228 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -426,11 +426,11 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dmix = pcm->private_data; int err; + if (dmix->slowptr) + snd_pcm_hwsync(dmix->spcm); err = snd_pcm_direct_check_xrun(dmix, pcm); if (err < 0) return err; - if (dmix->slowptr) - snd_pcm_hwsync(dmix->spcm); return snd_pcm_dmix_sync_ptr0(pcm, *dmix->spcm->hw.ptr); } diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index 0ff43a90d270..c46e8d6a53da 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -201,11 +201,11 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; int err; + if (dshare->slowptr) + snd_pcm_hwsync(dshare->spcm); err = snd_pcm_direct_check_xrun(dshare, pcm); if (err < 0) return err; - if (dshare->slowptr) - snd_pcm_hwsync(dshare->spcm); return snd_pcm_dshare_sync_ptr0(pcm, *dshare->spcm->hw.ptr); } diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 729ff447b41f..9abbbef2c1b6 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -134,14 +134,14 @@ static int snd_pcm_dsnoop_sync_ptr(snd_pcm_t *pcm) snd_pcm_sframes_t diff; int err; - err = snd_pcm_direct_check_xrun(dsnoop, pcm); - if (err < 0) - return err; if (dsnoop->slowptr) snd_pcm_hwsync(dsnoop->spcm); old_slave_hw_ptr = dsnoop->slave_hw_ptr; snoop_timestamp(pcm); slave_hw_ptr = dsnoop->slave_hw_ptr; + err = snd_pcm_direct_check_xrun(dsnoop, pcm); + if (err < 0) + return err; diff = pcm_frame_diff(slave_hw_ptr, old_slave_hw_ptr, dsnoop->slave_boundary); if (diff == 0) /* fast path */ return 0;