From nobody Tue Nov 12 15:23:57 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 4XnqvM04pFz5cgj6; Tue, 12 Nov 2024 15:23:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 4XnqvL6CKVz4Shx; Tue, 12 Nov 2024 15:23:58 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1731425038; 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=johdrE8hAqkY7wdFK8ywmxAvcYeFpV+QlsoydzpJDl4=; b=LSjQ/FOVVY7QNQny193dx1d5ZxALJyf9I38vOV7AYHkLkFKMxUEsxut8VV3AmAJt9x+XFY zvUfzmknNqMgzaKvVh0lr4HzUVvBfjvV7mF8axBuHLt+oGKqgUGJ58Wk0OqhwvZycYQ122 ZyCdOpdzz78g24Fq16merzgdNWlLVNQ577Sz5TNTtTdYeLTA4R2yEEUk7S7H3AnGxcQ/wZ EpkjbTaq4de8yp3dtG35niP1ydto5UlBuWRAA/n63vi39ce3rGflq32M3QP+xKqHd0WWeb zihzR/VTiP96U890IumNN6GxIDo9gAXpjEYlwS3ccNOBZzRD/hZ/ihIuVK9ijw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1731425038; 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=johdrE8hAqkY7wdFK8ywmxAvcYeFpV+QlsoydzpJDl4=; b=e2MmLezaTHH3Kvg9QEAzZaDqJLgS2dCcRCk/93mf0nRJogrdXShJtdY5DR0PgXRXD87TOO KB9xrDP6PwlyJ58c6hQyk3u9Erk9qyZ6qppu4ejDyujROpiSkyyhOv2xus6WthZS8xNG/8 jnS/9bBUwWoWoEv1yQxL6WjfkL/++RadhekSgg4a4xv52cSxggl7jOanD8HvCsrnG61IYS 0KdmoDTsbMUY34UEiaq/U8eNT7Lio7PVOAo2ip1YNoheSGkZgmoV9OOar1IMy6pkpmuZ8E 0hSCgN8jkUif1dpzBaoYJ/BdLfeJ2MSGUkcxz2N6Ae9mQvC6XTtXd/21i3nakw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1731425038; a=rsa-sha256; cv=none; b=bmaY08JilrIlyx5Pf/iIcyJ+TtjR+iC+4CO0PaHtrt0uc4DvwoVvF1cKDhAz4hmfW9Raaj Dz79Pt5wUzRhDFq9qTr1xxi99dTrEqfZ28tcXE3s6s7cYgPKIquZed8tNa0ieZNC38MhgX YDz2afmBV2AazxVnotb00T0mn92CDnHyby8+3YxVXy1do7FZWVnfD4dIH8Yu0S93Q4keKn GHgOXqYhJKn8q8jIMyGlO7mZROHBzHxu0nz0kGW1HvbEwCyPts3McT3U7Bxc3tzQCIh638 2bEoWApNZQsd5ayepI51Y4sotXI/VlJEq/eI+jNkfY0NEjDDI/9VP1sMRE4Hjw== Received: from [IPV6:2601:5c0:4200:b830:d04f:46f9:caeb:6e94] (unknown [IPv6:2601:5c0:4200:b830:d04f:46f9:caeb:6e94]) (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 4XnqvL4GYBzjbx; Tue, 12 Nov 2024 15:23:58 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <09923ad3-4065-4a31-b35f-74f84e09cff1@FreeBSD.org> Date: Tue, 12 Nov 2024 10:23:57 -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: 1fa6daaafd74 - main - ipfilter: Avoid stopping with a lock held Content-Language: en-US To: Jose Luis Duran , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202411120347.4AC3lPOA015167@gitrepo.freebsd.org> From: John Baldwin In-Reply-To: <202411120347.4AC3lPOA015167@gitrepo.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/11/24 19:47, Jose Luis Duran wrote: > The branch main has been updated by jlduran: > > URL: https://cgit.FreeBSD.org/src/commit/?id=1fa6daaafd74c1a457dcfe26e0a5943b5441dc9d > > commit 1fa6daaafd74c1a457dcfe26e0a5943b5441dc9d > Author: Jose Luis Duran > AuthorDate: 2024-11-02 17:58:59 +0000 > Commit: Jose Luis Duran > CommitDate: 2024-11-12 03:46:15 +0000 > > ipfilter: Avoid stopping with a lock held > > Avoid calling _callout_stop_safe with a non-sleepable lock held when > detaching by initializing callout_init_rw() with CALLOUT_SHAREDLOCK. > > It avoids the following WITNESS warning when stopping the service: > > # service ipfilter stop > calling _callout_stop_safe with the following non-sleepable locks held: > shared rw ipf filter load/unload mutex (ipf filter load/unload mutex) r = 0 (0xffff0000417c7530) locked @ /usr/src/sys/netpfil/ipfilter/netinet/fil.c:7926 > stack backtrace: > #0 0xffff00000052d394 at witness_debugger+0x60 > #1 0xffff00000052e620 at witness_warn+0x404 > #2 0xffff0000004d4ffc at _callout_stop_safe+0x8c > #3 0xffff0000f7236674 at ipfdetach+0x3c > #4 0xffff0000f723fa4c at ipf_ipf_ioctl+0x788 > #5 0xffff0000f72367e0 at ipfioctl+0x144 > #6 0xffff00000034abd8 at devfs_ioctl+0x100 > #7 0xffff0000005c66a0 at vn_ioctl+0xbc > #8 0xffff00000034b2cc at devfs_ioctl_f+0x24 > #9 0xffff0000005331ec at kern_ioctl+0x2e0 > #10 0xffff000000532eb4 at sys_ioctl+0x140 > #11 0xffff000000880480 at do_el0_sync+0x604 > #12 0xffff0000008579ac at handle_el0_sync+0x4c > > PR: 282478 > Suggested by: markj > Reviewed by: cy > Approved by: emaste (mentor) > MFC after: 1 week > --- > sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c > index bcde0d2c7323..b3dea40c3d8c 100644 > --- a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c > +++ b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c > @@ -181,7 +181,7 @@ ipf_timer_func(void *arg) > #if 0 > softc->ipf_slow_ch = timeout(ipf_timer_func, softc, hz/2); > #endif > - callout_init(&softc->ipf_slow_ch, 1); > + callout_init_rw(&softc->ipf_slow_ch, &softc->ipf_global.ipf_lk, CALLOUT_SHAREDLOCK); > callout_reset(&softc->ipf_slow_ch, > (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT, > ipf_timer_func, softc); > @@ -221,7 +221,7 @@ ipfattach(ipf_main_softc_t *softc) > softc->ipf_slow_ch = timeout(ipf_timer_func, softc, > (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT); > #endif > - callout_init(&softc->ipf_slow_ch, 1); > + callout_init_rw(&softc->ipf_slow_ch, &softc->ipf_global.ipf_lk, CALLOUT_SHAREDLOCK); > callout_reset(&softc->ipf_slow_ch, (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT, > ipf_timer_func, softc); > return (0); But this means the callout is now called with the lock held, and I don't see any changes to ipf_timer_func. Hmm, so you now recurse on the lock? You need to take out the locking in ipf_timer_func I think as it is now spurious. You can also axe the #if 0'd code around timeout() while you are at it. -- John Baldwin