[LTP] [PATCH] close_range: check for kernel support below 5.9

[LTP] [PATCH] close_range: check for kernel support below 5.9

[Edward Liaw via ltp]

Check for close_range syscall support in the kernel before running
close_range01 and close_range02 tests.

Signed-off-by: Edward Liaw <edliaw@google.com>
---
 include/lapi/close_range.h                          | 13 +++++++++++++
 .../kernel/syscalls/close_range/close_range01.c     |  2 ++
 .../kernel/syscalls/close_range/close_range02.c     |  1 +
 3 files changed, 16 insertions(+)

diff --git a/include/lapi/close_range.h b/include/lapi/close_range.h
index 19db52d3d..0e464bb30 100644
--- a/include/lapi/close_range.h
+++ b/include/lapi/close_range.h
@@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
 	return tst_syscall(__NR_close_range, fd, max_fd, flags);
 }
 # endif
+
+static inline void close_range_supported_by_kernel(void)
+{
+	long ret;
+
+	if ((tst_kvercmp(5, 9, 0)) < 0) {
+		/* Check if the syscall is backported on an older kernel */
+		ret = syscall(__NR_close_range, 1, 0, 0);
+		if (ret == -1 && errno == ENOSYS)
+			tst_brk(TCONF, "Test not supported on kernel version < v5.9");
+	}
+}
+
 #endif	/* LAPI_CLOSE_RANGE_H__ */
