From patchwork Thu Mar 10 10:00: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: 12776116 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 04FC9C433EF for ; Thu, 10 Mar 2022 10:02:59 +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 4AA73185F; Thu, 10 Mar 2022 11:02:07 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 4AA73185F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646906577; bh=kCzNPSwBFUuFadK9QRN3ClPIwgropEinjgLt3i9UEWI=; h=From:To:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From; b=ioJ7y8QfxnuBd/bbF9mbzgSEU6psJ2bACZNPWieMlIn7Eg39997w8uzfDi5704CWK 5WuhDkAaQaa8mledmTekFpI9KeHX907Bu2iBXjpH63suZyK2sRI7SxBfthaR5PiUdr J4cnwhqCnpAA9081bw88UX55qORttG5bIz34yMeM= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id DCE66F8012F; Thu, 10 Mar 2022 11:02:06 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id D39DEF80137; Thu, 10 Mar 2022 11:02:04 +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 CE94FF800D2 for ; Thu, 10 Mar 2022 11:00:20 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz CE94FF800D2 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="vPIQiSAG"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="Uf6rwFlx" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 4867F1F442 for ; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646906420; 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=vPIQiSAGVQzLh3XMQFAtZzbCXSjJSKS4N2jzAxCU4OPxBpwNju4nfIDyqixVPC6/r+Grhj EgY8NapmONQG7oiWhW8K8MPIugIze9kQIQjhX4fpqSu+FIF03Ij1x75U9Ys5wx5x4Jh8Zq RYV6idwIMps4CUsM6PkZOlch/yjIQt4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646906420; 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=Uf6rwFlxVfEZqyauexBBhLxQHyWP+GuPRF+Y52WSSgMQvZb13uX8dfjcXaApCNyhwN9FF4 +xcag8S8834S8kDA== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 37A2AA3B96; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib v2 1/3] pcm: direct: Improved suspend/resume support Date: Thu, 10 Mar 2022 11:00:16 +0100 Message-Id: <20220310100018.10038-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 10:00: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: 12776115 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 60F30C433F5 for ; Thu, 10 Mar 2022 10:01:21 +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 9D3CB1845; Thu, 10 Mar 2022 11:00:29 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 9D3CB1845 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646906479; 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=WQUKSkZvcolQq1394G8ZqTmVjnC5XKvZskUObAXtQ2phgtmCNrdSlW9eAscGXrZ1b NXCKen0Lh/zJhecJPH/Y3FhVm7fPX4Jl8EhlVI/bVcws4uYvQlhFOvsd0EPxih3h95 ijEozduKhjYx0V1dONRNFmhUrCXmP7hZRbuyC0R8= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 3135FF80137; Thu, 10 Mar 2022 11:00:29 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 95D22F8012F; Thu, 10 Mar 2022 11:00:28 +0100 (CET) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (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 AC988F8012F for ; Thu, 10 Mar 2022 11:00:20 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz AC988F8012F Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="pbsPiiDh"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="4MvQRXnj" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 4D713210ED for ; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646906420; 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=pbsPiiDhpAK8bOxo90V37rPVHbVn3ctPzyapLAng5/vxcAzG5S5eXoPIB54fja4WE2M9+U j2GRDajVM7ooh+tNfu91+3bPWZWkS0kR28vYu0snsJ50IOdOjWqmnE2U87+lE0f3OQG4pl iRSEUrhBdYyVvo1zd6taqHcNZeWp/u4= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646906420; 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=4MvQRXnjPmayMdBgyiWD8qGF9O7kwFrJmTg0VDE96HqVPc3wb4QmVVPq96znnRYqxeIcLM jChuemCDxlbQcAAw== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 46CD0A3B89; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib v2 2/3] pcm: direct: Move slave PCM state checks into XRUN check helper Date: Thu, 10 Mar 2022 11:00:17 +0100 Message-Id: <20220310100018.10038-2-tiwai@suse.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20220310100018.10038-1-tiwai@suse.de> References: <20220310100018.10038-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 10:00: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: 12776114 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 C7507C433EF for ; Thu, 10 Mar 2022 10:01:16 +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 E914917EB; Thu, 10 Mar 2022 11:00:24 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz E914917EB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646906475; bh=TvwRXoakGQQI3EHYMoW0++Fnjj6tsuzgkYwE7cVUDks=; h=From:To:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tbvIxwqSYQM4walULvrrBXkqlnua7l3hbKLC0G+Nis9PgnjfHQTyf+Eyw5VMrT7aN s3SXExQmcn9OSdhSsfuVTtB14OlIRCFhd8LKXP/UVtd7rNntYLroXoJ0qOmYfEbYyN +jGzSnuw6cPFwqCZTbXaqEV+UYKxPqnSOtc7kr+g= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 7FA7CF8016C; Thu, 10 Mar 2022 11:00:24 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 83753F800D2; Thu, 10 Mar 2022 11:00:23 +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 1169BF800D2 for ; Thu, 10 Mar 2022 11:00:20 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 1169BF800D2 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="ciZbvCPh"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="K6BZE/K8" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 5A98F1F443 for ; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1646906420; 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=P5N8Aa73JtYypFbUeV12FHusZ6v7tU6PoPPalTT+fyg=; b=ciZbvCPhPThrF/jpgSVSRqUaFGaJfzy5wVxp3U4NRv9MXjgYvM7HrSAUjrAaUaHQndgihj pirMc1ekkdHymFcuNBOj/u08YrkXRbQ6QxIO3yN3l3M9I9CwFRrxJ7JZn+8U/eoQakNPuh Btf8E/Lj1kG5fZeGk6+HC/LRfeK87uE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1646906420; 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=P5N8Aa73JtYypFbUeV12FHusZ6v7tU6PoPPalTT+fyg=; b=K6BZE/K8cIojaUKzAanjhcx2TKDS4C+w7rawq349n05pedC+5IJS8f1vGq4oCJcByc/A1O M/jHXsA8JicO1GBg== Received: from alsa1.nue.suse.com (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 4BBA3A3B9B; Thu, 10 Mar 2022 10:00:20 +0000 (UTC) From: Takashi Iwai To: alsa-devel@alsa-project.org Subject: [PATCH alsa-lib v2 3/3] pcm: direct: Check xrun/suspend before the slave hwptr update Date: Thu, 10 Mar 2022 11:00:18 +0100 Message-Id: <20220310100018.10038-3-tiwai@suse.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20220310100018.10038-1-tiwai@suse.de> References: <20220310100018.10038-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 after the slave hwptr update (but before the actual sync_ptr update in direct pcm side). 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 | 8 +++++--- src/pcm/pcm_dshare.c | 8 +++++--- src/pcm/pcm_dsnoop.c | 6 +++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index d00d53bef604..c6cb47f0f840 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -424,15 +424,17 @@ static int snd_pcm_dmix_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data; + snd_pcm_uframes_t slave_hw_ptr; int err; + if (dmix->slowptr) + snd_pcm_hwsync(dmix->spcm); + slave_hw_ptr = *dmix->spcm->hw.ptr; 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); + return snd_pcm_dmix_sync_ptr0(pcm, slave_hw_ptr); } /* diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index 0ff43a90d270..461adafc77f8 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -199,15 +199,17 @@ static int snd_pcm_dshare_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_p static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) { snd_pcm_direct_t *dshare = pcm->private_data; + snd_pcm_uframes_t slave_hw_ptr; int err; + if (dshare->slowptr) + snd_pcm_hwsync(dshare->spcm); + slave_hw_ptr = *dshare->spcm->hw.ptr; 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); + return snd_pcm_dshare_sync_ptr0(pcm, slave_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;