| Message ID | 20240220194244.2056384-1-jannh@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d2efeb52c344c3e987d143dc5232f53f9b96c86a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethtool: avoid rebuilds on UTS_RELEASE change | expand |
On Tue, 20 Feb 2024 20:42:44 +0100 Jann Horn wrote: > Currently, when you switch between branches or something like that and > rebuild, net/ethtool/ioctl.c has to be built again because it depends > on UTS_RELEASE. > > By instead referencing a string variable stored in another object file, > this can be avoided. > > Signed-off-by: Jann Horn <jannh@google.com> > --- > (alternatively we could also use the utsname info from the current UTS > namespace, but that'd be a bit of a behavior change, and I wanted to > keep this change a no-op) Is this related to John's work from: https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/ ?
On Wed, Feb 21, 2024 at 8:23 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 20 Feb 2024 20:42:44 +0100 Jann Horn wrote: > > Currently, when you switch between branches or something like that and > > rebuild, net/ethtool/ioctl.c has to be built again because it depends > > on UTS_RELEASE. > > > > By instead referencing a string variable stored in another object file, > > this can be avoided. > > > > Signed-off-by: Jann Horn <jannh@google.com> > > --- > > (alternatively we could also use the utsname info from the current UTS > > namespace, but that'd be a bit of a behavior change, and I wanted to > > keep this change a no-op) > > Is this related to John's work from: > https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/ > ? Ah, I didn't see his patch, but that seems like he had the same idea (but implemented it less sloppily). You can drop this one then...
On Wed, 21 Feb 2024 20:25:00 +0100 Jann Horn wrote: > > Is this related to John's work from: > > https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/ > > ? > > Ah, I didn't see his patch, but that seems like he had the same idea > (but implemented it less sloppily). You can drop this one then... No preference on my side, just wanted to make sure we don't have to decide within netdev which approach is better, not really our wheelhouse :)
On 21/02/2024 19:35, Jakub Kicinski wrote: Thanks for including me > On Wed, 21 Feb 2024 20:25:00 +0100 Jann Horn wrote: >>> Is this related to John's work from: >>> https://urldefense.com/v3/__https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/__;!!ACWV5N9M2RV99hQ!IghWg9_5HEQ-ng1XiYH9hes6vmgylmcQF5l2RITIrCSB20BzKzOhWWKHrgQZw_DkgKlZRNllglTanuY$ >>> ? >> >> Ah, I didn't see his patch, but that seems like he had the same idea >> (but implemented it less sloppily). You can drop this one then... > > No preference on my side, just wanted to make sure we don't have > to decide within netdev which approach is better, not really our > wheelhouse :) I was not going to pursue the change to introduce uts_release, as we can instead use init_uts_ns.name.release or init_utsname()->release instead. I think that init_utsname()->release is a bit neater to use, as it slightly hides the init_uts_ns structure. @Jann, please feel free to pursue upstreaming the change in this patch with: Reviewed-by: John Garry <john.g.garry@oracle.com> However, note that I do have many other changes pending on this subject, see: https://github.com/johnpgarry/linux/commits/uts-version-string/ I just need to find a bit of time to update and post them. We also need to be wary of CONFIG_MODVERSIONS, as discussed in that thread. Thanks, John
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 20 Feb 2024 20:42:44 +0100 you wrote: > Currently, when you switch between branches or something like that and > rebuild, net/ethtool/ioctl.c has to be built again because it depends > on UTS_RELEASE. > > By instead referencing a string variable stored in another object file, > this can be avoided. > > [...] Here is the summary with links: - net: ethtool: avoid rebuilds on UTS_RELEASE change https://git.kernel.org/netdev/net-next/c/d2efeb52c344 You are awesome, thank you!
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 7519b0818b91..575642b3070e 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -26,12 +26,12 @@ #include <linux/sched/signal.h> #include <linux/net.h> #include <linux/pm_runtime.h> +#include <linux/utsname.h> #include <net/devlink.h> #include <net/ipv6.h> #include <net/xdp_sock_drv.h> #include <net/flow_offload.h> #include <linux/ethtool_netlink.h> -#include <generated/utsrelease.h> #include "common.h" /* State held across locks and calls for commands which have devlink fallback */ @@ -713,7 +713,8 @@ ethtool_get_drvinfo(struct net_device *dev, struct ethtool_devlink_compat *rsp) struct device *parent = dev->dev.parent; rsp->info.cmd = ETHTOOL_GDRVINFO; - strscpy(rsp->info.version, UTS_RELEASE, sizeof(rsp->info.version)); + strscpy(rsp->info.version, init_uts_ns.name.release, + sizeof(rsp->info.version)); if (ops->get_drvinfo) { ops->get_drvinfo(dev, &rsp->info); if (!rsp->info.bus_info[0] && parent)
Currently, when you switch between branches or something like that and rebuild, net/ethtool/ioctl.c has to be built again because it depends on UTS_RELEASE. By instead referencing a string variable stored in another object file, this can be avoided. Signed-off-by: Jann Horn <jannh@google.com> --- (alternatively we could also use the utsname info from the current UTS namespace, but that'd be a bit of a behavior change, and I wanted to keep this change a no-op) net/ethtool/ioctl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)