From nobody Thu Mar 06 19:32:27 2025 X-Original-To: dev-commits-src-all@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 4Z801S68tlz5q4JV; Thu, 06 Mar 2025 19:32:28 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Z801S1gQDz3YNt; Thu, 06 Mar 2025 19:32:28 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1741289548; 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=0vw1L4Dlaxj0SIFeeuXTSEwgnvI/jSnbfTYbD7aGF9s=; b=rlqe3/BobvV1g4bEnC+7MA1wCNF/PFq0ZRP3wQD0mcTqeIbJiJY+Shfsg8abEVEVbSOvWK eapffdBCdOzOWbdXqJ/IxfNktuojt1zlwDWXgbxWnPaNIfY0LnZoGTxEEPRopX/XTQWpbx g/QMvSg3zHVkK2uoGDx0gprqM6P/nY1DeBQis/3dbAI9eNyU3imtsya+hPwbOvQu1/eLdE HnoaT0RJ+UX40cPv5ijXZlfS1bbWRUwN43BP5Che45ekRNHD318gRPG81hLsdY1z46gLzG WIwz4Qaq+hcJq7eL6FGMx2vNdc5jABfTmrcHsHgv/XKUQNy+Bil10Jlc/TSrZQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1741289548; a=rsa-sha256; cv=none; b=VDCaJ4Lf9ck7r5e9rVzP9sPVCIYfu9SuyrLGUnBGZobiA5BFOW6WC6lVUiRmgV7fmbIvRc DKokN5Iv3L1P9P7LcrYwnbPpGgs0WA4bkgdvy43CndMr64pTTrM7G8WcCr4m7v6gpdonfy /6ab3+JLqbUefzwtbhlm2rA+zLIgmdqhM7JQE23YtwUYEyRhDmZOCGWJ0nKd1iFtP1YMpk ubxLpJSl6AG1QeZvDQxKPdLgA/HgDJhxkBS6g6kEB4UDjbiMcPGvo7sWr93Ms4ZwcV5BEt IgGPwpurWYechCClCIJl238rePMc6RVhjAYjm4eO3sNj5UG4Glc1brqVEafePQ== 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=1741289548; 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=0vw1L4Dlaxj0SIFeeuXTSEwgnvI/jSnbfTYbD7aGF9s=; b=qivLoFDM6mnIh/S+c8NvjKwrWxyXehvy5dPEVZD0d37opaMt5nKTmgydH1nUhsObh82HJr I2X5U1yAb1kUQ7rg/VJxvmiKi5WRhsXYTpo9O+97OUJQezKwlutsuJ4tZ3/sejk3or7OxB XB530qPO7Ud66XbMmyGIAw6OfMIMowYWTa3kCaxbEXjE5QbR5XzFCoF4CkBK8a2AOv095N Vi8tnjXAB+OuAt1oCeaIqBHsQhCkUss0yomPoZh4VNd0PNZTNRn2f95FwJpdTtbi4uNTao ZX/pD5pgfPa9fW/AJ7dXZ6rlYuLaE9IcnEBHmw+LabmEGfMqL8NrwlfzhfYshw== 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 4Z801R63dzz13pl; Thu, 06 Mar 2025 19:32:27 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <137702dd-6190-452f-be0d-9d04b779ce0a@FreeBSD.org> Date: Thu, 6 Mar 2025 14:32:27 -0500 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@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: Warner Losh Cc: Mateusz Guzik , Zhenlei Huang , Mateusz Guzik , src-committers , 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 09:56, Warner Losh wrote: > On Thu, Mar 6, 2025, 5:33 AM John Baldwin wrote: > >> 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. >> > > I should dust off the branch that i have this one. There's about a dozen > places I had to change at the time... > > 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. >> > > Its a holdover from spl days for sure. > > I will fix sdhci to use a proper taskqueue and then revert this commit. >> > > Thanks. You can add me to the review It turned into a series. I'll post for review in a bit. -- John Baldwin