Message-ID: <24a0f3fa-2121-4de3-89fd-482b217ab98d@linux.dev>
Date: Tue, 17 Jun 2025 13:14:31 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Saravana Kannan <saravanak@...gle.com>,
 "Rafael J . Wysocki" <rafael@...nel.org>, Danilo Krummrich
 <dakr@...nel.org>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 Christoph Hellwig <hch@....de>, Rob Herring <robh+dt@...nel.org>,
 Grant Likely <grant.likely@...aro.org>
Subject: Re: [PATCH] driver core: Prevent deferred probe loops

On 6/17/25 11:49, Greg Kroah-Hartman wrote:
> On Tue, Jun 17, 2025 at 11:35:04AM -0400, Sean Anderson wrote:
>> On 6/17/25 04:50, Greg Kroah-Hartman wrote:
>> > On Thu, Jun 12, 2025 at 04:40:48PM -0400, Sean Anderson wrote:
>> >> On 6/12/25 13:56, Saravana Kannan wrote:
>> >> > On Thu, Jun 12, 2025 at 8:53 AM Sean Anderson <sean.anderson@...ux.dev> wrote:
>> >> >>
>> >> >> On 6/11/25 08:23, Greg Kroah-Hartman wrote:
>> >> >> > On Tue, Jun 10, 2025 at 07:44:27PM -0400, Sean Anderson wrote:
>> >> >> >> On 6/10/25 19:32, Saravana Kannan wrote:
>> >> >> >> > On Tue, Jun 10, 2025 at 11:35 AM Sean Anderson <sean.anderson@...ux.dev> wrote:
>> >> >> >> >>
>> >> >> >> >> A deferred probe loop can occur when a device returns EPROBE_DEFER after
>> >> >> >> >> registering a bus with children:
>> >> >> >> >
>> >> >> >> > This is a broken driver. A parent device shouldn't register child
>> >> >> >> > devices unless it is fully read itself. It's not logical to say the
>> >> >> >> > child devices are available, if the parent itself isn't fully ready.
>> >> >> >> > So, adding child devices/the bus should be the last thing done in the
>> >> >> >> > parent's probe function.
>> >> >> >> >
>> >> >> >> > I know there are odd exceptions where the parent depends on the child,
>> >> >> >> > so they might add the child a bit earlier in the probe
>> >> >> >>
>> >> >> >> This is exactly the case here. So the bus probing cannot happen any
>> >> >> >> later than it already does.
>> >> >> >
>> >> >> > Please fix the driver not to do this.
>> >> >>
>> >> >> How? The driver needs the PCS to work. And the PCS can live on the MDIO
>> >> >> bus.
>> >> > 
>> >> > Obviously I don't know the full details, but you could implement it as
>> >> > MFD. So the bus part would not get removed even if the PCS fails to
>> >> > probe. Then the PCS can probe when whatever it needs ends up probing.
>> >> 
>> >> I was thinking about making the MDIO bus a separate device. But I think
>> >> it will be tricky to get suspend/resume working correctly. And this
>> >> makes conversions more difficult because you cannot just add some
>> >> pcs_get/pcs_put calls, you have to split out the MDIO bus too (which is
>> >> invariably created as a child of the MAC).
>> >> 
>> >> And what happens if a developer doesn't realize they have to split off
>> >> the MDIO bus before converting? Everything works fine, except if there
>> >> is some problem loading the PCS driver, which they may not test. Is this
>> >> prohibition against failing after creating a bus documented anywhere? I
>> >> don't recall seeing it...
>> > 
>> > What do you mean "failing after creating a bus"?  If a bus is failed to
>> > be created, you fail like normal, no difference here.
>> 
>> Creating the bus is successful, but there's an EPROBE_DEFER failure after
>> that. Which induces the probe loop as described in my initial email.
> 
> Then don't allow a defer to happen :)

Well, I could require all PCS drivers to be built-in I guess. But I suspect
users will want them to be modules to reduce kernel size.

> Or better yet, just succeed and spin up a new thread for the new bus to
> attach it's devices to.  That's what many other busses do today.

Sorry, I'm not sure I follow. How can you attach a device to a thread? Do
you have an example for this?

--Sean

Powered by blists - more mailing lists