diff mbox series

pipe/pipe15.c: Adjust fd check for pipe creation

Message ID EF89BFA1-4088-4497-B0C3-788743AAED3C@baidu.com
State Accepted
Headers show
Series pipe/pipe15.c: Adjust fd check for pipe creation | expand

Commit Message

Wenjie Xu March 1, 2024, 2:12 a.m. UTC
A pipe occupies 2 fds, and considering 3 standard fds,
we should compare rlim_max with such *2+3 calculated value
to verify whether the maximum file descriptor configuration
of the current machine is sufficient.

Signed-off-by: Wenjie Xu <xuwenjie04@baidu.com>
---
testcases/kernel/syscalls/pipe/pipe15.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.41.0

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

Comments

Petr Vorel March 1, 2024, 12:41 p.m. UTC | #1
HI Wenjie, Marius,

> A pipe occupies 2 fds, and considering 3 standard fds,
> we should compare rlim_max with such *2+3 calculated value
> to verify whether the maximum file descriptor configuration
> of the current machine is sufficient.

Indeed, 1024*2+3 is the lowest number which passes with non-default ulimit:

ulimit -n $((1024*2+3)) && ./pipe15

Therefore I merged, thank you!

BTW I wonder how did you encounter this?

@Marius FYI I also fixed SIGSEGV when low ulimit, see
https://github.com/linux-test-project/ltp/commit/fc6adb8454df34fa87b462844b740cc3a0b84caa

> Signed-off-by: Wenjie Xu <xuwenjie04@baidu.com>
> ---
> testcases/kernel/syscalls/pipe/pipe15.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/testcases/kernel/syscalls/pipe/pipe15.c b/testcases/kernel/syscalls/pipe/pipe15.c
> index c85ad1820..9e02fe2eb 100644
> --- a/testcases/kernel/syscalls/pipe/pipe15.c
> +++ b/testcases/kernel/syscalls/pipe/pipe15.c
> @@ -59,7 +59,7 @@ static void setup(void)
>       tst_res(TINFO, "Creating %i pipes", pipe_count);

>       SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
> -     if (nfd.rlim_max < (unsigned long)pipe_count)
> +    if (nfd.rlim_max < (unsigned long)pipe_count * 2 + 3)
nit: you mixed tabs with spaces or broke indent. And patch was not applicable
(even before my changes), therefore I needed to do the changes myself and thus I
put my fix fc6adb845 ("pipe15: Avoid SIGSEGV in cleanup") before.

Kind regards,
Petr

