[PATCH] kbuild: simplify access to the kernel's version

[PATCH] kbuild: simplify access to the kernel's version

[Sedat Dilek]

[ Please CC me directly ]

Hi Sasha,

I like that patch from [1].

What was the Linux-source base?

Searching in Linux v5.11-rc7:

$ git grep -E 'LINUX_VERSION_CODE >> (16|8)'
drivers/net/ethernet/mellanox/mlx5/core/main.c:
(u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >>
8) & 0xff),
drivers/scsi/gdth.c:        osv.version = (u8)(LINUX_VERSION_CODE >> 16);
drivers/scsi/gdth.c:        osv.subversion = (u8)(LINUX_VERSION_CODE >> 8);
drivers/usb/core/hcd.c:#define KERNEL_REL
bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
drivers/usb/core/hcd.c:#define KERNEL_VER
bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_REL
bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_VER
bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
include/linux/usb/composite.h:  bcdDevice =
bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
include/linux/usb/composite.h:  bcdDevice |=
bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
kernel/sys.c:           v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;

So, drivers/scsi/gdth.c file is missing in your list of file:

 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(-)

Thanks.

Regards,
- Sedat -

[1] https://marc.info/?l=linux-kbuild&m=161271450004520&w=2

Re: [PATCH] kbuild: simplify access to the kernel's version

[Sasha Levin]

On Mon, Feb 08, 2021 at 04:28:00AM +0100, Sedat Dilek wrote:
>[ Please CC me directly ]
>
>Hi Sasha,
>
>I like that patch from [1].
>
>What was the Linux-source base?
>
>Searching in Linux v5.11-rc7:
>
>$ git grep -E 'LINUX_VERSION_CODE >> (16|8)'
>drivers/net/ethernet/mellanox/mlx5/core/main.c:
>(u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >>
>8) & 0xff),
>drivers/scsi/gdth.c:        osv.version = (u8)(LINUX_VERSION_CODE >> 16);
>drivers/scsi/gdth.c:        osv.subversion = (u8)(LINUX_VERSION_CODE >> 8);
>drivers/usb/core/hcd.c:#define KERNEL_REL
>bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
>drivers/usb/core/hcd.c:#define KERNEL_VER
>bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
>drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_REL
>bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
>drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_VER
>bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
>include/linux/usb/composite.h:  bcdDevice =
>bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
>include/linux/usb/composite.h:  bcdDevice |=
>bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
>kernel/sys.c:           v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;
>
>So, drivers/scsi/gdth.c file is missing in your list of file:

Right. It's on it's way out: 0653c358d2dc ("scsi: Drop gdth driver").

In theory I should have fixed it too, but given that this patch is
cosmetic I just wanted to avoid unnecessary conflicts for SFR... If it
doesn't end up being removed we can always fix it up there.

-- 
Thanks,
Sasha

Re: [PATCH] kbuild: simplify access to the kernel's version

[Sedat Dilek]

On Mon, Feb 8, 2021 at 7:17 PM Sasha Levin <sashal@kernel.org> wrote:
>
> On Mon, Feb 08, 2021 at 04:28:00AM +0100, Sedat Dilek wrote:
> >[ Please CC me directly ]
> >
> >Hi Sasha,
> >
> >I like that patch from [1].
> >
> >What was the Linux-source base?
> >
> >Searching in Linux v5.11-rc7:
> >
> >$ git grep -E 'LINUX_VERSION_CODE >> (16|8)'
> >drivers/net/ethernet/mellanox/mlx5/core/main.c:
> >(u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >>
> >8) & 0xff),
> >drivers/scsi/gdth.c:        osv.version = (u8)(LINUX_VERSION_CODE >> 16);
> >drivers/scsi/gdth.c:        osv.subversion = (u8)(LINUX_VERSION_CODE >> 8);
> >drivers/usb/core/hcd.c:#define KERNEL_REL
> >bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
> >drivers/usb/core/hcd.c:#define KERNEL_VER
> >bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
> >drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_REL
> >bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
> >drivers/usb/gadget/udc/aspeed-vhub/hub.c:#define KERNEL_VER
> >bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
> >include/linux/usb/composite.h:  bcdDevice =
> >bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
> >include/linux/usb/composite.h:  bcdDevice |=
> >bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
> >kernel/sys.c:           v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;
> >
> >So, drivers/scsi/gdth.c file is missing in your list of file:
>
> Right. It's on it's way out: 0653c358d2dc ("scsi: Drop gdth driver").
>
> In theory I should have fixed it too, but given that this patch is
> cosmetic I just wanted to avoid unnecessary conflicts for SFR... If it
> doesn't end up being removed we can always fix it up there.
>
>

OK, Thanks - wasn't aware of that scsi/gdth removal.

Feel free to add my:

Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>

- Sedat -

Re: [PATCH] kbuild: simplify access to the kernel's version

[Sasha Levin]

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.

-- 
Thanks,
Sasha

Re: [PATCH] kbuild: simplify access to the kernel's version

[Masahiro Yamada]

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.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: simplify access to the kernel's version

[Masahiro Yamada]

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

Re: [PATCH] kbuild: simplify access to the kernel's version

[Masahiro Yamada]

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

Re: [PATCH] kbuild: simplify access to the kernel's version

[Sasha Levin]

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.

-- 
Thanks,
Sasha

Re: [PATCH] kbuild: simplify access to the kernel's version

[Christoph Hellwig]

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.

Re: [PATCH] kbuild: simplify access to the kernel's version

[Greg KH]

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>

[PATCH] kbuild: simplify access to the kernel's version

[Sasha Levin]

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(-)

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);
-- 
2.27.0