* [LTP] [PATCH] open04: add EMFILE check @ 2022-09-15 3:10 Li Wang 2022-09-15 12:52 ` Petr Vorel 0 siblings, 1 reply; 9+ messages in thread From: Li Wang @ 2022-09-15 3:10 UTC (permalink / raw) To: ltp [pre-release testing fix] Test in automation easily get EMFILE error before reaching the fds_limit, but hard to reproduce it again manually. The possible reason is that some shared fd being opened in the parent shell and occupying the fd numbers which inherited by test then results in open failed with EMFILE early. This patch adds back of the EMFILE check in the open() loops, to flexible test fd limitation. open04.c:36: TBROK: open(open04.1020,66,0777) failed: EMFILE (24) open04.c:53: TWARN: close(0) failed: EBADF (9) Signed-off-by: Li Wang <liwang@redhat.com> --- testcases/kernel/syscalls/open/open04.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/testcases/kernel/syscalls/open/open04.c b/testcases/kernel/syscalls/open/open04.c index d452405d4..01a8b12d6 100644 --- a/testcases/kernel/syscalls/open/open04.c +++ b/testcases/kernel/syscalls/open/open04.c @@ -33,7 +33,12 @@ static void setup(void) for (i = first + 1; i < fds_limit; i++) { sprintf(fname, FNAME ".%d", i); - fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0777); + fd = open(fname, O_RDWR | O_CREAT, 0777); + if (fd == -1) { + if (errno != EMFILE) + tst_brk(TBROK, "Expected EMFILE but got %d", errno); + break; + } fds[i - first] = fd; } } -- 2.35.3 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-15 3:10 [LTP] [PATCH] open04: add EMFILE check Li Wang @ 2022-09-15 12:52 ` Petr Vorel 2022-09-15 14:21 ` Cyril Hrubis 0 siblings, 1 reply; 9+ messages in thread From: Petr Vorel @ 2022-09-15 12:52 UTC (permalink / raw) To: Li Wang; +Cc: ltp Hi LI, > [pre-release testing fix] > Test in automation easily get EMFILE error before reaching the fds_limit, > but hard to reproduce it again manually. The possible reason is that some > shared fd being opened in the parent shell and occupying the fd numbers > which inherited by test then results in open failed with EMFILE early. > This patch adds back of the EMFILE check in the open() loops, to flexible > test fd limitation. > open04.c:36: TBROK: open(open04.1020,66,0777) failed: EMFILE (24) > open04.c:53: TWARN: close(0) failed: EBADF (9) > Signed-off-by: Li Wang <liwang@redhat.com> > --- > testcases/kernel/syscalls/open/open04.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > diff --git a/testcases/kernel/syscalls/open/open04.c b/testcases/kernel/syscalls/open/open04.c > index d452405d4..01a8b12d6 100644 > --- a/testcases/kernel/syscalls/open/open04.c > +++ b/testcases/kernel/syscalls/open/open04.c > @@ -33,7 +33,12 @@ static void setup(void) > for (i = first + 1; i < fds_limit; i++) { > sprintf(fname, FNAME ".%d", i); > - fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0777); > + fd = open(fname, O_RDWR | O_CREAT, 0777); > + if (fd == -1) { > + if (errno != EMFILE) > + tst_brk(TBROK, "Expected EMFILE but got %d", errno); > + break; > + } > fds[i - first] = fd; > } > } LGTM. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-15 12:52 ` Petr Vorel @ 2022-09-15 14:21 ` Cyril Hrubis 2022-09-16 1:36 ` Li Wang 0 siblings, 1 reply; 9+ messages in thread From: Cyril Hrubis @ 2022-09-15 14:21 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp Hi! > > diff --git a/testcases/kernel/syscalls/open/open04.c b/testcases/kernel/syscalls/open/open04.c > > index d452405d4..01a8b12d6 100644 > > --- a/testcases/kernel/syscalls/open/open04.c > > +++ b/testcases/kernel/syscalls/open/open04.c > > @@ -33,7 +33,12 @@ static void setup(void) > > > for (i = first + 1; i < fds_limit; i++) { > > sprintf(fname, FNAME ".%d", i); > > - fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0777); > > + fd = open(fname, O_RDWR | O_CREAT, 0777); > > + if (fd == -1) { > > + if (errno != EMFILE) > > + tst_brk(TBROK, "Expected EMFILE but got %d", errno); > > + break; > > + } > > fds[i - first] = fd; > > } > > } > > LGTM. > Reviewed-by: Petr Vorel <pvorel@suse.cz> I faintly remmeber a similar patch where we decided not to work around for a test harness leaking filedescriptors into testcases. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-15 14:21 ` Cyril Hrubis @ 2022-09-16 1:36 ` Li Wang 2022-09-16 9:39 ` Cyril Hrubis 0 siblings, 1 reply; 9+ messages in thread From: Li Wang @ 2022-09-16 1:36 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 1327 bytes --] On Thu, Sep 15, 2022 at 10:19 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > > diff --git a/testcases/kernel/syscalls/open/open04.c > b/testcases/kernel/syscalls/open/open04.c > > > index d452405d4..01a8b12d6 100644 > > > --- a/testcases/kernel/syscalls/open/open04.c > > > +++ b/testcases/kernel/syscalls/open/open04.c > > > @@ -33,7 +33,12 @@ static void setup(void) > > > > > for (i = first + 1; i < fds_limit; i++) { > > > sprintf(fname, FNAME ".%d", i); > > > - fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0777); > > > + fd = open(fname, O_RDWR | O_CREAT, 0777); > > > + if (fd == -1) { > > > + if (errno != EMFILE) > > > + tst_brk(TBROK, "Expected EMFILE but got > %d", errno); > > > + break; > > > + } > > > fds[i - first] = fd; > > > } > > > } > > > > LGTM. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > I faintly remmeber a similar patch where we decided not to work around > for a test harness leaking filedescriptors into testcases. > This also should be a solution, I searched the mailing list and got a patch[1]. Do you mean adding that close-on-exec flag when opening fd in harness? [1] https://lists.linux.it/pipermail/ltp/2020-November/019650.html -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 2465 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-16 1:36 ` Li Wang @ 2022-09-16 9:39 ` Cyril Hrubis 2022-09-19 12:12 ` Li Wang 0 siblings, 1 reply; 9+ messages in thread From: Cyril Hrubis @ 2022-09-16 9:39 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hi! > > I faintly remmeber a similar patch where we decided not to work around > > for a test harness leaking filedescriptors into testcases. > > > > This also should be a solution, I searched the mailing list and got a > patch[1]. > Do you mean adding that close-on-exec flag when opening fd in harness? Yes, that way you can be sure that no file descriptors are leaked to the tests. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-16 9:39 ` Cyril Hrubis @ 2022-09-19 12:12 ` Li Wang 2022-09-20 5:53 ` Li Wang 0 siblings, 1 reply; 9+ messages in thread From: Li Wang @ 2022-09-19 12:12 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 2378 bytes --] Cyril Hrubis <chrubis@suse.cz> wrote: Hi! > > > I faintly remmeber a similar patch where we decided not to work around > > > for a test harness leaking filedescriptors into testcases. > > > > > > > This also should be a solution, I searched the mailing list and got a > > patch[1]. > > Do you mean adding that close-on-exec flag when opening fd in harness? > > Yes, that way you can be sure that no file descriptors are leaked to the > tests. > Ok, should I send patch v2 like this below? Note: the automation test open04 got passed but I'm not sure if this has a side effect on logs. But from my observation, some tests (with old-API) log can't be collected anymore. --- a/pan/ltp-pan.c +++ b/pan/ltp-pan.c @@ -443,7 +443,7 @@ int main(int argc, char **argv) } if (outputfilename) { - if (!freopen(outputfilename, "a+", stdout)) { + if (!freopen(outputfilename, "a+e", stdout)) { fprintf(stderr, "pan(%s): Error %s (%d) opening output file '%s'\n", panname, strerror(errno), errno, @@ -565,7 +565,7 @@ int main(int argc, char **argv) } else if (starts == -1) //wjh { FILE *f = (FILE *) - 1; - if ((f = fopen(PAN_STOP_FILE, "r")) != 0) { + if ((f = fopen(PAN_STOP_FILE, "r+")) != 0) { printf("Got %s Stopping!\n", PAN_STOP_FILE); fclose(f); unlink(PAN_STOP_FILE); @@ -1277,7 +1277,7 @@ static char *slurp(char *file) int fd; struct stat sbuf; - if ((fd = open(file, O_RDONLY)) < 0) { + if ((fd = open(file, O_RDONLY | O_CLOEXEC)) < 0) { fprintf(stderr, "pan(%s): open(%s,O_RDONLY) failed. errno:%d %s\n", panname, file, errno, strerror(errno)); @@ -1372,7 +1372,7 @@ static void write_kmsg(const char *fmt, ...) FILE *kmsg; va_list ap; - if ((kmsg = fopen("/dev/kmsg", "r+")) == NULL) { + if ((kmsg = fopen("/dev/kmsg", "r+e")) == NULL) { fprintf(stderr, "Error %s: (%d) opening /dev/kmsg\n", strerror(errno), errno); exit(1); -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 3939 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-19 12:12 ` Li Wang @ 2022-09-20 5:53 ` Li Wang 2022-09-20 14:21 ` Cyril Hrubis 2022-09-20 18:29 ` Petr Vorel 0 siblings, 2 replies; 9+ messages in thread From: Li Wang @ 2022-09-20 5:53 UTC (permalink / raw) To: Cyril Hrubis; +Cc: LTP List [-- Attachment #1.1: Type: text/plain, Size: 1026 bytes --] On Mon, Sep 19, 2022 at 8:12 PM Li Wang <liwang@redhat.com> wrote: > Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! >> > > I faintly remmeber a similar patch where we decided not to work around >> > > for a test harness leaking filedescriptors into testcases. >> > > >> > >> > This also should be a solution, I searched the mailing list and got a >> > patch[1]. >> > Do you mean adding that close-on-exec flag when opening fd in harness? >> >> Yes, that way you can be sure that no file descriptors are leaked to the >> tests. >> > > Ok, should I send patch v2 like this below? > > Note: the automation test open04 got passed but I'm not sure > if this has a side effect on logs. But from my observation, some > tests (with old-API) log can't be collected anymore. > Seems we shouldn't fix by adding 'close-on-exec' flag simply, it brings more issues to some old-API tests, I'm still looking into the problems which look like caused by ltp-pan designed. So can we just merge the patch as the original? -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 2401 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-20 5:53 ` Li Wang @ 2022-09-20 14:21 ` Cyril Hrubis 2022-09-20 18:29 ` Petr Vorel 1 sibling, 0 replies; 9+ messages in thread From: Cyril Hrubis @ 2022-09-20 14:21 UTC (permalink / raw) To: Li Wang; +Cc: LTP List Hi! > >> > > I faintly remmeber a similar patch where we decided not to work around > >> > > for a test harness leaking filedescriptors into testcases. > >> > > > >> > > >> > This also should be a solution, I searched the mailing list and got a > >> > patch[1]. > >> > Do you mean adding that close-on-exec flag when opening fd in harness? > >> > >> Yes, that way you can be sure that no file descriptors are leaked to the > >> tests. > >> > > > > Ok, should I send patch v2 like this below? > > > > Note: the automation test open04 got passed but I'm not sure > > if this has a side effect on logs. But from my observation, some > > tests (with old-API) log can't be collected anymore. > > > > Seems we shouldn't fix by adding 'close-on-exec' flag simply, > it brings more issues to some old-API tests, I'm still looking into > the problems which look like caused by ltp-pan designed. Sound strange, what is the problem here. > So can we just merge the patch as the original? Sounds sensible for the release, feel free to add: Reviewed-by: Cyril Hrubis <chrubis@suse.cz> -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] open04: add EMFILE check 2022-09-20 5:53 ` Li Wang 2022-09-20 14:21 ` Cyril Hrubis @ 2022-09-20 18:29 ` Petr Vorel 1 sibling, 0 replies; 9+ messages in thread From: Petr Vorel @ 2022-09-20 18:29 UTC (permalink / raw) To: Li Wang; +Cc: LTP List > On Mon, Sep 19, 2022 at 8:12 PM Li Wang <liwang@redhat.com> wrote: > > Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > >> > > I faintly remmeber a similar patch where we decided not to work around > >> > > for a test harness leaking filedescriptors into testcases. > >> > This also should be a solution, I searched the mailing list and got a > >> > patch[1]. > >> > Do you mean adding that close-on-exec flag when opening fd in harness? > >> Yes, that way you can be sure that no file descriptors are leaked to the > >> tests. > > Ok, should I send patch v2 like this below? > > Note: the automation test open04 got passed but I'm not sure > > if this has a side effect on logs. But from my observation, some > > tests (with old-API) log can't be collected anymore. > Seems we shouldn't fix by adding 'close-on-exec' flag simply, > it brings more issues to some old-API tests, I'm still looking into > the problems which look like caused by ltp-pan designed. > So can we just merge the patch as the original? Acked-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-09-20 18:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-15 3:10 [LTP] [PATCH] open04: add EMFILE check Li Wang 2022-09-15 12:52 ` Petr Vorel 2022-09-15 14:21 ` Cyril Hrubis 2022-09-16 1:36 ` Li Wang 2022-09-16 9:39 ` Cyril Hrubis 2022-09-19 12:12 ` Li Wang 2022-09-20 5:53 ` Li Wang 2022-09-20 14:21 ` Cyril Hrubis 2022-09-20 18:29 ` Petr Vorel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.