diff --git a/testcases/kernel/syscalls/close_range/close_range01.c b/testcases/kernel/syscalls/close_range/close_range01.c
index 30bb600b6..072bbab66 100644
--- a/testcases/kernel/syscalls/close_range/close_range01.c
+++ b/testcases/kernel/syscalls/close_range/close_range01.c
@@ -53,6 +53,8 @@ static inline void do_close_range(unsigned int fd, unsigned int max_fd,
 
 static void setup(void)
 {
+	close_range_supported_by_kernel();
+
 	struct rlimit nfd;
 
 	SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
diff --git a/testcases/kernel/syscalls/close_range/close_range02.c b/testcases/kernel/syscalls/close_range/close_range02.c
index aec899261..2aa6d2c9f 100644
--- a/testcases/kernel/syscalls/close_range/close_range02.c
+++ b/testcases/kernel/syscalls/close_range/close_range02.c
@@ -111,4 +111,5 @@ static struct tst_test test = {
 	.tcnt = 6,
 	.forks_child = 1,
 	.test = run,
+	.setup = close_range_supported_by_kernel,
 };
-- 
2.39.1.456.gfc5497dd1b-goog


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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

Hi Edward,

good catch :).

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

...
> +++ b/include/lapi/close_range.h
> @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
>  	return tst_syscall(__NR_close_range, fd, max_fd, flags);
>  }
>  # endif
> +
> +static inline void close_range_supported_by_kernel(void)
> +{
> +	long ret;
> +
> +	if ((tst_kvercmp(5, 9, 0)) < 0) {
> +		/* Check if the syscall is backported on an older kernel */
BTW what particular use case this fixed? Is it backported to some android
kernel? Or to some enterprise distro? Because I don't think kernel stable trees
accept new functionality, just fixes.

Kind regards,
Petr

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Edward Liaw via ltp]

Hi Petr,

> > +++ b/include/lapi/close_range.h
> > @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
> >       return tst_syscall(__NR_close_range, fd, max_fd, flags);
> >  }
> >  # endif
> > +
> > +static inline void close_range_supported_by_kernel(void)
> > +{
> > +     long ret;
> > +
> > +     if ((tst_kvercmp(5, 9, 0)) < 0) {
> > +             /* Check if the syscall is backported on an older kernel */
> BTW what particular use case this fixed? Is it backported to some android
> kernel? Or to some enterprise distro? Because I don't think kernel stable trees
> accept new functionality, just fixes.

Oops, should I use .min_kver instead?  It isn't backported on Android;
I just wasn't sure what the right approach was.

Thanks,
Edward

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

Hi Edward,

> Hi Petr,

> > > +++ b/include/lapi/close_range.h
> > > @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
> > >       return tst_syscall(__NR_close_range, fd, max_fd, flags);
> > >  }
> > >  # endif
> > > +
> > > +static inline void close_range_supported_by_kernel(void)
> > > +{
> > > +     long ret;
> > > +
> > > +     if ((tst_kvercmp(5, 9, 0)) < 0) {
> > > +             /* Check if the syscall is backported on an older kernel */
> > BTW what particular use case this fixed? Is it backported to some android
> > kernel? Or to some enterprise distro? Because I don't think kernel stable trees
> > accept new functionality, just fixes.

> Oops, should I use .min_kver instead?  It isn't backported on Android;
> I just wasn't sure what the right approach was.

Actually, looking into SLES kernel sources, we backported close_range() to SLES
15-SP3 (bsc#1179090), which was 5.3.18 based. Thus you actually did good work :).

@Li, @Jan out of curiosity, was this backported to RHEL kernel as well?
It'd be useful to note that (one day we will be able to remove it once kernels
which backported are EOL).

Kind regards,
Petr

> Thanks,
> Edward

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Jan Stancek]

On Thu, Feb 2, 2023 at 8:50 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Edward,
>
> > Hi Petr,
>
> > > > +++ b/include/lapi/close_range.h
> > > > @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
> > > >       return tst_syscall(__NR_close_range, fd, max_fd, flags);
> > > >  }
> > > >  # endif
> > > > +
> > > > +static inline void close_range_supported_by_kernel(void)
> > > > +{
> > > > +     long ret;
> > > > +
> > > > +     if ((tst_kvercmp(5, 9, 0)) < 0) {
> > > > +             /* Check if the syscall is backported on an older kernel */
> > > BTW what particular use case this fixed? Is it backported to some android
> > > kernel? Or to some enterprise distro? Because I don't think kernel stable trees
> > > accept new functionality, just fixes.
>
> > Oops, should I use .min_kver instead?  It isn't backported on Android;
> > I just wasn't sure what the right approach was.
>
> Actually, looking into SLES kernel sources, we backported close_range() to SLES
> 15-SP3 (bsc#1179090), which was 5.3.18 based. Thus you actually did good work :).
>
> @Li, @Jan out of curiosity, was this backported to RHEL kernel as well?
> It'd be useful to note that (one day we will be able to remove it once kernels
> which backported are EOL).

Yes, it's present in 8.4 since kernel-4.18.0-290.el8.

>
> Kind regards,
> Petr
>
> > Thanks,
> > Edward
>


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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

> On Thu, Feb 2, 2023 at 8:50 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Edward,

> > > Hi Petr,

> > > > > +++ b/include/lapi/close_range.h
> > > > > @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
> > > > >       return tst_syscall(__NR_close_range, fd, max_fd, flags);
> > > > >  }
> > > > >  # endif
> > > > > +
> > > > > +static inline void close_range_supported_by_kernel(void)
> > > > > +{
> > > > > +     long ret;
> > > > > +
> > > > > +     if ((tst_kvercmp(5, 9, 0)) < 0) {
> > > > > +             /* Check if the syscall is backported on an older kernel */
> > > > BTW what particular use case this fixed? Is it backported to some android
> > > > kernel? Or to some enterprise distro? Because I don't think kernel stable trees
> > > > accept new functionality, just fixes.

> > > Oops, should I use .min_kver instead?  It isn't backported on Android;
> > > I just wasn't sure what the right approach was.

> > Actually, looking into SLES kernel sources, we backported close_range() to SLES
> > 15-SP3 (bsc#1179090), which was 5.3.18 based. Thus you actually did good work :).

> > @Li, @Jan out of curiosity, was this backported to RHEL kernel as well?
> > It'd be useful to note that (one day we will be able to remove it once kernels
> > which backported are EOL).

> Yes, it's present in 8.4 since kernel-4.18.0-290.el8.

Jan, thanks a lot!

@Edward we have Hackweek at SUSE, I'll merge this on Monday,
noting the reason why to keep this instead simple .min_kver.

Kind regards,
Petr

> > Kind regards,
> > Petr

> > > Thanks,
> > > Edward



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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Edward Liaw via ltp]

Oh, lucky coincidence I guess.  Thanks, Petr!

On Thu, Feb 2, 2023 at 12:20 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Thu, Feb 2, 2023 at 8:50 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Edward,
>
> > > > Hi Petr,
>
> > > > > > +++ b/include/lapi/close_range.h
> > > > > > @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned int max_fd,
> > > > > >       return tst_syscall(__NR_close_range, fd, max_fd, flags);
> > > > > >  }
> > > > > >  # endif
> > > > > > +
> > > > > > +static inline void close_range_supported_by_kernel(void)
> > > > > > +{
> > > > > > +     long ret;
> > > > > > +
> > > > > > +     if ((tst_kvercmp(5, 9, 0)) < 0) {
> > > > > > +             /* Check if the syscall is backported on an older kernel */
> > > > > BTW what particular use case this fixed? Is it backported to some android
> > > > > kernel? Or to some enterprise distro? Because I don't think kernel stable trees
> > > > > accept new functionality, just fixes.
>
> > > > Oops, should I use .min_kver instead?  It isn't backported on Android;
> > > > I just wasn't sure what the right approach was.
>
> > > Actually, looking into SLES kernel sources, we backported close_range() to SLES
> > > 15-SP3 (bsc#1179090), which was 5.3.18 based. Thus you actually did good work :).
>
> > > @Li, @Jan out of curiosity, was this backported to RHEL kernel as well?
> > > It'd be useful to note that (one day we will be able to remove it once kernels
> > > which backported are EOL).
>
> > Yes, it's present in 8.4 since kernel-4.18.0-290.el8.
>
> Jan, thanks a lot!
>
> @Edward we have Hackweek at SUSE, I'll merge this on Monday,
> noting the reason why to keep this instead simple .min_kver.
>
> Kind regards,
> Petr
>
> > > Kind regards,
> > > Petr
>
> > > > Thanks,
> > > > Edward
>
>

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

Hi all,

Edward, thanks for your work, merged.

Kind regards,
Petr

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

Hi,

hm, I was too fast to merge it. Looking at older result, 
It looks like this (merged as 7b5ee03899) was not needed,
because tst_syscall() properly detects missing support:

close_range01.c:134: TINFO: Plain close range
../../../../include/lapi/close_range.h:25: TCONF: syscall(436) __NR_close_range not supported on your arch

@Edward: Before we revert it, did you encounter some problem that it's really
needed? If yes, please share details.

Kind regards,
Petr

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Edward Liaw via ltp]

Hey Petr,

We turned on the HAVE_CLOSE_RANGE flag to test the bionic-defined
close_range on Android, but doing so bypasses tst_syscall in
include/lapi/close_range.h.  We don't currently have a way to
configure ltp differently across each kernel version, so I wanted to
use this check as a fallback to gate earlier kernels.

Thanks,
Edward


On Fri, Feb 3, 2023 at 2:25 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi,
>
> hm, I was too fast to merge it. Looking at older result,
> It looks like this (merged as 7b5ee03899) was not needed,
> because tst_syscall() properly detects missing support:
>
> close_range01.c:134: TINFO: Plain close range
> ../../../../include/lapi/close_range.h:25: TCONF: syscall(436) __NR_close_range not supported on your arch
>
> @Edward: Before we revert it, did you encounter some problem that it's really
> needed? If yes, please share details.
>
> Kind regards,
> Petr

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Petr Vorel]

Hi Edward,

> Hey Petr,

> We turned on the HAVE_CLOSE_RANGE flag to test the bionic-defined
> close_range on Android, but doing so bypasses tst_syscall in
> include/lapi/close_range.h.  We don't currently have a way to
> configure ltp differently across each kernel version, so I wanted to
> use this check as a fallback to gate earlier kernels.

Thanks for info. OK, at least it's needed :).
If these fixes would be needed for more syscalls, we might want to check if
there is a way to enhance tst_syscall() (likely there is no way).

Also (for next time) it's always safe to be more verbose for the reason in the
commit message to prevent cleanup of workarounds too early (before EOL of that
particular kernel).

Kind regards,
Petr

> Thanks,
> Edward


> On Fri, Feb 3, 2023 at 2:25 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi,

> > hm, I was too fast to merge it. Looking at older result,
> > It looks like this (merged as 7b5ee03899) was not needed,
> > because tst_syscall() properly detects missing support:

> > close_range01.c:134: TINFO: Plain close range
> > ../../../../include/lapi/close_range.h:25: TCONF: syscall(436) __NR_close_range not supported on your arch

> > @Edward: Before we revert it, did you encounter some problem that it's really
> > needed? If yes, please share details.

> > Kind regards,
> > Petr

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Edward Liaw via ltp]

Hi Petr,

Thanks, I will do that in the future.  This is the only time I've
encountered this so far but will look into doing that if there is
another.

Thanks,
Edward

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

Re: [LTP] [PATCH] close_range: check for kernel support below 5.9

[Li Wang]

Hi Edward, Petr,

Sorry for the late append reply.

On Tue, Jan 31, 2023 at 8:25 AM Edward Liaw via ltp <ltp@lists.linux.it>
wrote:

> Check for close_range syscall support in the kernel before running
> close_range01 and close_range02 tests.
>
> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
>  include/lapi/close_range.h                          | 13 +++++++++++++
>  .../kernel/syscalls/close_range/close_range01.c     |  2 ++
>  .../kernel/syscalls/close_range/close_range02.c     |  1 +
>  3 files changed, 16 insertions(+)
>
> diff --git a/include/lapi/close_range.h b/include/lapi/close_range.h
> index 19db52d3d..0e464bb30 100644
> --- a/include/lapi/close_range.h
> +++ b/include/lapi/close_range.h
> @@ -25,4 +25,17 @@ static inline int close_range(unsigned int fd, unsigned
> int max_fd,
>         return tst_syscall(__NR_close_range, fd, max_fd, flags);
>  }
>  # endif
> +
> +static inline void close_range_supported_by_kernel(void)
> +{
> +       long ret;
> +
> +       if ((tst_kvercmp(5, 9, 0)) < 0) {
>

Using tst_kvercmp together with ENOSYS check is a repetitive practice,
I think verifying the returned value and errno is quite enough to determine
an un-support syscall.

And there is possible a lite kernel version newer than 5.9 removes this
syscall support. Then this test will fail again because this tst_kvercmp
skips the ENOSYS check.

Otherwise:
Reviewed-by: Li Wang <liwang@redhat.com>



> +               /* Check if the syscall is backported on an older kernel */
> +               ret = syscall(__NR_close_range, 1, 0, 0);
> +               if (ret == -1 && errno == ENOSYS)
> +                       tst_brk(TCONF, "Test not supported on kernel
> version < v5.9");
> +       }
> +}
> +
>  #endif /* LAPI_CLOSE_RANGE_H__ */
> diff --git a/testcases/kernel/syscalls/close_range/close_range01.c
> b/testcases/kernel/syscalls/close_range/close_range01.c
> index 30bb600b6..072bbab66 100644
> --- a/testcases/kernel/syscalls/close_range/close_range01.c
> +++ b/testcases/kernel/syscalls/close_range/close_range01.c
> @@ -53,6 +53,8 @@ static inline void do_close_range(unsigned int fd,
> unsigned int max_fd,
>
>  static void setup(void)
>  {
> +       close_range_supported_by_kernel();
> +
>         struct rlimit nfd;
>
>         SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
> diff --git a/testcases/kernel/syscalls/close_range/close_range02.c
> b/testcases/kernel/syscalls/close_range/close_range02.c
> index aec899261..2aa6d2c9f 100644
> --- a/testcases/kernel/syscalls/close_range/close_range02.c
> +++ b/testcases/kernel/syscalls/close_range/close_range02.c
> @@ -111,4 +111,5 @@ static struct tst_test test = {
>         .tcnt = 6,
>         .forks_child = 1,
>         .test = run,
> +       .setup = close_range_supported_by_kernel,
>  };
> --
> 2.39.1.456.gfc5497dd1b-goog
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

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