mbox series

[net,v2,0/3] Clean up and fix error handling in mdio_mux_init()

Message ID 20210817180841.3210484-1-saravanak@google.com
Headers show
Series Clean up and fix error handling in mdio_mux_init() | expand

Message

Saravana Kannan Aug. 17, 2021, 6:08 p.m. UTC 
This patch series was started due to -EPROBE_DEFER not being handled
correctly in mdio_mux_init() and causing issues [1]. While at it, I also
did some more error handling fixes and clean ups. The -EPROBE_DEFER fix is
the last patch.

Ideally, in the last patch we'd treat any error similar to -EPROBE_DEFER
but I'm not sure if it'll break any board/platforms where some child
mdiobus never successfully registers. If we treated all errors similar to
-EPROBE_DEFER, then none of the child mdiobus will work and that might be a
regression. If people are sure this is not a real case, then I can fix up
the last patch to always fail the entire mdio-mux init if any of the child
mdiobus registration fails.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Kevin Hilman <khilman@baylibre.com>
[1] - https://lore.kernel.org/lkml/CAGETcx95kHrv8wA-O+-JtfH7H9biJEGJtijuPVN0V5dUKUAB3A@mail.gmail.com/#t

v1 -> v2:
- Added Acked-by, Tested-by and Reviewed-by
- Fixing the subject so it goes to "net" tree

Saravana Kannan (3):
  net: mdio-mux: Delete unnecessary devm_kfree
  net: mdio-mux: Don't ignore memory allocation errors
  net: mdio-mux: Handle -EPROBE_DEFER correctly

 drivers/net/mdio/mdio-mux.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

Comments

Saravana Kannan Aug. 18, 2021, 2:56 a.m. UTC | #1 
On Tue, Aug 17, 2021 at 2:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
>

> On Tue, Aug 17, 2021 at 11:08:39AM -0700, Saravana Kannan wrote:

> > The whole point of devm_* APIs is that you don't have to undo them if you

> > are returning an error that's going to get propagated out of a probe()

> > function. So delete unnecessary devm_kfree() call in the error return path.

> >

> > Signed-off-by: Saravana Kannan <saravanak@google.com>

> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>

> > Acked-by: Marc Zyngier <maz@kernel.org>

> > Tested-by: Marc Zyngier <maz@kernel.org>

> > Acked-by: Kevin Hilman <khilman@baylibre.com>

> > Tested-by: Kevin Hilman <khilman@baylibre.com>

>

> Please add a Fixes: tag, since you want this in stable.

>

> All three patches need fixes tags, possibly different for each patch?


I generally ask for patches to be picked up by stable only if it fixes
a bug that puts the kernel in a bad state or if it fixes an issue
someone actually reported on the stable kernel. In this case, it's
just failing device probes in some cases and I didn't think that met
the bar for stable. But if you think they should, then that's fine by
me.

I'll send out v3 patches with Fixes. I'm fairly sure these issues were
present since the time mdio-mux was added. Hopefully v3 will be the
last version I have to send out :)

-Saravana
Andrew Lunn Aug. 18, 2021, 6:46 p.m. UTC | #2 
On Tue, Aug 17, 2021 at 07:56:42PM -0700, Saravana Kannan wrote:
> On Tue, Aug 17, 2021 at 2:10 PM Andrew Lunn <andrew@lunn.ch> wrote:

> >

> > On Tue, Aug 17, 2021 at 11:08:39AM -0700, Saravana Kannan wrote:

> > > The whole point of devm_* APIs is that you don't have to undo them if you

> > > are returning an error that's going to get propagated out of a probe()

> > > function. So delete unnecessary devm_kfree() call in the error return path.

> > >

> > > Signed-off-by: Saravana Kannan <saravanak@google.com>

> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>

> > > Acked-by: Marc Zyngier <maz@kernel.org>

> > > Tested-by: Marc Zyngier <maz@kernel.org>

> > > Acked-by: Kevin Hilman <khilman@baylibre.com>

> > > Tested-by: Kevin Hilman <khilman@baylibre.com>

> >

> > Please add a Fixes: tag, since you want this in stable.

> >

> > All three patches need fixes tags, possibly different for each patch?

> 

> I generally ask for patches to be picked up by stable only if it fixes

> a bug that puts the kernel in a bad state


Actually, you asked for stable. You set the subject to

[PATCH net v3 0/3] Clean up and fix error

where [PATCH net ] means stable. If you think this is just ongoing
development work, use [PATCH net-next ]. Then the Fixes tags are
optional.

	Andrew