| Message ID | 20250314044257.1673303-1-lufei@uniontech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | unshare03: using soft limit of NOFILE | expand |
Hi lufei, Al, @Al, you're the author of the original test unshare_test.c [1] in kselftest. This is a patch to LTP test unshare03.c, which is based on your test. > I think it's safer to set NOFILE increasing from soft limit than from > hard limit. > Hard limit may lead to dup2 ENOMEM error which bring the result to > TBROK on little memory machine. (e.g. 2GB memory in my situation, hard > limit in /proc/sys/fs/nr_open come out to be 1073741816) IMHO lowering number to ~ half (in my case) by using rlimit.rlim_max instead of /proc/sys/fs/nr_open should not affect the functionality of the test, right? Or am I missing something obvious? @lufei I guess kselftest tools/testing/selftests/core/unshare_test.c would fail for you as well, right? Kind regards, Petr [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=611fbeb44a777e5ab54ab3127ec85f72147911d8 > Signed-off-by: lufei <lufei@uniontech.com> > --- > testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c > index 7c5e71c4e..bb568264c 100644 > --- a/testcases/kernel/syscalls/unshare/unshare03.c > +++ b/testcases/kernel/syscalls/unshare/unshare03.c > @@ -24,7 +24,7 @@ > static void run(void) > { > - int nr_open; > + int rlim_max; > int nr_limit; > struct rlimit rlimit; > struct tst_clone_args args = { > @@ -32,14 +32,12 @@ static void run(void) > .exit_signal = SIGCHLD, > }; > - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); > - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > + rlim_max = rlimit.rlim_max; > - nr_limit = nr_open + NR_OPEN_LIMIT; > + nr_limit = rlim_max + NR_OPEN_LIMIT; > SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); > - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > - > rlimit.rlim_cur = nr_limit; > rlimit.rlim_max = nr_limit; > @@ -47,10 +45,10 @@ static void run(void) > tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", > nr_limit); > - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); > + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); > if (!SAFE_CLONE(&args)) { > - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); > + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); > TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); > exit(0); > }
Hi lufei, Al, > Hi Petr, > Yes, kselftest tools/testing/selftests/core/unshare_test.c failed as > well, dup2 failed: > ``` > unshare_test.c:60:unshare_EMFILE:Expected dup2(2, nr_open + 64) (-1) >= 0 (0) > ``` Thanks for info. Maybe also sending a patch to kselftest? Kind regards, Petr PS: lufei, please keep Cc to keep other informed. > Thanks for reply. > Best regards. > On Thu, Mar 27, 2025 at 11:33:36AM +0100, Petr Vorel wrote: > > Hi lufei, Al, > > @Al, you're the author of the original test unshare_test.c [1] in kselftest. > > This is a patch to LTP test unshare03.c, which is based on your test. > > > I think it's safer to set NOFILE increasing from soft limit than from > > > hard limit. > > > Hard limit may lead to dup2 ENOMEM error which bring the result to > > > TBROK on little memory machine. (e.g. 2GB memory in my situation, hard > > > limit in /proc/sys/fs/nr_open come out to be 1073741816) > > IMHO lowering number to ~ half (in my case) by using rlimit.rlim_max instead of > > /proc/sys/fs/nr_open should not affect the functionality of the test, right? > > Or am I missing something obvious? > > @lufei I guess kselftest tools/testing/selftests/core/unshare_test.c would fail > > for you as well, right? > > Kind regards, > > Petr > > [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=611fbeb44a777e5ab54ab3127ec85f72147911d8 > > > Signed-off-by: lufei <lufei@uniontech.com> > > > --- > > > testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c > > > index 7c5e71c4e..bb568264c 100644 > > > --- a/testcases/kernel/syscalls/unshare/unshare03.c > > > +++ b/testcases/kernel/syscalls/unshare/unshare03.c > > > @@ -24,7 +24,7 @@ > > > static void run(void) > > > { > > > - int nr_open; > > > + int rlim_max; > > > int nr_limit; > > > struct rlimit rlimit; > > > struct tst_clone_args args = { > > > @@ -32,14 +32,12 @@ static void run(void) > > > .exit_signal = SIGCHLD, > > > }; > > > - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); > > > - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); > > > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > > + rlim_max = rlimit.rlim_max; > > > - nr_limit = nr_open + NR_OPEN_LIMIT; > > > + nr_limit = rlim_max + NR_OPEN_LIMIT; > > > SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); > > > - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > > - > > > rlimit.rlim_cur = nr_limit; > > > rlimit.rlim_max = nr_limit; > > > @@ -47,10 +45,10 @@ static void run(void) > > > tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", > > > nr_limit); > > > - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); > > > + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); > > > if (!SAFE_CLONE(&args)) { > > > - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); > > > + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); > > > TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); > > > exit(0); > > > }
Hi! > I think it's safer to set NOFILE increasing from soft limit than from > hard limit. > > Hard limit may lead to dup2 ENOMEM error which bring the result to > TBROK on little memory machine. (e.g. 2GB memory in my situation, hard > limit in /proc/sys/fs/nr_open come out to be 1073741816) > > Signed-off-by: lufei <lufei@uniontech.com> > --- > testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c > index 7c5e71c4e..bb568264c 100644 > --- a/testcases/kernel/syscalls/unshare/unshare03.c > +++ b/testcases/kernel/syscalls/unshare/unshare03.c > @@ -24,7 +24,7 @@ > > static void run(void) > { > - int nr_open; > + int rlim_max; > int nr_limit; > struct rlimit rlimit; > struct tst_clone_args args = { > @@ -32,14 +32,12 @@ static void run(void) > .exit_signal = SIGCHLD, > }; > > - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); > - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > + rlim_max = rlimit.rlim_max; > > - nr_limit = nr_open + NR_OPEN_LIMIT; > + nr_limit = rlim_max + NR_OPEN_LIMIT; > SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); > > - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > - > rlimit.rlim_cur = nr_limit; > rlimit.rlim_max = nr_limit; > > @@ -47,10 +45,10 @@ static void run(void) > tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", > nr_limit); > > - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); > + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); > > if (!SAFE_CLONE(&args)) { > - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); > + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); > TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); > exit(0); > } Why do we bother with reading the /rpoc/sys/fs/nr_open file? All that we need to to do is to dup() a file descriptor and tnen set the nr_open limit to fd - 2. And if we do so we can drop the rlimit that increases the limit so that it's greater than nr_open as well.
> Hi! > > I think it's safer to set NOFILE increasing from soft limit than from > > hard limit. > > Hard limit may lead to dup2 ENOMEM error which bring the result to > > TBROK on little memory machine. (e.g. 2GB memory in my situation, hard > > limit in /proc/sys/fs/nr_open come out to be 1073741816) > > Signed-off-by: lufei <lufei@uniontech.com> > > --- > > testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c > > index 7c5e71c4e..bb568264c 100644 > > --- a/testcases/kernel/syscalls/unshare/unshare03.c > > +++ b/testcases/kernel/syscalls/unshare/unshare03.c > > @@ -24,7 +24,7 @@ > > static void run(void) > > { > > - int nr_open; > > + int rlim_max; > > int nr_limit; > > struct rlimit rlimit; > > struct tst_clone_args args = { > > @@ -32,14 +32,12 @@ static void run(void) > > .exit_signal = SIGCHLD, > > }; > > - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); > > - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); > > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > + rlim_max = rlimit.rlim_max; > > - nr_limit = nr_open + NR_OPEN_LIMIT; > > + nr_limit = rlim_max + NR_OPEN_LIMIT; > > SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); > > - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > - > > rlimit.rlim_cur = nr_limit; > > rlimit.rlim_max = nr_limit; > > @@ -47,10 +45,10 @@ static void run(void) > > tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", > > nr_limit); > > - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); > > + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); > > if (!SAFE_CLONE(&args)) { > > - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); > > + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); > > TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); > > exit(0); > > } > Why do we bother with reading the /rpoc/sys/fs/nr_open file? All that we > need to to do is to dup() a file descriptor and tnen set the nr_open > limit to fd - 2. And if we do so we can drop the rlimit that increases > the limit so that it's greater than nr_open as well. IMHO file descriptor will be 3, fd - 2 == 1. And trying to set 1 to /rpoc/sys/fs/nr_open leads to EINVAL. You probably mean something different. The test is based on unshare_test.c [1] from Al Viro (611fbeb44a777 [2]). Both tests IMHO use nr_open + 1024 nr_open + 1024 and then call dup2() on nr_open + 64 to avoid later EINVAL when writing /rpoc/sys/fs/nr_open. Kind regards, Petr [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/core/unshare_test.c [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=611fbeb44a777e5ab54ab3127ec85f72147911d8
Hi! > IMHO file descriptor will be 3, fd - 2 == 1. And trying to set 1 to > /rpoc/sys/fs/nr_open leads to EINVAL. You probably mean something different. Ah, we cannot set nr_open to anything that is smaller than BITS_PER_LONG: ... unsigned int sysctl_nr_open __read_mostly = 1024*1024; unsigned int sysctl_nr_open_min = BITS_PER_LONG; /* our min() is unusable in constant expressions ;-/ */ #define __const_min(x, y) ((x) < (y) ? (x) : (y)) unsigned int sysctl_nr_open_max = __const_min(INT_MAX, ~(size_t)0/sizeof(void *)) & -BITS_PER_LONG; ... Then we need a filedescriptor that is >= 64 and set the nr_open to 64.
Hi, Cyril Thanks very much for the smart suggestion. I'll submit a v2 patch. On Tue, Apr 01, 2025 at 12:11:21PM +0200, Cyril Hrubis wrote: > Hi! > > I think it's safer to set NOFILE increasing from soft limit than from > > hard limit. > > > > Hard limit may lead to dup2 ENOMEM error which bring the result to > > TBROK on little memory machine. (e.g. 2GB memory in my situation, hard > > limit in /proc/sys/fs/nr_open come out to be 1073741816) > > > > Signed-off-by: lufei <lufei@uniontech.com> > > --- > > testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c > > index 7c5e71c4e..bb568264c 100644 > > --- a/testcases/kernel/syscalls/unshare/unshare03.c > > +++ b/testcases/kernel/syscalls/unshare/unshare03.c > > @@ -24,7 +24,7 @@ > > > > static void run(void) > > { > > - int nr_open; > > + int rlim_max; > > int nr_limit; > > struct rlimit rlimit; > > struct tst_clone_args args = { > > @@ -32,14 +32,12 @@ static void run(void) > > .exit_signal = SIGCHLD, > > }; > > > > - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); > > - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); > > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > + rlim_max = rlimit.rlim_max; > > > > - nr_limit = nr_open + NR_OPEN_LIMIT; > > + nr_limit = rlim_max + NR_OPEN_LIMIT; > > SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); > > > > - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); > > - > > rlimit.rlim_cur = nr_limit; > > rlimit.rlim_max = nr_limit; > > > > @@ -47,10 +45,10 @@ static void run(void) > > tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", > > nr_limit); > > > > - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); > > + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); > > > > if (!SAFE_CLONE(&args)) { > > - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); > > + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); > > TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); > > exit(0); > > } > > Why do we bother with reading the /rpoc/sys/fs/nr_open file? All that we > need to to do is to dup() a file descriptor and tnen set the nr_open > limit to fd - 2. And if we do so we can drop the rlimit that increases > the limit so that it's greater than nr_open as well. > > -- > Cyril Hrubis > chrubis@suse.cz >
diff --git a/testcases/kernel/syscalls/unshare/unshare03.c b/testcases/kernel/syscalls/unshare/unshare03.c index 7c5e71c4e..bb568264c 100644 --- a/testcases/kernel/syscalls/unshare/unshare03.c +++ b/testcases/kernel/syscalls/unshare/unshare03.c @@ -24,7 +24,7 @@ static void run(void) { - int nr_open; + int rlim_max; int nr_limit; struct rlimit rlimit; struct tst_clone_args args = { @@ -32,14 +32,12 @@ static void run(void) .exit_signal = SIGCHLD, }; - SAFE_FILE_SCANF(FS_NR_OPEN, "%d", &nr_open); - tst_res(TDEBUG, "Maximum number of file descriptors: %d", nr_open); + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); + rlim_max = rlimit.rlim_max; - nr_limit = nr_open + NR_OPEN_LIMIT; + nr_limit = rlim_max + NR_OPEN_LIMIT; SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_limit); - SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlimit); - rlimit.rlim_cur = nr_limit; rlimit.rlim_max = nr_limit; @@ -47,10 +45,10 @@ static void run(void) tst_res(TDEBUG, "Set new maximum number of file descriptors to : %d", nr_limit); - SAFE_DUP2(2, nr_open + NR_OPEN_DUP); + SAFE_DUP2(2, rlim_max + NR_OPEN_DUP); if (!SAFE_CLONE(&args)) { - SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", nr_open); + SAFE_FILE_PRINTF(FS_NR_OPEN, "%d", rlim_max); TST_EXP_FAIL(unshare(CLONE_FILES), EMFILE); exit(0); }
I think it's safer to set NOFILE increasing from soft limit than from hard limit. Hard limit may lead to dup2 ENOMEM error which bring the result to TBROK on little memory machine. (e.g. 2GB memory in my situation, hard limit in /proc/sys/fs/nr_open come out to be 1073741816) Signed-off-by: lufei <lufei@uniontech.com> --- testcases/kernel/syscalls/unshare/unshare03.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)