Re: [LTP] [PATCH v2 2/5] lib: Add support for max_kver to struct tst_test and tst_fs

Hi Avinesh,

> Hi Petr,

> few comments below
Thanks!

> On 7/29/26 11:57 AM, Petr Vorel wrote:
> > Signed-off-by: Petr Vorel <[email protected]>
> > ---
> > The same as v1.

> >   include/tst_test.h |  9 ++++++++-
> >   lib/tst_test.c     | 39 +++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 47 insertions(+), 1 deletion(-)

> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index c69362485e..0910660827 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -280,6 +280,9 @@ struct tst_ulimit_val {
> >    *
> >    * @min_kver: A minimum kernel version supporting the filesystem which has been
> >    *            created with mkfs.
> > + *
> > + * @max_kver: A maximum kernel version supporting the filesystem which has been
> > + *            created with mkfs.
> >    */
> >   struct tst_fs {
> >   	const char *type;
> > @@ -292,6 +295,7 @@ struct tst_fs {
> >   	const void *mnt_data;
> >   	const char *min_kver;
> > +	const char *max_kver;
> >   };
> >   /**
> > @@ -301,7 +305,9 @@ struct tst_fs {
> >    *        and each time passed an increasing counter value.
> >    * @options: An NULL optstr terminated array of struct tst_option.
> >    *
> > - * @min_kver: A minimal kernel version the test can run on. e.g. "3.10".
> > + * @min_kver: A minimal kernel version the test can run on. e.g. "4.4".
> s/minimal/minimum

You spotted more grammar errors so I trust you :). But can you please double
check that it'đ really correct? Minimal is an adjective, minimum is an adjective
but also a noun and an adverb.

When googling I even found use of both in the same document :)
https://kubernetes.io/docs/reference/node/kernel-version-requirements/
"have minimum kernel version requirements"
"minimal kernel version requirement"

> > + *
> > + * @max_kver: A maximal kernel version the test can run on. e.g. "7.2".
> s/maximal/maximum
dtto

> Also, I assume test will *not be skipped* on kernel versions
> 7.1.1, 7.1.2 etc just with
> .max_kver="7.1"

Good point, thanks => v3 (sigh).

check_max_kver() needs to have this condition to take an account the sublevel.

-if (tst_kvercmp(v1, v2, v3) > 0) {
+if (tst_kvercmp(v1, v2, v3) >= 1024) {
		msg = "The test requires kernel %s or older";
...

Sublevel can be above 255 since 4.4.256
https://lwn.net/ml/linux-kernel/[email protected]/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9b82f13e7ef3

But it should be below 1024 (practically the highest sublevel has been 337).

Kind regards,
Petr

> maybe we should clarify.

> >    *
> >    * @supported_archs: A NULL terminated array of architectures the test runs on
> >    *                   e.g. {"x86_64, "x86", NULL}. Calls tst_is_on_arch() to
> > @@ -551,6 +557,7 @@ struct tst_fs {
> >   	struct tst_option *options;
> >   	const char *min_kver;
> > +	const char *max_kver;
> >   	const char *const *supported_archs;
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 778a1fed40..fa38b3a7a3 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1087,6 +1087,36 @@ static bool check_min_kver(const char *min_kver, const int brk_nosupp)
> >   	return true;
> >   }
> > +/*
> > + * Check for the maximal required kernel version.
> > + *
> > + * return: true if the kernel version is low enough, false otherwise.

> This sounds a bit confusing. How about-

> Check the running kernel against the last version the test applies to.

Makes sense. Based on check_min_kver(), which was renamed from check_kver() in
the previous commit. Could you have look on these as well. Anyway, if you have
time, having a look on docs wording as a separate effort would help.

> return: true if the kernel is old enough, false otherwise.

+1

> > + */
> > +static bool check_max_kver(const char *max_kver, const int brk_nosupp)
> > +{
> > +	char *msg;
> > +	int v1, v2, v3;
> > +
> > +	if (tst_parse_kver(max_kver, &v1, &v2, &v3)) {
> > +		tst_res(TWARN,
> > +			"Invalid kernel version %s, expected %%d.%%d.%%d",
> I wouldn't say 'kernel version is invalid'. maybe test isn't
> supported/applicable on..

tst_parse_kver() returns 1 on error => this is really an error check.
Other thing is that the same error handling is on both places, but that's very
minor.

Kind regards,
Petr
...

> Thanks,
> Avinesh

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