| Message ID | 20231018015016.1897021-1-edliaw@google.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tst_kvercmp: Handle larger kernel version numbers | expand |
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
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; > }
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; > > } >
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.
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
Hi Edward, merged, thanks! Kind regards, Petr
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; }
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(-)