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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 52C33103E2FC for ; Thu, 12 Mar 2026 01:33:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:To: Subject:Cc:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0ZuXcANpbO5f9lJpjp5DojLJZeC/g1ENjyYfealo8kY=; b=Ze+cX/xvlWWaBdAg/VXVJ0lCY3 1mey01yDYVQVoYT6KxW72KPZY89x11A+seEvTsC7MBPblY59EUjbJEXQvcjxmJzrG+YcmW5nvr+BY MPU8UB4KnX5CvkNu5ykz6OcRee3tvsbxRj6YOVRciQwgBXm0M9lubwKTixV04Bs1VLBnuxlTIwxGY sMytve05fhvp2csNNy58/OaZUcBiNXIuHUSUNPmCG6MhE9c1OfsF3CSy7TVderEh7TKExqGMrehYk htzwmu3qNZnrk8PyFbL++gvwlYe/MbK22dmAGCRUK3c1pJ/9Cg8szyDQ7Ggf34an85PBu0cl/0ol4 VyJN5/KA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w0UvS-0000000D0s3-0Wlx; Thu, 12 Mar 2026 01:33:22 +0000 Received: from mail-m3291.qiye.163.com ([220.197.32.91]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w0UvP-0000000D0rM-1XVm; Thu, 12 Mar 2026 01:33:21 +0000 Received: from [192.168.61.151] (unknown [110.83.51.2]) by smtp.qiye.163.com (Hmail) with ESMTP id 36a07fd9a; Thu, 12 Mar 2026 09:33:14 +0800 (GMT+08:00) Cc: shawn.lin@rock-chips.com, Robin Murphy , Manivannan Sadhasivam , Lorenzo Pieralisi , =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= , Rob Herring , Bjorn Helgaas , Heiko Stuebner , Niklas Cassel , Hans Zhang <18255117159@163.com>, Nicolas Frattaroli , Wilfred Mallawa , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, Anand Moon , Grimmauld , Greg Kroah-Hartman , "Rafael J. Wysocki" , driver-core@lists.linux.dev, Lukas Wunner Subject: Re: [PATCH v3] PCI: dw-rockchip: Enable async probe by default To: Danilo Krummrich , Manivannan Sadhasivam References: <20260226101032.1042-1-linux.amoon@gmail.com> <177260693908.10259.13055467642416391434.b4-ty@kernel.org> <87bc37ee-234c-4568-b72e-955c130a6838@arm.com> <5d88fb5b-e771-4ea6-8d2c-c5cfd21e5860@arm.com> From: Shawn Lin Message-ID: <7b5592e7-8a9d-2cf0-bb3e-e306a6337165@rock-chips.com> Date: Thu, 12 Mar 2026 09:33:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Tid: 0a9cdfad2d6809cckunm494d1a9c5f1dd X-HM-MType: 1 X-HM-Spam-Status: e1kfGhgUHx5ZQUpXWQgPGg8OCBgUHx5ZQUlOS1dZFg8aDwILHllBWSg2Ly tZV1koWUFITzdXWS1ZQUlXWQ8JGhUIEh9ZQVlCT0NCVklOHx1DSklNGB4YS1YVFAkWGhdVEwETFh oSFyQUDg9ZV1kYEgtZQVlKSktVQ0hVTkpVSVlXWRYaDxIVHRRZQVlPS0hVSktJT09PSFVKS0tVSk JLS1kG DKIM-Signature: a=rsa-sha256; b=Sed4lWnDmwfKYVOVkbc4Y2rVhQcql/iiS/F9miQlGz9uPMIyj3zv76EbAV+aISfTpvB1Pl0bTfUZ8ldLKi584Z60igIIaGZ1u201GT2RVB3PU4/whmHBZz4C1IcfkvB7Echqz08J9oPFiIQyh0MGyr09uqGtdX4YzS4OFnJPaHc=; s=default; c=relaxed/relaxed; d=rock-chips.com; v=1; bh=0ZuXcANpbO5f9lJpjp5DojLJZeC/g1ENjyYfealo8kY=; h=date:mime-version:subject:message-id:from; X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260311_183319_604552_DC9C1672 X-CRM114-Status: GOOD ( 31.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mani and Danilo, 在 2026/03/12 星期四 5:09, Danilo Krummrich 写道: > On Wed Mar 11, 2026 at 1:28 PM CET, Manivannan Sadhasivam wrote: >> On Wed, Mar 11, 2026 at 12:46:03PM +0100, Danilo Krummrich wrote: >>> On Wed Mar 11, 2026 at 6:24 AM CET, Manivannan Sadhasivam wrote: >>>> I have a contrary view here. If just a single driver or lib doesn't handle async >>>> probe, it cannot just force other drivers to not take the advantage of async >>>> probe. As I said above, enabling async probe easily saves a few hunderd ms or >>>> even more if there are more than one Root Port or Root Complex in an SoC. >>> >>> Then the driver or lib has to be fixed / improved first or the driver core has >>> to be enabled to deal with a case where PROBE_FORCE_SYNCHRONOUS is requested >>> from an async path, etc. >>> >>> In any case, applying the patch and breaking things (knowingly?) doesn't seem >>> like the correct approach. >>> >>>> I strongly agree with you here that the underlying issue should be fixed. But >>>> the real impact to end users is not this splat, but not having the boot time >>>> optimization that this patch brings in. As an end user, one would want their >>>> systems to boot quickly and they wouldn't bother much about a harmless warning >>>> splat appearing in the dmesg log. >>> >>> You mean quickly booting into a "harmless" potential deadlock condition the >>> warning splat tries to make people aware of? :) >>> >> >> Hmm, I overlooked the built-as-module part where the deadlock could be possible >> as indicated by the comment about the WARN_ON_ONCE(). >> >> But what is the path forward here? Do you want the phylib to fix the >> request_module() call or fix the driver core instead? > > Here are a few thoughts. > > In general, I think the best would be to get rid of the (affected) > PROBE_FORCE_SYNCHRONOUS cases. > > Now, I guess this can be pretty hard for a PCI controller driver, as you can't > really predict what ends up being probed from you async context, i.e. it could > even be some other bus controller and things could even propagate further. > > Not sure how big of a deal it is in practice though, there are not a lot of > PROBE_FORCE_SYNCHRONOUS drivers (left), but note that specifying neither > PROBE_FORCE_SYNCHRONOUS nor PROBE_PREFER_ASYNCHRONOUS currently results in > synchronous by default. > > (Also, quite some other PCI controller drivers do set PROBE_PREFER_ASYNCHRONOUS > and apparently got lucky with it.) > >>>From a driver-core perspective I think we're rather limited on what we can do; > we are already in async context at this point and can't magically go back to > initcall context. > > So, the only thing I can think of is to kick off work on a workqueue, which in > the end would be the same as the deferred probe handling. > > Another alternative would be to let the subsystem handle such cases, which in > this case would probably mean to handle the current_is_async() case in > pci_host_probe() or pci_bus_add_devices(). > > On the other hand, this would probably end up to be a subsystem specific > implementation of "kick of work on a workqueue". Actually, this is exactly what we've been doing in our downstream code[1] for quite a long time. We've even pushed this optimization down to the individual driver level. To be honest, I'm not particularly fond of this approach either. However, on our platform, we have more than 5 Root Port instances that need to be initialized during boot. When all of them probe synchronously, it significantly delays the execution of many other critical drivers. Our customers have been complaining about the boot time, and our downstream team had no real choice but to kick off a kthread similar to your suggestion. The code is very ugly and expose a lot of corner cases problem which wasted quite lot of time to fix. That's why I was hesitant to bring up the this approach upstream initially. But since you mentioned it, I think it's worth evaluating whether we can implement this at the subsystem level instead of forcing each driver to handle it individually. This way, the complexity would be centralized, and drivers that benefit from async probe (like multi-Root-Port SoCs) could opt-in without worrying about the request_module() deadlock scenario. What do you think? [1] https://github.com/rockchip-linux/kernel/blob/develop-6.6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L1764 > >> I can drop this patch in the meantime. But holding this prolong wouldn't help. >