[LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Edward Liaw via ltp]

Current implementation can only handle revision numbers up to 256.  Bump
this up to 1024 as some revision numbers are in the 300s.

Signed-off-by: Edward Liaw <edliaw@google.com>
---
 lib/tst_kvercmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
index 552920fac..9e1a511af 100644
--- a/lib/tst_kvercmp.c
+++ b/lib/tst_kvercmp.c
@@ -92,8 +92,8 @@ int tst_kvcmp(const char *cur_kver, int r1, int r2, int r3)
 		         cur_kver);
 	}
 
-	testver = (r1 << 16) + (r2 << 8) + r3;
-	currver = (a1 << 16) + (a2 << 8) + a3;
+	testver = (r1 << 20) + (r2 << 10) + r3;
+	currver = (a1 << 20) + (a2 << 10) + a3;
 
 	return currver - testver;
 }
-- 
2.42.0.655.g421f12c284-goog


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Petr Vorel]

Hi Edward,

> Current implementation can only handle revision numbers up to 256.  Bump
> this up to 1024 as some revision numbers are in the 300s.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks!

FYI I'm waiting for CI fixes before merging this.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Petr Vorel]

Hi Edward,

> Current implementation can only handle revision numbers up to 256.  Bump
> this up to 1024 as some revision numbers are in the 300s.

> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
>  lib/tst_kvercmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
> index 552920fac..9e1a511af 100644
> --- a/lib/tst_kvercmp.c
> +++ b/lib/tst_kvercmp.c
> @@ -92,8 +92,8 @@ int tst_kvcmp(const char *cur_kver, int r1, int r2, int r3)
>  		         cur_kver);
>  	}

> -	testver = (r1 << 16) + (r2 << 8) + r3;
> -	currver = (a1 << 16) + (a2 << 8) + a3;
> +	testver = (r1 << 20) + (r2 << 10) + r3;
> +	currver = (a1 << 20) + (a2 << 10) + a3;

I wonder why you need this change. Number > 256 is actually only the third
number, but we haven't checked that so far. Do you plan to use it actually?

We try to detect functional changes to avoid problems to compare about mainline
and *also* with stable/LTS kernels.

NOTE: checking for 3rd number actually does not work for Debian, which reports
`uname -r` as: 5.10.0-8-amd64 (the real version is only as an comment in
/boot/config-*). If something specific in stable would be needed, it would have
to use tst_kvercmp2() (e.g. utimensat01.c for Ubuntu). Maybe you have a similar
problem in AOSP kernel.

Kind regards,
Petr

>  	return currver - testver;
>  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Edward Liaw via ltp]

On Tue, Oct 24, 2023, 4:14 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Edward,
>
> > Current implementation can only handle revision numbers up to 256.  Bump
> > this up to 1024 as some revision numbers are in the 300s.
>
> > Signed-off-by: Edward Liaw <edliaw@google.com>
> > ---
> >  lib/tst_kvercmp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> > diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
> > index 552920fac..9e1a511af 100644
> > --- a/lib/tst_kvercmp.c
> > +++ b/lib/tst_kvercmp.c
> > @@ -92,8 +92,8 @@ int tst_kvcmp(const char *cur_kver, int r1, int r2,
> int r3)
> >                        cur_kver);
> >       }
>
> > -     testver = (r1 << 16) + (r2 << 8) + r3;
> > -     currver = (a1 << 16) + (a2 << 8) + a3;
> > +     testver = (r1 << 20) + (r2 << 10) + r3;
> > +     currver = (a1 << 20) + (a2 << 10) + a3;
>
> I wonder why you need this change. Number > 256 is actually only the third
> number, but we haven't checked that so far. Do you plan to use it actually?
>

It was causing the minor rev of the kernel I was running to overflow into
the major rev, so a test that had a min kver of 4.19 was running and
failing on a 4.14.327 kernel.

We try to detect functional changes to avoid problems to compare about
> mainline
> and *also* with stable/LTS kernels.
>
> NOTE: checking for 3rd number actually does not work for Debian, which
> reports
> `uname -r` as: 5.10.0-8-amd64 (the real version is only as an comment in
> /boot/config-*). If something specific in stable would be needed, it would
> have
> to use tst_kvercmp2() (e.g. utimensat01.c for Ubuntu). Maybe you have a
> similar
> problem in AOSP kernel.


> Kind regards,
> Petr
>
> >       return currver - testver;
> >  }
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Edward Liaw via ltp]

Hi Petr,

On Tue, Oct 24, 2023 at 5:46 PM Edward Liaw <edliaw@google.com> wrote:
>
> On Tue, Oct 24, 2023, 4:14 PM Petr Vorel <pvorel@suse.cz> wrote:
>>
>> Hi Edward,
>>
>> > Current implementation can only handle revision numbers up to 256.  Bump
>> > this up to 1024 as some revision numbers are in the 300s.
>>
>> > Signed-off-by: Edward Liaw <edliaw@google.com>
>> > ---
>> >  lib/tst_kvercmp.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> > diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
>> > index 552920fac..9e1a511af 100644
>> > --- a/lib/tst_kvercmp.c
>> > +++ b/lib/tst_kvercmp.c
>> > @@ -92,8 +92,8 @@ int tst_kvcmp(const char *cur_kver, int r1, int r2, int r3)
>> >                        cur_kver);
>> >       }
>>
>> > -     testver = (r1 << 16) + (r2 << 8) + r3;
>> > -     currver = (a1 << 16) + (a2 << 8) + a3;
>> > +     testver = (r1 << 20) + (r2 << 10) + r3;
>> > +     currver = (a1 << 20) + (a2 << 10) + a3;
>>
>> I wonder why you need this change. Number > 256 is actually only the third
>> number, but we haven't checked that so far. Do you plan to use it actually?
>
>
> It was causing the minor rev of the kernel I was running to overflow into the major rev, so a test that had a min kver of 4.19 was running and failing on a 4.14.327 kernel.

I remembered it incorrectly.  It was the mmap20 test with min_kver
4.15 that was failing on a 4.14.327 kernel.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Petr Vorel]

Hi Edward,

...
> >> I wonder why you need this change. Number > 256 is actually only the third
> >> number, but we haven't checked that so far. Do you plan to use it actually?


> > It was causing the minor rev of the kernel I was running to overflow into the major rev, so a test that had a min kver of 4.19 was running and failing on a 4.14.327 kernel.

> I remembered it incorrectly.  It was the mmap20 test with min_kver
> 4.15 that was failing on a 4.14.327 kernel.

Good to know, thank for info. It will be merged during this week,
once we solve CI issue (should be fairly soon).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_kvercmp: Handle larger kernel version numbers

[Petr Vorel]

Hi Edward,

merged, thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp