From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Nathan Chancellor <nathan@kernel.org>,
davem@davemloft.net, Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>
Cc: "Maxime Chevallier" <maxime.chevallier@bootlin.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
thomas.petazzoni@bootlin.com, "Andrew Lunn" <andrew@lunn.ch>,
"Eric Dumazet" <edumazet@google.com>,
linux-arm-kernel@lists.infradead.org,
"Christophe Leroy" <christophe.leroy@csgroup.eu>,
"Herve Codina" <herve.codina@bootlin.com>,
"Florian Fainelli" <f.fainelli@gmail.com>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Vladimir Oltean" <vladimir.oltean@nxp.com>,
"Köry Maincent" <kory.maincent@bootlin.com>,
"Jesse Brandeburg" <jesse.brandeburg@intel.com>,
"Marek Behún" <kabel@kernel.org>,
"Piergiorgio Beruto" <piergiorgio.beruto@gmail.com>,
"Oleksij Rempel" <o.rempel@pengutronix.de>,
"Nicolò Veronese" <nicveronese@gmail.com>,
"Simon Horman" <horms@kernel.org>,
mwojtas@chromium.org, "Antoine Tenart" <atenart@kernel.org>
Subject: Re: [PATCH net-next 0/2] Fix phy_link_topology initialization
Date: Mon, 13 May 2024 10:15:48 +0100 [thread overview]
Message-ID: <ZkHaRD8WGrhrzemn@shell.armlinux.org.uk> (raw)
In-Reply-To: <20240513063636.GA652533@thelio-3990X>
On Sun, May 12, 2024 at 11:36:36PM -0700, Nathan Chancellor wrote:
> Hi Maxime,
>
> On Tue, May 07, 2024 at 12:28:19PM +0200, Maxime Chevallier wrote:
> > Nathan and Heiner reported issues that occur when phylib and phy drivers
> > built as modules expect the phy_link_topology to be initialized, due to
> > wrong use of IS_REACHABLE.
> >
> > This small fixup series addresses that by moving the initialization code
> > into net/core/dev.c, but at the same time implementing lazy
> > initialization to only allocate the topology upon the first PHY
> > insertion.
> >
> > This needed some refactoring, namely pass the netdevice itself as a
> > parameter for phy_link_topology helpers.
> >
> > Thanks Heiner for the help on untangling this, and Nathan for the
> > report.
>
> Are you able to prioritize getting this series merged? This has been a
> problem in -next for over a month now and the merge window is now open.
> I would hate to see this regress in mainline, as my main system may be
> affected by it (not sure, I got a new test machine that got bit by it in
> addition to the other two I noticed it on).
... and Maxime has been working on trying to get an acceptable fix for
it over that time, with to-and-fro discussions. Maxime still hasn't got
an ack from Heiner for the fixes, and changes are still being
requested.
I think, sadly, the only way forward at this point would be to revert
the original commit. I've just tried reverting 6916e461e793 in my
net-next tree and it's possible, although a little noisy:
$ git revert 6916e461e793
Performing inexact rename detection: 100% (8904/8904), done.
Auto-merging net/core/dev.c
Auto-merging include/uapi/linux/ethtool.h
Removing include/linux/phy_link_topology_core.h
Removing include/linux/phy_link_topology.h
Auto-merging include/linux/phy.h
Auto-merging include/linux/netdevice.h
Removing drivers/net/phy/phy_link_topology.c
Auto-merging drivers/net/phy/phy_device.c
Auto-merging MAINTAINERS
hint: Waiting for your editor to close the file...
I haven't checked whether that ends up with something that's buildable.
Any views Jakub/Dave/Paolo?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Nathan Chancellor <nathan@kernel.org>,
davem@davemloft.net, Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>
Cc: "Maxime Chevallier" <maxime.chevallier@bootlin.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
thomas.petazzoni@bootlin.com, "Andrew Lunn" <andrew@lunn.ch>,
"Eric Dumazet" <edumazet@google.com>,
linux-arm-kernel@lists.infradead.org,
"Christophe Leroy" <christophe.leroy@csgroup.eu>,
"Herve Codina" <herve.codina@bootlin.com>,
"Florian Fainelli" <f.fainelli@gmail.com>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Vladimir Oltean" <vladimir.oltean@nxp.com>,
"Köry Maincent" <kory.maincent@bootlin.com>,
"Jesse Brandeburg" <jesse.brandeburg@intel.com>,
"Marek Behún" <kabel@kernel.org>,
"Piergiorgio Beruto" <piergiorgio.beruto@gmail.com>,
"Oleksij Rempel" <o.rempel@pengutronix.de>,
"Nicolò Veronese" <nicveronese@gmail.com>,
"Simon Horman" <horms@kernel.org>,
mwojtas@chromium.org, "Antoine Tenart" <atenart@kernel.org>
Subject: Re: [PATCH net-next 0/2] Fix phy_link_topology initialization
Date: Mon, 13 May 2024 10:15:48 +0100 [thread overview]
Message-ID: <ZkHaRD8WGrhrzemn@shell.armlinux.org.uk> (raw)
In-Reply-To: <20240513063636.GA652533@thelio-3990X>
On Sun, May 12, 2024 at 11:36:36PM -0700, Nathan Chancellor wrote:
> Hi Maxime,
>
> On Tue, May 07, 2024 at 12:28:19PM +0200, Maxime Chevallier wrote:
> > Nathan and Heiner reported issues that occur when phylib and phy drivers
> > built as modules expect the phy_link_topology to be initialized, due to
> > wrong use of IS_REACHABLE.
> >
> > This small fixup series addresses that by moving the initialization code
> > into net/core/dev.c, but at the same time implementing lazy
> > initialization to only allocate the topology upon the first PHY
> > insertion.
> >
> > This needed some refactoring, namely pass the netdevice itself as a
> > parameter for phy_link_topology helpers.
> >
> > Thanks Heiner for the help on untangling this, and Nathan for the
> > report.
>
> Are you able to prioritize getting this series merged? This has been a
> problem in -next for over a month now and the merge window is now open.
> I would hate to see this regress in mainline, as my main system may be
> affected by it (not sure, I got a new test machine that got bit by it in
> addition to the other two I noticed it on).
... and Maxime has been working on trying to get an acceptable fix for
it over that time, with to-and-fro discussions. Maxime still hasn't got
an ack from Heiner for the fixes, and changes are still being
requested.
I think, sadly, the only way forward at this point would be to revert
the original commit. I've just tried reverting 6916e461e793 in my
net-next tree and it's possible, although a little noisy:
$ git revert 6916e461e793
Performing inexact rename detection: 100% (8904/8904), done.
Auto-merging net/core/dev.c
Auto-merging include/uapi/linux/ethtool.h
Removing include/linux/phy_link_topology_core.h
Removing include/linux/phy_link_topology.h
Auto-merging include/linux/phy.h
Auto-merging include/linux/netdevice.h
Removing drivers/net/phy/phy_link_topology.c
Auto-merging drivers/net/phy/phy_device.c
Auto-merging MAINTAINERS
hint: Waiting for your editor to close the file...
I haven't checked whether that ends up with something that's buildable.
Any views Jakub/Dave/Paolo?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-05-13 9:18 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 10:28 [PATCH net-next 0/2] Fix phy_link_topology initialization Maxime Chevallier
2024-05-07 10:28 ` Maxime Chevallier
2024-05-07 10:28 ` [PATCH net-next 1/2] net: phy: phy_link_topology: Pass netdevice to phy_link_topo helpers Maxime Chevallier
2024-05-07 10:28 ` Maxime Chevallier
2024-05-08 5:43 ` Heiner Kallweit
2024-05-08 5:43 ` Heiner Kallweit
2024-05-07 10:28 ` [PATCH net-next 2/2] net: phy: phy_link_topology: Lazy-initialize the link topology Maxime Chevallier
2024-05-07 10:28 ` Maxime Chevallier
2024-05-07 23:08 ` kernel test robot
2024-05-07 23:08 ` kernel test robot
2024-05-08 5:44 ` Heiner Kallweit
2024-05-08 5:44 ` Heiner Kallweit
2024-05-13 7:30 ` Maxime Chevallier
2024-05-13 7:30 ` Maxime Chevallier
2024-05-13 8:07 ` Maxime Chevallier
2024-05-13 8:07 ` Maxime Chevallier
2024-05-13 8:15 ` Maxime Chevallier
2024-05-13 8:15 ` Maxime Chevallier
2024-05-13 6:36 ` [PATCH net-next 0/2] Fix phy_link_topology initialization Nathan Chancellor
2024-05-13 6:36 ` Nathan Chancellor
2024-05-13 9:15 ` Russell King (Oracle) [this message]
2024-05-13 9:15 ` Russell King (Oracle)
2024-05-13 15:11 ` Jakub Kicinski
2024-05-13 15:11 ` Jakub Kicinski
2024-05-14 6:46 ` Maxime Chevallier
2024-05-14 6:46 ` Maxime Chevallier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZkHaRD8WGrhrzemn@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=atenart@kernel.org \
--cc=christophe.leroy@csgroup.eu \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=herve.codina@bootlin.com \
--cc=hkallweit1@gmail.com \
--cc=horms@kernel.org \
--cc=jesse.brandeburg@intel.com \
--cc=kabel@kernel.org \
--cc=kory.maincent@bootlin.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.chevallier@bootlin.com \
--cc=mwojtas@chromium.org \
--cc=nathan@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nicveronese@gmail.com \
--cc=o.rempel@pengutronix.de \
--cc=pabeni@redhat.com \
--cc=piergiorgio.beruto@gmail.com \
--cc=thomas.petazzoni@bootlin.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.