Re: [PATCH] net: phy: don't issue a module request if a driver is available
From: Andrew Lunn <andrew@lunn.ch>
Date: 2025-01-02 13:52:25
On Thu, Jan 02, 2025 at 02:26:58PM +0100, Francesco Valla wrote:
On Thursday, 2 January 2025 at 12:06:15 Russell King (Oracle) [off-list ref] wrote:quoted
On Thu, Jan 02, 2025 at 12:51:22AM +0100, Francesco Valla wrote:quoted
Whenever a new PHY device is created, request_module() is called unconditionally, without checking if a driver for the new PHY is already available (either built-in or from a previous probe). This conflicts with async probing of the underlying MDIO bus and always throws a warning (because if a driver is loaded it _might_ cause a deadlock, if in turn it calls async_synchronize_full()).Why aren't any of the phylib maintainers seeing this warning? Where does the warning come from?I'm not sure. For me, it was pretty easy to trigger.
Please include the information how you triggered it into the commit message.
This is expected, as request_module() is not meant to be called from an async context: https://lore.kernel.org/lkml/20130118221227.GG24579@htj.dyndns.org/ (local) It should be noted that: - the davincio_mdio device is a child of the am65-cpsw-nuss device - the am65-cpsw-nuss driver is NOT marked with neither PROBE_PREFER_ASYNCHRONOUS nor PROBE_FORCE_SYNCHRONOUS and the behavior is being triggered specifying driver_async_probe=am65-cpsw-nuss on the command line.
So the phylib core is currently async probe incompatible. The whole module loading story is a bit shaky in phylib, so we need to be very careful with any changes, or you are going to break stuff, in interesting ways, with it first appearing to work, because the fallback genphy is used rather than the specific PHY driver, but then breaking when genphy is not sufficient. Please think about this as a generic problem with async probe. Is this really specific to phylib? Should some or all of the solution to the problem be moved into the driver core? Could we maybe first try an async probe using the existing drivers, and then fall back to a sync probe which can load additional drivers? One other question, how much speadup do you get with async probe of PHYs? Is it really worth the effort? Andrew