| Message ID | 20240417142707.2082523-1-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] net: phy: link_topology: Don't stub-away the topology creation | expand |
Hi Maxime, On Wed, Apr 17, 2024 at 04:27:05PM +0200, Maxime Chevallier wrote: > Some of the phy_link_topology operations are protected by IS_REACHABLE, > which can lead to scenarios where the consumer, built as modules, sees the topology > unstubbed, whereas the initialization didn't occur. > > Don't stub away the creation of the topology, it has no dependency on > any other parts like phylib, so we can make it always available. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/ > Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/ > --- > Hi Nathan, Heiner, > > I'm currently at EOSS, so I'm sending this patch without having been > able to properly test it (build-tested only), but as this addresses an > issue for people using -next, I'm sending this anyway, sorry about that. No worries, thanks for continuing to take a look. Unfortunately, this patch fails to build during the linking stage for me with my configuration: x86_64-linux-ld: vmlinux.o: in function `free_netdev': net/core/dev.c:11060:(.text+0xb14030): undefined reference to `phy_link_topo_destroy' x86_64-linux-ld: vmlinux.o: in function `alloc_netdev_mqs': net/core/dev.c:10966:(.text+0xb142d6): undefined reference to `phy_link_topo_create' > Hopefully it can address the issue for now, I haven't given-up on your > idea to introduce a config option Heiner :) > > Thanks, > > Maxime > > include/linux/phy_link_topology_core.h | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h > index 0a6479055745..61e2592f24ac 100644 > --- a/include/linux/phy_link_topology_core.h > +++ b/include/linux/phy_link_topology_core.h > @@ -4,22 +4,7 @@ > > struct phy_link_topology; > > -#if IS_REACHABLE(CONFIG_PHYLIB) > - > struct phy_link_topology *phy_link_topo_create(struct net_device *dev); > void phy_link_topo_destroy(struct phy_link_topology *topo); > > -#else > - > -static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev) > -{ > - return NULL; > -} > - > -static inline void phy_link_topo_destroy(struct phy_link_topology *topo) > -{ > -} > - > -#endif > - > #endif /* __PHY_LINK_TOPOLOGY_CORE_H */ > -- > 2.44.0 >
Hello Nathan, On Wed, 17 Apr 2024 11:07:21 -0700 Nathan Chancellor <nathan@kernel.org> wrote: > Hi Maxime, > > On Wed, Apr 17, 2024 at 04:27:05PM +0200, Maxime Chevallier wrote: > > Some of the phy_link_topology operations are protected by IS_REACHABLE, > > which can lead to scenarios where the consumer, built as modules, sees the topology > > unstubbed, whereas the initialization didn't occur. > > > > Don't stub away the creation of the topology, it has no dependency on > > any other parts like phylib, so we can make it always available. > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > > Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/ > > Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/ > > --- > > Hi Nathan, Heiner, > > > > I'm currently at EOSS, so I'm sending this patch without having been > > able to properly test it (build-tested only), but as this addresses an > > issue for people using -next, I'm sending this anyway, sorry about that. > > No worries, thanks for continuing to take a look. Unfortunately, this > patch fails to build during the linking stage for me with my > configuration: > > x86_64-linux-ld: vmlinux.o: in function `free_netdev': > net/core/dev.c:11060:(.text+0xb14030): undefined reference to `phy_link_topo_destroy' > x86_64-linux-ld: vmlinux.o: in function `alloc_netdev_mqs': > net/core/dev.c:10966:(.text+0xb142d6): undefined reference to `phy_link_topo_create' Heh serves me well for trying to go too fast :) phy_link_topo_create then need to compile unconditionally, so I'm indeed missing some bits here. Thanks a lot for testing, Maxime > > > Hopefully it can address the issue for now, I haven't given-up on your > > idea to introduce a config option Heiner :) > > > > Thanks, > > > > Maxime > > > > include/linux/phy_link_topology_core.h | 15 --------------- > > 1 file changed, 15 deletions(-) > > > > diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h > > index 0a6479055745..61e2592f24ac 100644 > > --- a/include/linux/phy_link_topology_core.h > > +++ b/include/linux/phy_link_topology_core.h > > @@ -4,22 +4,7 @@ > > > > struct phy_link_topology; > > > > -#if IS_REACHABLE(CONFIG_PHYLIB) > > - > > struct phy_link_topology *phy_link_topo_create(struct net_device *dev); > > void phy_link_topo_destroy(struct phy_link_topology *topo); > > > > -#else > > - > > -static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev) > > -{ > > - return NULL; > > -} > > - > > -static inline void phy_link_topo_destroy(struct phy_link_topology *topo) > > -{ > > -} > > - > > -#endif > > - > > #endif /* __PHY_LINK_TOPOLOGY_CORE_H */ > > -- > > 2.44.0 > >
diff --git a/include/linux/phy_link_topology_core.h b/include/linux/phy_link_topology_core.h index 0a6479055745..61e2592f24ac 100644 --- a/include/linux/phy_link_topology_core.h +++ b/include/linux/phy_link_topology_core.h @@ -4,22 +4,7 @@ struct phy_link_topology; -#if IS_REACHABLE(CONFIG_PHYLIB) - struct phy_link_topology *phy_link_topo_create(struct net_device *dev); void phy_link_topo_destroy(struct phy_link_topology *topo); -#else - -static inline struct phy_link_topology *phy_link_topo_create(struct net_device *dev) -{ - return NULL; -} - -static inline void phy_link_topo_destroy(struct phy_link_topology *topo) -{ -} - -#endif - #endif /* __PHY_LINK_TOPOLOGY_CORE_H */
Some of the phy_link_topology operations are protected by IS_REACHABLE, which can lead to scenarios where the consumer, built as modules, sees the topology unstubbed, whereas the initialization didn't occur. Don't stub away the creation of the topology, it has no dependency on any other parts like phylib, so we can make it always available. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/ Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/ --- Hi Nathan, Heiner, I'm currently at EOSS, so I'm sending this patch without having been able to properly test it (build-tested only), but as this addresses an issue for people using -next, I'm sending this anyway, sorry about that. Hopefully it can address the issue for now, I haven't given-up on your idea to introduce a config option Heiner :) Thanks, Maxime include/linux/phy_link_topology_core.h | 15 --------------- 1 file changed, 15 deletions(-)