From nobody Thu Mar 06 13:33:12 2025 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 4Z7r2y3g6bz5pc1t; Thu, 06 Mar 2025 13:33:14 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Z7r2y2nMpz3mJ1; Thu, 06 Mar 2025 13:33:14 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1741267994; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=naIaQvdX10XeUYdVGzrsF1xvgpeQ3zT3uVQFW6KOHzU=; b=VJokD7ctOqlz4z1l35U8p7sro27DfEz8jXTFeNTiCXVb76+kbdnFynpnG/wGA3HJYo7ePN JgQJ07xS219eZaRfI4RyxrrUcRn3JxQWupaBeYcAmHalA0lY/Q7IptlfGIfBcz/f6OJkZH aR2O58LmbGZ2/PjcH1cNFX9juOVIiNA+WPuN3nUj1n0TZYReOMlBbIg9nU6drpThgMYOF+ KO7aj5BhBTj9SDfPqSL9eJEnFdKReWvFuv51Yb2+KHqN+JRxvZDgGmh8z34RuEnPakMiuR ZEpUXGo23jwbe6LzwCVIseIi/jZEvQlefx6N8Fx8RVJuW4pcEzO776xZtHXGcw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1741267994; a=rsa-sha256; cv=none; b=Zpk9I1ggzXWpQib+irJr159WxUDHMnCfcFPPOJpBe1AZIHP4zs/wHm/qvONCGlrWp9NmUy 0m0ycQgtNU4S6G9IGSzkFhNbeUaAKknzYaXAObmOvJDXLkxmBhdIIGxMbERs5Eb5y8fdsW +ZYdZUtoiM4wrnOjtjyxCh+8i5xrxTRpWEsxQEgn1l0XWywiU0YVNJo4IMahuBo0xEIBOG veXKf4Dfvtz6C8GfcQyB52TmZNDW1K+KqlXdvmlWKJDXrO/Q48Ve/HBi94wXL810Jrdq/g 1rpCTjHtqMNfmr0byzKog528Xjbs7A8E4bvWf1grH4d6+w/iYQNLwVtcxz2emg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1741267994; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=naIaQvdX10XeUYdVGzrsF1xvgpeQ3zT3uVQFW6KOHzU=; b=erX9NekCr8jKro1GdNEX656Dxb8Cc+KJlarCxqpnppWUr42RuUrpVE4LSP7nqwDhkeOf3/ z79RvMdta7Efie6msvTtoOvgUmLGYdslnDL7HTD1lHsJDpwjN2tDFGvWGfoUaDae98YJKI 3jIFhFj70AkPbdMU/YOZEDXHuLZwNDnVSba4yus6Q2Io/2qstejwIBJLhVNlb2ri6c0dGq NMBVhH3J1ZVXzg+kuP7LZHsQloVEDTmNt+RAiIYomY/tcn3wCXL1BiI9W6jsteKcu0rxpE wmDVFET5KuBqwH0KRfSr8z+V8cP/nvDa53a1V3f5HepfjSzH/JLbUOQRD6V9QA== Received: from [IPV6:2601:5c0:4200:b830:ec45:8834:27fa:4893] (unknown [IPv6:2601:5c0:4200:b830:ec45:8834:27fa:4893]) (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 4Z7r2y0N3NzwmW; Thu, 06 Mar 2025 13:33:14 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Thu, 6 Mar 2025 08:33:12 -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: 234683726708 - main - devclass: make devclass_alloc_unit use M_NOWAIT Content-Language: en-US To: Mateusz Guzik , Zhenlei Huang Cc: Mateusz Guzik , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , Warner Losh , "dev-commits-src-main@freebsd.org" References: <202503061103.526B32Id022652@gitrepo.freebsd.org> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 3/6/25 06:35, Mateusz Guzik wrote: > On Thu, Mar 6, 2025 at 12:32 PM Zhenlei Huang wrote: >> >> >> >> On Mar 6, 2025, at 7:03 PM, Mateusz Guzik wrote: >> >> The branch main has been updated by mjg: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859 >> >> commit 234683726708cf5212d672d676d30056d4133859 >> Author: Mateusz Guzik >> AuthorDate: 2025-03-06 11:01:49 +0000 >> Commit: Mateusz Guzik >> CommitDate: 2025-03-06 11:01:49 +0000 >> >> devclass: make devclass_alloc_unit use M_NOWAIT >> >> The only caller already does this. >> >> The routine can be called with a mutex held making M_WAITOK illegal. >> >> Sponsored by: Rubicon Communications, LLC ("Netgate") >> --- >> sys/kern/subr_bus.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c >> index 9506e471705c..0422352bba51 100644 >> --- a/sys/kern/subr_bus.c >> +++ b/sys/kern/subr_bus.c >> @@ -1208,6 +1208,7 @@ devclass_get_sysctl_tree(devclass_t dc) >> static int >> devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp) >> { >> + device_t *devices; >> const char *s; >> int unit = *unitp; >> >> @@ -1264,8 +1265,11 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp) >> int newsize; >> >> newsize = unit + 1; >> - dc->devices = reallocf(dc->devices, >> - newsize * sizeof(*dc->devices), M_BUS, M_WAITOK); >> + devices = reallocf(dc->devices, >> + newsize * sizeof(*dc->devices), M_BUS, M_NOWAIT); >> >> >> I'd recommend against this. From the commit message of f3d3c63442ff, Warner said, >>> In addition, transition to M_WAITOK since this is a sleepable context >> So, the M_WAITOK is intentional. >> >> Rather than reverting this, the caller devclass_add_device() should use M_WAITOK. >> > > Per my commit message this is callable from a *NOT* sleepable context. > > Here is a splat we got at Netgate: > > uma_zalloc_debug: zone "malloc-16" with the following non-sleepable locks held: > exclusive sleep mutex SD slot mtx (sdhci) r = 0 (0xd8dec028) locked @ > /var/jenkins/workspace/pfSense-Plus-snapshots-25_03-main/sources/FreeBSD-src-plus-RELENG_25_03/sys/dev/sdhci/sdhci.c:688 > stack backtrace: > #0 0xc0330ebc at witness_debugger+0x78 > #1 0xc033217c at witness_warn+0x428 > #2 0xc05b0a58 at uma_zalloc_debug+0x34 > #3 0xc05b067c at uma_zalloc_arg+0x30 > #4 0xc0291760 at malloc+0x8c > #5 0xc02920ec at reallocf+0x14 > #6 0xc02f8894 at devclass_add_device+0x1e8 > #7 0xc02f6c78 at make_device+0xe0 > #8 0xc02f6abc at device_add_child_ordered+0x30 > #9 0xc0156e0c at sdhci_card_task+0x238 > #10 0xc0324090 at taskqueue_run_locked+0x1b4 > #11 0xc0323ea0 at taskqueue_run+0x50 > #12 0xc0275f88 at ithread_loop+0x264 Just use a regular taskqueue like taskqueue_thread instead of taskqueue_swi? PCI hotplug defines its own thread taskqueue for adding and removing devices. The bug is here, IMO. Eventually new-bus will need some sort of topology lock and that will have to be an sx lock, so this code needs to change anyway. The sound code that tries to frob devices with a regular mutex also needs to change. In terms of taskqueue_swi, it's probably something that needs to go away. Generally speaking, code uses tasks for functions that need to sleep, and taskqueue_swi breaks that. I will fix sdhci to use a proper taskqueue and then revert this commit. -- John Baldwin