From nobody Tue Nov 26 18:32:14 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XyWQ82BPyz5dqCV; Tue, 26 Nov 2024 18:32:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XyWQ81ckHz578L; Tue, 26 Nov 2024 18:32:16 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732645936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=U3qKNBXHEf5wZihoYOGOIf2k81J7Ab1asF+/L6WrbLk=; b=HhMF5a38309MqeKiakfm0G3/daM8xkvFoURtEx6BXAwLXG9XKZNlUupo4zTzFcmFB+NCNF QsCLSsd8XO7gmItYa0kLM2nSzPjDgvSCKoKc9Gw3B7gy6Dsmyg6AaDdPsBh8SHhPWzHOTu sbWhGUZxEKUImOQVF9vma8RF39J02B34p07BIImnRw9FjXKOw8lV2CTNu8I76btDWeZOt6 J8hCYow4LRD6QtgB2neJzF1YZItqRksgIK9FpMsPU35FpZZyO55OkWEx7f19gJxDFIa4Om pTL/KIDwlGsNNCaWtIZt6WZdAJ4kbhHd3XXtJq6WOGrfL8bnV3RJ4Lxm6dFn/Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732645936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=U3qKNBXHEf5wZihoYOGOIf2k81J7Ab1asF+/L6WrbLk=; b=T0UmLi9KWfrVzPmvJTqMmfLPDhQOdxMZNvcskE69CEvbqUUJ4Iho6oBjqscryIxVoiN/sW oJbfecGsA53xP3D7H4aGb458E9GV4qWjXRLM570sysm2pTUpAE7HDiqFsF0d4oQDkk2hOn Ajh+p4eS767Izp36uhN10s+DO7jMzGlz6oLG64tb+C/69e9teox/qWEaP1nw16VkKKKB3M La9gAlGFg5MCtMUv3sRLDl2sov+pmjV/mlER3z3OOs9SSRA6bCUJdu1/LzO7Q588wwqQfm Zxe56ux16XN3rTKi2AC0sDilGMbMP2kYBPATAIYxMqEFkVyuZ07Ptw2wDpeBiA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1732645936; a=rsa-sha256; cv=none; b=osGI1/8G49nOmAnxt79mylf33T80gPv0FsI6kMlIa+o7IcY4drK2nSgDGU4mvqAGSGfYFx tcNC9PQ5io7dDwyu+4017yQyDAnw8ArTbjwWmuPieGxJGW90pKvJbTNGGE+offPePTXA8m MHfU7Twkusc29StKYqx091j95NdgSprwCUpDGPYhkbIwibARkM3FGufpsiMqb1E9GaoI4e T0R/GAe0KZ4n06wl2jsOkCCYYDKaillQ5o0vyV9N+xlUTEt1S2BLQf/NVtE42vBgTWIxVf /qWrijQaj3ftr5th2ud6/i2yHQi1vIXK+3kpeQMGz7g2e2BbXURDYZ5nzhKU3A== Received: from [IPV6:2601:5c0:4200:b830:ec27:f42f:d16d:a9fa] (unknown [IPv6:2601:5c0:4200:b830:ec27:f42f:d16d:a9fa]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4XyWQ772Tqz12hL; Tue, 26 Nov 2024 18:32:15 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <77c4113c-c023-4db3-826a-83be4e9baa42@FreeBSD.org> Date: Tue, 26 Nov 2024 13:32:14 -0500 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: 5317480967bf - main - sound: Remove CHN_F_SLEEPING Content-Language: en-US To: Christos Margiolis , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202411261448.4AQEmw7Y084208@gitrepo.freebsd.org> From: John Baldwin In-Reply-To: <202411261448.4AQEmw7Y084208@gitrepo.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/26/24 06:48, Christos Margiolis wrote: > The branch main has been updated by christos: > > URL: https://cgit.FreeBSD.org/src/commit/?id=5317480967bfc8bf678e4da3fce81bcb3f5b7836 > > commit 5317480967bfc8bf678e4da3fce81bcb3f5b7836 > Author: Christos Margiolis > AuthorDate: 2024-11-26 14:48:36 +0000 > Commit: Christos Margiolis > CommitDate: 2024-11-26 14:48:36 +0000 > > sound: Remove CHN_F_SLEEPING > > The KASSERT in chn_sleep() can be triggered if more than one thread > wants to sleep on a given channel at the same time. While this is not > really a common scenario, tools such as stress2, which use fork() and > the child process(es) inherit the parent's FDs as a result, we can end > up triggering such scenarios. > > Fix this by removing CHN_F_SLEEPING altogether, which is not very useful > in the first place: > - CHN_BROADCAST() checks cv_waiters already, so there is no need to > check CHN_F_SLEEPING as well. > - We can check whether cv_waiters is 0 in pcm_killchans(), instead of > whether CHN_F_SLEEPING is not set. > > Reported by: dougm, pho (stress2) > Sponsored by: The FreeBSD Foundation > MFC after: 2 days > Reviewed by: dev_submerge.ch, markj > Differential Revision: https://reviews.freebsd.org/D47559 > --- > sys/dev/sound/pcm/channel.c | 13 +------------ > sys/dev/sound/pcm/channel.h | 4 ++-- > sys/dev/sound/pcm/sound.c | 4 ++-- > 3 files changed, 5 insertions(+), 16 deletions(-) > > diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h > index 79a8d35b22f7..d226adfba06b 100644 > --- a/sys/dev/sound/pcm/channel.h > +++ b/sys/dev/sound/pcm/channel.h > @@ -354,7 +354,7 @@ enum { > #define CHN_F_RUNNING 0x00000004 /* dma is running */ > #define CHN_F_TRIGGERED 0x00000008 > #define CHN_F_NOTRIGGER 0x00000010 > -#define CHN_F_SLEEPING 0x00000020 > +/* unused 0x00000020 */ > > #define CHN_F_NBIO 0x00000040 /* do non-blocking i/o */ > #define CHN_F_MMAP 0x00000080 /* has been mmap()ed */ > @@ -381,8 +381,8 @@ enum { > "\002ABORTING" \ > "\003RUNNING" \ > "\004TRIGGERED" \ > + /* \006 */ \ > "\005NOTRIGGER" \ > - "\006SLEEPING" \ > "\007NBIO" \ Hmm, new comment is mis-sorted? > "\010MMAP" \ > "\011BUSY" \ > diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c > index 4b7bcc64397f..218147b73db0 100644 > --- a/sys/dev/sound/pcm/sound.c > +++ b/sys/dev/sound/pcm/sound.c > @@ -221,8 +221,8 @@ pcm_killchans(struct snddev_info *d) > /* Make sure all channels are stopped. */ > CHN_FOREACH(ch, d, channels.pcm) { > CHN_LOCK(ch); > - if ((ch->flags & CHN_F_SLEEPING) == 0 && > - CHN_STOPPED(ch) && ch->inprog == 0) { > + if (ch->intr_cv.cv_waiters == 0 && CHN_STOPPED(ch) && > + ch->inprog == 0) { I'm not super excited about reading cv_waiters directly. Generally speaking 'struct cv' is opaque to the rest of the kernel. Maybe add a little inline routine or macro cv_waiters() that returns this value instead? Then it can be documented in condvar.9 along with the caveats about when it is safe to use. -- John Baldwin