On Thursday, 2 January 2025 at 14:52:18 Andrew Lunn [off-list ref] wrote:

On Thu, Jan 02, 2025 at 02:26:58PM +0100, Francesco Valla wrote:

quoted

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.

Ok, will do.

quoted

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?

It probably isn't, but considering the way phylib works currently (with the
genphy filling up for missing drivers etc.) I'm not sure if/how this can work.
I need to think about it.

One other question, how much speadup do you get with async probe of
PHYs? Is it really worth the effort?

For me it's a reduction of ~170ms, which currently accounts for roughly the 25%
of the time spent before starting userspace (660ms vs 490ms, give or take a
couple of milliseconds). That's due to the large reset time required by the PHYs
to initialize, so I expect it would be much lower on most of the systems.
But, I've done much more to shave much less time in the past, so I think it is
at least worth investigating.

Thank you!

Regards,
Francesco

	Andrew