| Message ID | 20230911094043.25511-1-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] Refactor getdtablesize01 using new LTP API | expand |
Hello, Andrea Cervesato <andrea.cervesato@suse.de> writes: > From: Andrea Cervesato <andrea.cervesato@suse.com> > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > .../syscalls/getdtablesize/getdtablesize01.c | 146 +++++++----------- > 1 file changed, 52 insertions(+), 94 deletions(-) > > diff --git a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c > index d25cac261..80321e24f 100644 > --- a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c > +++ b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c > @@ -1,119 +1,77 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) International Business Machines Corp., 2005 > * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of version 2 of the GNU General Public License as > - * published by the Free Software Foundation. > - * > - * This program is distributed in the hope that it would be useful, but > - * WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > - * > - * You should have received a copy of the GNU General Public License along > - * with this program; if not, write the Free Software Foundation, Inc., > - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > - * > + * Prashant P Yendigeri <prashant.yendigeri@wipro.com> > + * Robbie Williamson <robbiew@us.ibm.com> > + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> > */ > -/********************************************************** > - * > - * TEST IDENTIFIER : getdtablesize01 > - * > - * EXECUTED BY : root / superuser > - * > - * TEST TITLE : Basic tests for getdtablesize01(2) > - * > - * TEST CASE TOTAL : 1 > - * > - * AUTHOR : Prashant P Yendigeri > - * <prashant.yendigeri@wipro.com> > - * Robbie Williamson > - * <robbiew@us.ibm.com> > - * > - * DESCRIPTION > - * This is a Phase I test for the getdtablesize01(2) system call. > - * It is intended to provide a limited exposure of the system > call. It seems this test has been broken for a long time unless I am mistaken. glibc does not call the actual syscall, it calls prlimit. Which has different behaviour (see below). > - * > - **********************************************************/ > > -#include <stdio.h> > -#include <errno.h> > -#include <sys/types.h> > -#include <sys/stat.h> > -#include <fcntl.h> > -#include <sys/time.h> > -#include <sys/resource.h> > -#include <unistd.h> > -#include "test.h" > +/*\ > + * [Description] > + * > + * This test verifies that getdtablesize() syscall returns the right max number > + * of files which can be opened by a process. We test this in 2 ways: firstly by > + * comparing its return value with RLIMIT_NOFILE, secondly by opening as many > + * files as possible and then checking when open() raises EMFILE. > + */ > > -void setup(); > -void cleanup(); > +#include <stdlib.h> > +#include "tst_test.h" > > -char *TCID = "getdtablesize01"; > -int TST_TOTAL = 1; > +#define FILENAME "rofile" > > -int main(void) > +static void run(void) > { > - int table_size, fd = 0, count = 0; > - int max_val_opfiles; > + int *fds; > + int overfd; > + int open_files; > struct rlimit rlp; > > - setup(); > - table_size = getdtablesize(); > - getrlimit(RLIMIT_NOFILE, &rlp); > - max_val_opfiles = (rlim_t) rlp.rlim_cur; > - > - tst_resm(TINFO, > - "Maximum number of files a process can have opened is %d", > - table_size); > - tst_resm(TINFO, > - "Checking with the value returned by getrlimit...RLIMIT_NOFILE"); > - > - if (table_size == max_val_opfiles) > - tst_resm(TPASS, "got correct dtablesize, value is %d", > - max_val_opfiles); > - else { > - tst_resm(TFAIL, "got incorrect table size, value is %d", > - max_val_opfiles); > - cleanup(); > - } > + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlp); > + > + TST_EXP_EQ_LI(getdtablesize(), rlp.rlim_cur); The actual syscall may not be equal to the prlimit AFAICT (this can be tested using the prlimit command). The table size can be set with a system wide sysctl /proc/sys/fs/nr_open defined in fs/file_table.c. This is used to set the initial prlimit, but can be reduced. > + > + tst_res(TINFO, "Start opening as many files as possible"); > + > + overfd = rlp.rlim_cur + 5; > > - tst_resm(TINFO, > - "Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1"); > - for (;;) { > - fd = open("/etc/hosts", O_RDONLY); > + fds = (int *)SAFE_MALLOC(overfd * sizeof(int)); > > - if (fd == -1) > + for (open_files = 0; open_files < overfd; open_files++) { > + TEST(open(FILENAME, O_RDONLY)); > + > + if (TST_RET == -1 && TST_ERR == EMFILE) { This could also fail with ENOMEM at least. Especially in a memcg and arch with large page sizes if there are any page sized allocations. Perhaps on some FS there would be other limits as well, resulting in a different error, but you can ignore that for now if you want. > + tst_res(TINFO, "Reached max amount of open files per process"); > break; > - count = fd; > + } > > -#ifdef DEBUG > - printf("Opened file num %d\n", fd); > -#endif > + fds[open_files] = TST_RET; > } > > -//Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page > + --open_files; > > - if (count > 0) > - close(count); > - if (count == (max_val_opfiles - 1)) > - tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1)); > - else if (fd < 0 && errno == ENFILE) > - tst_brkm(TCONF, cleanup, "Reached maximum number of open files for the system"); > - else > - tst_resm(TFAIL, "%d != %d", count, (max_val_opfiles - 1)); > + tst_res(TINFO, "Opened %d files", open_files); > > - cleanup(); > - tst_exit(); > -} > + for (int i = 0; i <= open_files; i++) > + SAFE_CLOSE(fds[i]); > > -void setup(void) > -{ > - tst_sig(NOFORK, DEF_HANDLER, cleanup); > + free(fds); > > - TEST_PAUSE; > + TST_EXP_EXPR(getdtablesize() >= open_files, > + "max amount of open files per process hasn't been overflowed"); > } > > -void cleanup(void) > +static void setup(void) > { > + int fd; > + > + fd = SAFE_CREAT(FILENAME, 0644); > + SAFE_CLOSE(fd); > } > + > +static struct tst_test test = { > + .test_all = run, > + .setup = setup, > + .needs_tmpdir = 1, This tests the underlying file system to some extent, so we should test all available file systems. > +}; > -- > 2.35.3
Hi! > > +static void setup(void) > > { > > + int fd; > > + > > + fd = SAFE_CREAT(FILENAME, 0644); > > + SAFE_CLOSE(fd); > > } > > + > > +static struct tst_test test = { > > + .test_all = run, > > + .setup = setup, > > + .needs_tmpdir = 1, > > This tests the underlying file system to some extent, so we should test > all available file systems. As far as I can tell it does not actually. This is a compatibility function for od unixes and the name is literrally 'get descriptor table size' so I suppose that it once returned the size of the array allocated for the process to store file descritptors into. So I supose that the only sensible approach is to call the function and check that it returns a sane value.
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> > +static void setup(void) >> > { >> > + int fd; >> > + >> > + fd = SAFE_CREAT(FILENAME, 0644); >> > + SAFE_CLOSE(fd); >> > } >> > + >> > +static struct tst_test test = { >> > + .test_all = run, >> > + .setup = setup, >> > + .needs_tmpdir = 1, >> >> This tests the underlying file system to some extent, so we should test >> all available file systems. > > As far as I can tell it does not actually. This is a compatibility > function for od unixes and the name is literrally 'get descriptor table > size' so I suppose that it once returned the size of the array allocated > for the process to store file descritptors into. > > So I supose that the only sensible approach is to call the function and > check that it returns a sane value. +1
Hi! On 10/19/23 09:53, Richard Palethorpe wrote: > Hello, > > Andrea Cervesato <andrea.cervesato@suse.de> writes: > >> From: Andrea Cervesato <andrea.cervesato@suse.com> >> >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> .../syscalls/getdtablesize/getdtablesize01.c | 146 +++++++----------- >> 1 file changed, 52 insertions(+), 94 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c >> index d25cac261..80321e24f 100644 >> --- a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c >> +++ b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c >> @@ -1,119 +1,77 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> /* >> * Copyright (c) International Business Machines Corp., 2005 >> * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. >> - * >> - * This program is free software; you can redistribute it and/or modify it >> - * under the terms of version 2 of the GNU General Public License as >> - * published by the Free Software Foundation. >> - * >> - * This program is distributed in the hope that it would be useful, but >> - * WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. >> - * >> - * You should have received a copy of the GNU General Public License along >> - * with this program; if not, write the Free Software Foundation, Inc., >> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >> - * >> + * Prashant P Yendigeri <prashant.yendigeri@wipro.com> >> + * Robbie Williamson <robbiew@us.ibm.com> >> + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >> */ >> -/********************************************************** >> - * >> - * TEST IDENTIFIER : getdtablesize01 >> - * >> - * EXECUTED BY : root / superuser >> - * >> - * TEST TITLE : Basic tests for getdtablesize01(2) >> - * >> - * TEST CASE TOTAL : 1 >> - * >> - * AUTHOR : Prashant P Yendigeri >> - * <prashant.yendigeri@wipro.com> >> - * Robbie Williamson >> - * <robbiew@us.ibm.com> >> - * >> - * DESCRIPTION >> - * This is a Phase I test for the getdtablesize01(2) system call. >> - * It is intended to provide a limited exposure of the system >> call. > It seems this test has been broken for a long time unless I am > mistaken. glibc does not call the actual syscall, it calls > prlimit. Which has different behaviour (see below). > >> - * >> - **********************************************************/ >> >> -#include <stdio.h> >> -#include <errno.h> >> -#include <sys/types.h> >> -#include <sys/stat.h> >> -#include <fcntl.h> >> -#include <sys/time.h> >> -#include <sys/resource.h> >> -#include <unistd.h> >> -#include "test.h" >> +/*\ >> + * [Description] >> + * >> + * This test verifies that getdtablesize() syscall returns the right max number >> + * of files which can be opened by a process. We test this in 2 ways: firstly by >> + * comparing its return value with RLIMIT_NOFILE, secondly by opening as many >> + * files as possible and then checking when open() raises EMFILE. >> + */ >> >> -void setup(); >> -void cleanup(); >> +#include <stdlib.h> >> +#include "tst_test.h" >> >> -char *TCID = "getdtablesize01"; >> -int TST_TOTAL = 1; >> +#define FILENAME "rofile" >> >> -int main(void) >> +static void run(void) >> { >> - int table_size, fd = 0, count = 0; >> - int max_val_opfiles; >> + int *fds; >> + int overfd; >> + int open_files; >> struct rlimit rlp; >> >> - setup(); >> - table_size = getdtablesize(); >> - getrlimit(RLIMIT_NOFILE, &rlp); >> - max_val_opfiles = (rlim_t) rlp.rlim_cur; >> - >> - tst_resm(TINFO, >> - "Maximum number of files a process can have opened is %d", >> - table_size); >> - tst_resm(TINFO, >> - "Checking with the value returned by getrlimit...RLIMIT_NOFILE"); >> - >> - if (table_size == max_val_opfiles) >> - tst_resm(TPASS, "got correct dtablesize, value is %d", >> - max_val_opfiles); >> - else { >> - tst_resm(TFAIL, "got incorrect table size, value is %d", >> - max_val_opfiles); >> - cleanup(); >> - } >> + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlp); >> + >> + TST_EXP_EQ_LI(getdtablesize(), rlp.rlim_cur); > The actual syscall may not be equal to the prlimit AFAICT (this can be > tested using the prlimit command). > > The table size can be set with a system wide sysctl /proc/sys/fs/nr_open > defined in fs/file_table.c. This is used to set the initial prlimit, but > can be reduced. > >> + >> + tst_res(TINFO, "Start opening as many files as possible"); >> + >> + overfd = rlp.rlim_cur + 5; >> >> - tst_resm(TINFO, >> - "Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1"); >> - for (;;) { >> - fd = open("/etc/hosts", O_RDONLY); >> + fds = (int *)SAFE_MALLOC(overfd * sizeof(int)); >> >> - if (fd == -1) >> + for (open_files = 0; open_files < overfd; open_files++) { >> + TEST(open(FILENAME, O_RDONLY)); >> + >> + if (TST_RET == -1 && TST_ERR == EMFILE) { > This could also fail with ENOMEM at least. Especially in a memcg and > arch with large page sizes if there are any page sized allocations. > > Perhaps on some FS there would be other limits as well, resulting in a > different error, but you can ignore that for now if you want. > >> + tst_res(TINFO, "Reached max amount of open files per process"); >> break; >> - count = fd; >> + } >> >> -#ifdef DEBUG >> - printf("Opened file num %d\n", fd); >> -#endif >> + fds[open_files] = TST_RET; >> } >> >> -//Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page >> + --open_files; >> >> - if (count > 0) >> - close(count); >> - if (count == (max_val_opfiles - 1)) >> - tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1)); >> - else if (fd < 0 && errno == ENFILE) >> - tst_brkm(TCONF, cleanup, "Reached maximum number of open files for the system"); >> - else >> - tst_resm(TFAIL, "%d != %d", count, (max_val_opfiles - 1)); >> + tst_res(TINFO, "Opened %d files", open_files); >> >> - cleanup(); >> - tst_exit(); >> -} >> + for (int i = 0; i <= open_files; i++) >> + SAFE_CLOSE(fds[i]); >> >> -void setup(void) >> -{ >> - tst_sig(NOFORK, DEF_HANDLER, cleanup); >> + free(fds); >> >> - TEST_PAUSE; >> + TST_EXP_EXPR(getdtablesize() >= open_files, >> + "max amount of open files per process hasn't been overflowed"); >> } >> >> -void cleanup(void) >> +static void setup(void) >> { >> + int fd; >> + >> + fd = SAFE_CREAT(FILENAME, 0644); >> + SAFE_CLOSE(fd); >> } >> + >> +static struct tst_test test = { >> + .test_all = run, >> + .setup = setup, >> + .needs_tmpdir = 1, > This tests the underlying file system to some extent, so we should test > all available file systems. > >> +}; >> -- >> 2.35.3 > I think we can just remove this test, since the syscall number is not even defined in the kernel and glibc only use prlimit. I don't know, what do you think? Andrea
Hello, > I think we can just remove this test, since the syscall number is not > even defined in the kernel and glibc only use prlimit. I don't know, > what do you think? > > > Andrea Yes, it only exists on Alpha as a real syscall AFAICT. I suppose it could be added to a prlimit test as an alias. I don't think it is worth the effort, but I wouldn't try to block it.
diff --git a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c index d25cac261..80321e24f 100644 --- a/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c +++ b/testcases/kernel/syscalls/getdtablesize/getdtablesize01.c @@ -1,119 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) International Business Machines Corp., 2005 * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it would be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * + * Prashant P Yendigeri <prashant.yendigeri@wipro.com> + * Robbie Williamson <robbiew@us.ibm.com> + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> */ -/********************************************************** - * - * TEST IDENTIFIER : getdtablesize01 - * - * EXECUTED BY : root / superuser - * - * TEST TITLE : Basic tests for getdtablesize01(2) - * - * TEST CASE TOTAL : 1 - * - * AUTHOR : Prashant P Yendigeri - * <prashant.yendigeri@wipro.com> - * Robbie Williamson - * <robbiew@us.ibm.com> - * - * DESCRIPTION - * This is a Phase I test for the getdtablesize01(2) system call. - * It is intended to provide a limited exposure of the system call. - * - **********************************************************/ -#include <stdio.h> -#include <errno.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> -#include <sys/time.h> -#include <sys/resource.h> -#include <unistd.h> -#include "test.h" +/*\ + * [Description] + * + * This test verifies that getdtablesize() syscall returns the right max number + * of files which can be opened by a process. We test this in 2 ways: firstly by + * comparing its return value with RLIMIT_NOFILE, secondly by opening as many + * files as possible and then checking when open() raises EMFILE. + */ -void setup(); -void cleanup(); +#include <stdlib.h> +#include "tst_test.h" -char *TCID = "getdtablesize01"; -int TST_TOTAL = 1; +#define FILENAME "rofile" -int main(void) +static void run(void) { - int table_size, fd = 0, count = 0; - int max_val_opfiles; + int *fds; + int overfd; + int open_files; struct rlimit rlp; - setup(); - table_size = getdtablesize(); - getrlimit(RLIMIT_NOFILE, &rlp); - max_val_opfiles = (rlim_t) rlp.rlim_cur; - - tst_resm(TINFO, - "Maximum number of files a process can have opened is %d", - table_size); - tst_resm(TINFO, - "Checking with the value returned by getrlimit...RLIMIT_NOFILE"); - - if (table_size == max_val_opfiles) - tst_resm(TPASS, "got correct dtablesize, value is %d", - max_val_opfiles); - else { - tst_resm(TFAIL, "got incorrect table size, value is %d", - max_val_opfiles); - cleanup(); - } + SAFE_GETRLIMIT(RLIMIT_NOFILE, &rlp); + + TST_EXP_EQ_LI(getdtablesize(), rlp.rlim_cur); + + tst_res(TINFO, "Start opening as many files as possible"); + + overfd = rlp.rlim_cur + 5; - tst_resm(TINFO, - "Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1"); - for (;;) { - fd = open("/etc/hosts", O_RDONLY); + fds = (int *)SAFE_MALLOC(overfd * sizeof(int)); - if (fd == -1) + for (open_files = 0; open_files < overfd; open_files++) { + TEST(open(FILENAME, O_RDONLY)); + + if (TST_RET == -1 && TST_ERR == EMFILE) { + tst_res(TINFO, "Reached max amount of open files per process"); break; - count = fd; + } -#ifdef DEBUG - printf("Opened file num %d\n", fd); -#endif + fds[open_files] = TST_RET; } -//Now the max files opened should be RLIMIT_NOFILE - 1 , why ? read getdtablesize man page + --open_files; - if (count > 0) - close(count); - if (count == (max_val_opfiles - 1)) - tst_resm(TPASS, "%d = %d", count, (max_val_opfiles - 1)); - else if (fd < 0 && errno == ENFILE) - tst_brkm(TCONF, cleanup, "Reached maximum number of open files for the system"); - else - tst_resm(TFAIL, "%d != %d", count, (max_val_opfiles - 1)); + tst_res(TINFO, "Opened %d files", open_files); - cleanup(); - tst_exit(); -} + for (int i = 0; i <= open_files; i++) + SAFE_CLOSE(fds[i]); -void setup(void) -{ - tst_sig(NOFORK, DEF_HANDLER, cleanup); + free(fds); - TEST_PAUSE; + TST_EXP_EXPR(getdtablesize() >= open_files, + "max amount of open files per process hasn't been overflowed"); } -void cleanup(void) +static void setup(void) { + int fd; + + fd = SAFE_CREAT(FILENAME, 0644); + SAFE_CLOSE(fd); } + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .needs_tmpdir = 1, +};