| Message ID | 20210207161352.2044572-1-sashal@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: simplify access to the kernel's version | expand |
On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: > Instead of storing the version in a single integer and having various > kernel (and userspace) code how it's constructed, export individual > (major, patchlevel, sublevel) components and simplify kernel code that > uses it. > > This should also make it easier on userspace. > > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > Makefile | 5 ++++- > drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++-- > drivers/usb/core/hcd.c | 4 ++-- > drivers/usb/gadget/udc/aspeed-vhub/hub.c | 4 ++-- > include/linux/usb/composite.h | 4 ++-- > kernel/sys.c | 2 +- > 6 files changed, 13 insertions(+), 10 deletions(-) Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: > + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), > + (u16)(LINUX_VERSION_SUBLEVEL)); No need for the casts and braces. Otherwise this looks good, but please also kill off KERNEL_VERSION and LINUX_KERNEL_VERSION entirely while you're at it.
On Mon, Feb 08, 2021 at 05:50:07PM +0000, Christoph Hellwig wrote: >On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: >> + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), >> + (u16)(LINUX_VERSION_SUBLEVEL)); > >No need for the casts and braces. > >Otherwise this looks good, but please also kill off KERNEL_VERSION >and LINUX_KERNEL_VERSION entirely while you're at it. I don't think there are in-tree users left? We can't remove it completely because userspace is still using it, so if we drop those userspace will be sad.
On Tue, Feb 9, 2021 at 3:20 AM Sasha Levin <sashal@kernel.org> wrote: > > On Mon, Feb 08, 2021 at 05:50:07PM +0000, Christoph Hellwig wrote: > >On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: > >> + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), > >> + (u16)(LINUX_VERSION_SUBLEVEL)); > > > >No need for the casts and braces. > > I agree. Shall I remove the casts when I apply this? > >Otherwise this looks good, but please also kill off KERNEL_VERSION > >and LINUX_KERNEL_VERSION entirely while you're at it. > > I don't think there are in-tree users left? > > We can't remove it completely because userspace is still using it, so if > we drop those userspace will be sad. Right. Once we export a macros to userspace, we cannot remove it. > -- > Thanks, > Sasha
On Fri, Feb 12, 2021 at 5:18 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Feb 9, 2021 at 3:20 AM Sasha Levin <sashal@kernel.org> wrote: > > > > On Mon, Feb 08, 2021 at 05:50:07PM +0000, Christoph Hellwig wrote: > > >On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: > > >> + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), > > >> + (u16)(LINUX_VERSION_SUBLEVEL)); > > > > > >No need for the casts and braces. > > > > > > I agree. > > Shall I remove the casts when I apply this? > > > > > > >Otherwise this looks good, but please also kill off KERNEL_VERSION > > >and LINUX_KERNEL_VERSION entirely while you're at it. > > > > I don't think there are in-tree users left? > > > > We can't remove it completely because userspace is still using it, so if > > we drop those userspace will be sad. > > > Right. > Once we export a macros to userspace, we cannot remove it. > > > > > > > -- > > Thanks, > > Sasha > > > > -- > Best Regards > Masahiro Yamada Wait, this patch does not work. None of the submitter or reviewers tested this patch... Sigh. masahiro@grover:~/workspace/linux-kbuild$ make Makefile:1249: *** missing 'endef', unterminated 'define'. Stop. diff --git a/Makefile b/Makefile index ef81c8895abf..1fdd44fe1659 100644 --- a/Makefile +++ b/Makefile @@ -1258,7 +1258,7 @@ define filechk_version.h ((c) > 255 ? 255 : (c)))'; \ echo \#define LINUX_VERSION_MAJOR $(VERSION); \ echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL); \ - echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL); \ + echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL) endef $(version_h): FORCE
On Fri, Feb 12, 2021 at 12:40 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Feb 12, 2021 at 5:18 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Tue, Feb 9, 2021 at 3:20 AM Sasha Levin <sashal@kernel.org> wrote: > > > > > > On Mon, Feb 08, 2021 at 05:50:07PM +0000, Christoph Hellwig wrote: > > > >On Sun, Feb 07, 2021 at 11:13:52AM -0500, Sasha Levin wrote: > > > >> + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), > > > >> + (u16)(LINUX_VERSION_SUBLEVEL)); > > > > > > > >No need for the casts and braces. > > > > > > > > > > I agree. > > > > Shall I remove the casts when I apply this? > > > > > > > > > > > >Otherwise this looks good, but please also kill off KERNEL_VERSION > > > >and LINUX_KERNEL_VERSION entirely while you're at it. > > > > > > I don't think there are in-tree users left? > > > > > > We can't remove it completely because userspace is still using it, so if > > > we drop those userspace will be sad. > > > > > > Right. > > Once we export a macros to userspace, we cannot remove it. > > > > > > > > > > > > > -- > > > Thanks, > > > Sasha > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > > > > > Wait, this patch does not work. > None of the submitter or reviewers > tested this patch... Sigh. > > > masahiro@grover:~/workspace/linux-kbuild$ make > Makefile:1249: *** missing 'endef', unterminated 'define'. Stop. > > > > > > > > diff --git a/Makefile b/Makefile > index ef81c8895abf..1fdd44fe1659 100644 > --- a/Makefile > +++ b/Makefile > @@ -1258,7 +1258,7 @@ define filechk_version.h > ((c) > 255 ? 255 : (c)))'; \ > echo \#define LINUX_VERSION_MAJOR $(VERSION); \ > echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL); \ > - echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL); \ > + echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL) > endef > > $(version_h): FORCE > > > > > > -- > Best Regards > Masahiro Yamada Please send a correct patch, also drop the unneeded casts.
On Fri, Feb 12, 2021 at 12:41:21PM +0900, Masahiro Yamada wrote: >Please send a correct patch, >also drop the unneeded casts. Sorry about that, I've hand edited the patch right before sending it out and obviously messed it up. I'll resend.
diff --git a/Makefile b/Makefile index 157be50c691e5..8e002fb5cae7b 100644 --- a/Makefile +++ b/Makefile @@ -1266,7 +1266,10 @@ define filechk_version.h expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ fi; \ echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ - ((c) > 255 ? 255 : (c)))' + ((c) > 255 ? 255 : (c)))'; \ + echo \#define LINUX_VERSION_MAJOR $(VERSION); \ + echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL); \ + echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL); \ endef $(version_h): FORCE diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index e4c9627485aa5..989f15d9aa7d4 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -237,8 +237,8 @@ static void mlx5_set_driver_version(struct mlx5_core_dev *dev) remaining_size = max_t(int, 0, driver_ver_sz - strlen(string)); snprintf(string + strlen(string), remaining_size, "%u.%u.%u", - (u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >> 8) & 0xff), - (u16)(LINUX_VERSION_CODE & 0xffff)); + (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL), + (u16)(LINUX_VERSION_SUBLEVEL)); /*Send the command*/ MLX5_SET(set_driver_version_in, in, opcode, diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index ad5a0f405a75c..3f0381344221e 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -111,8 +111,8 @@ DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue); */ /*-------------------------------------------------------------------------*/ -#define KERNEL_REL bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff)) -#define KERNEL_VER bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff)) +#define KERNEL_REL bin2bcd(LINUX_VERSION_MAJOR) +#define KERNEL_VER bin2bcd(LINUX_VERSION_PATCHLEVEL) /* usb 3.1 root hub device descriptor */ static const u8 usb31_rh_dev_descriptor[18] = { diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c index bfd8e77788e29..5c7dea5e0ff16 100644 --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c @@ -46,8 +46,8 @@ * - Make vid/did overridable * - make it look like usb1 if usb1 mode forced */ -#define KERNEL_REL bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff)) -#define KERNEL_VER bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff)) +#define KERNEL_REL bin2bcd(LINUX_VERSION_MAJOR) +#define KERNEL_VER bin2bcd(LINUX_VERSION_PATCHLEVEL) enum { AST_VHUB_STR_INDEX_MAX = 4, diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 5646dad886e61..c71150f2c6390 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -575,8 +575,8 @@ static inline u16 get_default_bcdDevice(void) { u16 bcdDevice; - bcdDevice = bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8; - bcdDevice |= bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff)); + bcdDevice = bin2bcd(LINUX_VERSION_MAJOR) << 8; + bcdDevice |= bin2bcd(LINUX_VERSION_PATCHLEVEL); return bcdDevice; } diff --git a/kernel/sys.c b/kernel/sys.c index 8bb46e50f02d4..b09fe21e88ff5 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -1242,7 +1242,7 @@ static int override_release(char __user *release, size_t len) break; rest++; } - v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60; + v = LINUX_VERSION_PATCHLEVEL + 60; copy = clamp_t(size_t, len, 1, sizeof(buf)); copy = scnprintf(buf, copy, "2.6.%u%s", v, rest); ret = copy_to_user(release, buf, copy + 1);
Instead of storing the version in a single integer and having various kernel (and userspace) code how it's constructed, export individual (major, patchlevel, sublevel) components and simplify kernel code that uses it. This should also make it easier on userspace. Signed-off-by: Sasha Levin <sashal@kernel.org> --- Makefile | 5 ++++- drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++-- drivers/usb/core/hcd.c | 4 ++-- drivers/usb/gadget/udc/aspeed-vhub/hub.c | 4 ++-- include/linux/usb/composite.h | 4 ++-- kernel/sys.c | 2 +- 6 files changed, 13 insertions(+), 10 deletions(-)