From mboxrd@z Thu Jan 1 00:00:00 1970 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 E191CC6FD1C for ; Thu, 23 Mar 2023 13:38:34 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 4274BEC3; Thu, 23 Mar 2023 14:37:42 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 4274BEC3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1679578712; bh=UN/xrfJzI51X/L0fUOVzVqIYSqjOuGp6ROg5ubCqogI=; h=Date:From:To:Subject:In-Reply-To:References:CC:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=vEPFukMSTGZWq8vLvD6/KyxPTl9xskMX22fP282Bllu+KdyAVn94tWmfxEfcXY4qo TO6a5GOGI2Cgz62xVXrXeUB9xU1U5++oH0orARbBnm7atRbPVDYGKQDlFzt6A48W4/ COnhgf1mXeAbk0SYga/TQA9zkdo+b5rueJIvYTmI= Received: from mailman-core.alsa-project.org (mailman-core.alsa-project.org [10.254.200.10]) by alsa1.perex.cz (Postfix) with ESMTP id F1965F8027B; Thu, 23 Mar 2023 14:37:19 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 68930F8027B; Thu, 23 Mar 2023 14:37:16 +0100 (CET) Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D04C3F800C9 for ; Thu, 23 Mar 2023 14:37:13 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D04C3F800C9 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key, unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=X4wtv2aO; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=m788v6nr Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 3993522998; Thu, 23 Mar 2023 13:37:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1679578633; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8TDa2feyoE1uYNfpnSNfdF/V7hELQLoI+CoP9Sfggzk=; b=X4wtv2aOI+hyianrnuYfE/CKrDSKuMiEQM8+TdCYA4AS1w1NTChPxy3rfFt7WNCdPsm+YQ 8qcw6S87amFmFTV1/pDQ6KeLCbjTEE/Zi8NxGkw0hNf3eZhBRuDMpj+SsTbSg9cawNBMsz xfNpxxNgct1PInXqGm7Is9Mxs1vDVgM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1679578633; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8TDa2feyoE1uYNfpnSNfdF/V7hELQLoI+CoP9Sfggzk=; b=m788v6nrAA7uXvh/Hdb8Meb9t/41WPf47FAZ7Bm0dByD3oFDpTh0iv6obxmgOPAexhFthl ZzJmqqc0cmDtMGDA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1EFD213596; Thu, 23 Mar 2023 13:37:13 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 4tEUBwlWHGTzXgAAMHmgww (envelope-from ); Thu, 23 Mar 2023 13:37:13 +0000 Date: Thu, 23 Mar 2023 14:37:12 +0100 Message-ID: <87pm8z612v.wl-tiwai@suse.de> From: Takashi Iwai To: Takashi Sakamoto Subject: Re: [PATCH] ALSA: usb-audio: Fix recursive locking at XRUN during syncing In-Reply-To: <20230323132916.GA235532@workstation> References: <20230320142838.494-1-tiwai@suse.de> <20230321040258.GA164337@workstation> <87cz50htvi.wl-tiwai@suse.de> <20230323132916.GA235532@workstation> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Message-ID-Hash: QPBB5CNVOHSLNU5QZVQ6TPII5TY2HUIU X-Message-ID-Hash: QPBB5CNVOHSLNU5QZVQ6TPII5TY2HUIU X-MailFrom: tiwai@suse.de X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-alsa-devel.alsa-project.org-0; header-match-alsa-devel.alsa-project.org-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: alsa-devel@alsa-project.org, John Keeping X-Mailman-Version: 3.3.8 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 23 Mar 2023 14:29:16 +0100, Takashi Sakamoto wrote: > > On Thu, Mar 23, 2023 at 07:19:45AM +0100, Takashi Iwai wrote: > > On Tue, 21 Mar 2023 05:02:58 +0100, > > Takashi Sakamoto wrote: > > > > > > Hi, > > > > > > On Mon, Mar 20, 2023 at 03:28:38PM +0100, Takashi Iwai wrote: > > > > The recent support of low latency playback in USB-audio driver made > > > > the snd_usb_queue_pending_output_urbs() function to be called via PCM > > > > ack ops. In the new code path, the function is performed alread in > > > > > > 's/alread/already/' or slang. > > > > > > > the PCM stream lock. The problem is that, when an XRUN is detected, > > > > the function calls snd_pcm_xrun() to notify, but snd_pcm_xrun() is > > > > > > 's/the function/the function/' > > > > Corrected at applying. > > > > > > supposed to be called only outside the stream lock. As a result, it > > > > leads to a deadlock of PCM stream locking. > > > > > > > > For avoiding such a recursive locking, this patch adds an additional > > > > check to the code paths in PCM core that call the ack callback; now it > > > > checks the error code from the callback, and if it's -EPIPE, the XRUN > > > > is handled in the PCM core side gracefully. Along with it, the > > > > USB-audio driver code is changed to follow that, i.e. -EPIPE is > > > > returned instead of the explicit snd_pcm_xrun() call when the function > > > > is performed already in the stream lock. > > > > > > Practically, the implementation of 'pcm_hw' in alsa-lib never evaluates > > > the return value (see 'snd_pcm_hw_mmap_commit()' and the others). I guess > > > that it is inconvenient for the low-latency mode of USB Audio device class > > > driver for the case of failure. > > > > > > My additional concern is PCM indirect layer, since typically the layer > > > is used by drivers with SNDRV_PCM_INFO_SYNC_APPLPTR. But as long as I > > > read, the change does not matter to them. > > > > I find rather that's an extra bonus :) It allows the existing code to > > give a more proper error handling. For example, the indirect PCM > > helper returned -EINVAL, but this can be switched to -EPIPE for > > stopping the stream. > > > > I'm going to submit the patch together with the documentation > > updates. > > In my opinion, extra care is required for the simple replacement of > -EINVAL with -EPIPE, since it may not come from any operation relevant > to hardware. In my understanding, it comes from just careless > implementation of user space PCM application, thus it is still reasonable > to return -EINVAL. Yeah, we can't always replace such condition blindly. But in the case of indirect PCM helpers, it's the place where the bogus values have been already set and processed, so we can't return -EINVAL, hence an XRUN notification would be appreciate. Takashi