>              tst_brk(TCONF, "NOFILE limit max too low: %lu < %i", nfd.rlim_max, pipe_count);
>       if (nfd.rlim_cur < nfd.rlim_max) {
>              nfd.rlim_cur = nfd.rlim_max;
Wenjie Xu March 5, 2024, 7:20 a.m. UTC | #2
> HI Wenjie, Marius,
>
> > A pipe occupies 2 fds, and considering 3 standard fds,
> > we should compare rlim_max with such *2+3 calculated value
> > to verify whether the maximum file descriptor configuration
> > of the current machine is sufficient.
> 
> Indeed, 1024*2+3 is the lowest number which passes with non-default ulimit:
> 
> ulimit -n $((1024*2+3)) && ./pipe15
> 
> Therefore I merged, thank you!
> 
> BTW I wonder how did you encounter this?
 
In my test case, the system ulimit is set to 10240,
and the calculated pipe_count is also 10240, causing
the EMFILE failure when creating the pipe below.

> @Marius FYI I also fixed SIGSEGV when low ulimit, see
> https://github.com/linux-test-project/ltp/commit/fc6adb8454df34fa87b462844b740cc3a0b84caa
>
> > Signed-off-by: Wenjie Xu <xuwenjie04@baidu.com>
> > ---
> > testcases/kernel/syscalls/pipe/pipe15.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/testcases/kernel/syscalls/pipe/pipe15.c b/testcases/kernel/syscalls/pipe/pipe15.c
> > index c85ad1820..9e02fe2eb 100644
> > --- a/testcases/kernel/syscalls/pipe/pipe15.c
> > +++ b/testcases/kernel/syscalls/pipe/pipe15.c
> > @@ -59,7 +59,7 @@ static void setup(void)
> >       tst_res(TINFO, "Creating %i pipes", pipe_count);
> 
> >       SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
> > -     if (nfd.rlim_max < (unsigned long)pipe_count)
> > +    if (nfd.rlim_max < (unsigned long)pipe_count * 2 + 3)
> nit: you mixed tabs with spaces or broke indent. And patch was not applicable
> (even before my changes), therefore I needed to do the changes myself and thus I
> put my fix fc6adb845 ("pipe15: Avoid SIGSEGV in cleanup") before.
> 
> Kind regards,
> Petr
>
> >              tst_brk(TCONF, "NOFILE limit max too low: %lu < %i", nfd.rlim_max, pipe_count);
> >       if (nfd.rlim_cur < nfd.rlim_max) {
> >              nfd.rlim_cur = nfd.rlim_max;
Petr Vorel March 6, 2024, 12:46 p.m. UTC | #3
> > HI Wenjie, Marius,

> > > A pipe occupies 2 fds, and considering 3 standard fds,
> > > we should compare rlim_max with such *2+3 calculated value
> > > to verify whether the maximum file descriptor configuration
> > > of the current machine is sufficient.

> > Indeed, 1024*2+3 is the lowest number which passes with non-default ulimit:

> > ulimit -n $((1024*2+3)) && ./pipe15

> > Therefore I merged, thank you!

> > BTW I wonder how did you encounter this?

> In my test case, the system ulimit is set to 10240,
> and the calculated pipe_count is also 10240, causing
> the EMFILE failure when creating the pipe below.

+1, thanks for info.
Is it some embedded distro or a regular widely used distro?
I'm just curious what kernels are tested by LTP and whether where the
non-default setup comes from.

Kind regards,
Petr
Wenjie Xu March 7, 2024, 2:55 a.m. UTC | #4
> > > HI Wenjie, Marius,

> > > > A pipe occupies 2 fds, and considering 3 standard fds,
> > > > we should compare rlim_max with such *2+3 calculated value
> > > > to verify whether the maximum file descriptor configuration
> > > > of the current machine is sufficient.

> > > Indeed, 1024*2+3 is the lowest number which passes with non-default ulimit:

> > > ulimit -n $((1024*2+3)) && ./pipe15

> > > Therefore I merged, thank you!

> > > BTW I wonder how did you encounter this?

> > In my test case, the system ulimit is set to 10240,
> > and the calculated pipe_count is also 10240, causing
> > the EMFILE failure when creating the pipe below.

> +1, thanks for info.
> Is it some embedded distro or a regular widely used distro?
> I'm just curious what kernels are tested by LTP and whether where the
> non-default setup comes from.

> Kind regards,
> Petr

This may come from the modification of the kernel and image 
we maintain ourselves. The basic image is from CentOS 7.6.1810,
and the kernel based on version 5.10.

Regards,
Xu Wenjie
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/pipe/pipe15.c b/testcases/kernel/syscalls/pipe/pipe15.c
index c85ad1820..9e02fe2eb 100644
--- a/testcases/kernel/syscalls/pipe/pipe15.c
+++ b/testcases/kernel/syscalls/pipe/pipe15.c
@@ -59,7 +59,7 @@  static void setup(void)
      tst_res(TINFO, "Creating %i pipes", pipe_count);

      SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
-     if (nfd.rlim_max < (unsigned long)pipe_count)
+    if (nfd.rlim_max < (unsigned long)pipe_count * 2 + 3)
             tst_brk(TCONF, "NOFILE limit max too low: %lu < %i", nfd.rlim_max, pipe_count);
      if (nfd.rlim_cur < nfd.rlim_max) {
             nfd.rlim_cur = nfd.rlim_max;