Thread (18 messages) 18 messages, 5 authors, 2025-01-03

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help