| Message ID | 20240530144846.10915-1-akumar@suse.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] flock: Add test for verifying EINTR errno | expand |
Hi Avinesh, Yang Xu, > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > Signed-off-by: Avinesh Kumar <akumar@suse.de> ... > --- /dev/null > +++ b/testcases/kernel/syscalls/flock/flock07.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > + * Copyright (c) 2024 Linux Test Project > + */ > + > +/*\ > + * [Description] > + * > + * Verify that flock(2) fails with errno EINTR when waiting to acquire a lock, > + * and the call is interrupted by a signal. Avinesh, you mentioned at Yang Xu's v1 [1] that EINTR test is getting timed out randomly. I also experienced timeouts on aarch64 and ppc64le. v1 had 2 tests (EINTR and EWOULDBLOCK), you here posted only EINTR. I would expect you would put here only the other one - EWOULDBLOCK. Or am I missing something? Kind regards, Petr [1] https://lore.kernel.org/ltp/1934768.7Z3S40VBb9@localhost/
On Monday, October 21, 2024 9:55:21 PM CET Petr Vorel wrote: > Hi Avinesh, Yang Xu, > > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > ... > > --- /dev/null > > +++ b/testcases/kernel/syscalls/flock/flock07.c > > @@ -0,0 +1,70 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > > + * Copyright (c) 2024 Linux Test Project > > + */ > > + > > +/*\ > > + * [Description] > > + * > > + * Verify that flock(2) fails with errno EINTR when waiting to acquire a lock, > > + * and the call is interrupted by a signal. > > Avinesh, you mentioned at Yang Xu's v1 [1] that EINTR test is getting timed out > randomly. I also experienced timeouts on aarch64 and ppc64le. v1 had 2 tests > (EINTR and EWOULDBLOCK), you here posted only EINTR. I would expect you would > put here only the other one - EWOULDBLOCK. Or am I missing something? Hi Petr, I am sorry, I completely missed this. I sent the patch for EWOULDBLOCK case now - https://lore.kernel.org/ltp/20250114155013.7644-1-akumar@suse.de/T/#u Regards, Avinesh > > Kind regards, > Petr > > [1] https://lore.kernel.org/ltp/1934768.7Z3S40VBb9@localhost/ >
> On Monday, October 21, 2024 9:55:21 PM CET Petr Vorel wrote: > > Hi Avinesh, Yang Xu, > > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > ... > > > --- /dev/null > > > +++ b/testcases/kernel/syscalls/flock/flock07.c > > > @@ -0,0 +1,70 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +/* > > > + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > > > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > > > + * Copyright (c) 2024 Linux Test Project > > > + */ > > > + > > > +/*\ > > > + * [Description] > > > + * > > > + * Verify that flock(2) fails with errno EINTR when waiting to acquire a lock, > > > + * and the call is interrupted by a signal. > > Avinesh, you mentioned at Yang Xu's v1 [1] that EINTR test is getting timed out > > randomly. I also experienced timeouts on aarch64 and ppc64le. v1 had 2 tests > > (EINTR and EWOULDBLOCK), you here posted only EINTR. I would expect you would > > put here only the other one - EWOULDBLOCK. Or am I missing something? > Hi Petr, > I am sorry, I completely missed this. > I sent the patch for EWOULDBLOCK case now - > https://lore.kernel.org/ltp/20250114155013.7644-1-akumar@suse.de/T/#u Thanks for info. I'm closing this as rejected. Please let me know if I did not understand you and this is a valid patch. Kind regards, Petr > Regards, > Avinesh > > Kind regards, > > Petr > > [1] https://lore.kernel.org/ltp/1934768.7Z3S40VBb9@localhost/
On Friday, January 31, 2025 12:01:36 PM CET Petr Vorel wrote: > > On Monday, October 21, 2024 9:55:21 PM CET Petr Vorel wrote: > > > Hi Avinesh, Yang Xu, > > > > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > > > > Signed-off-by: Avinesh Kumar <akumar@suse.de> > > > ... > > > > --- /dev/null > > > > +++ b/testcases/kernel/syscalls/flock/flock07.c > > > > @@ -0,0 +1,70 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > > +/* > > > > + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > > > > + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> > > > > + * Copyright (c) 2024 Linux Test Project > > > > + */ > > > > + > > > > +/*\ > > > > + * [Description] > > > > + * > > > > + * Verify that flock(2) fails with errno EINTR when waiting to acquire a lock, > > > > + * and the call is interrupted by a signal. > > > > Avinesh, you mentioned at Yang Xu's v1 [1] that EINTR test is getting timed out > > > randomly. I also experienced timeouts on aarch64 and ppc64le. v1 had 2 tests > > > (EINTR and EWOULDBLOCK), you here posted only EINTR. I would expect you would > > > put here only the other one - EWOULDBLOCK. Or am I missing something? > > Hi Petr, > > > I am sorry, I completely missed this. > > I sent the patch for EWOULDBLOCK case now - > > https://lore.kernel.org/ltp/20250114155013.7644-1-akumar@suse.de/T/#u > > Thanks for info. I'm closing this as rejected. Please let me know if I did not > understand you and this is a valid patch. Hi Petr, the above v3 patch (for new test of flock/EINTR errno) is still valid. I took out the EWOULDBLOCK test from the original patch (by Yang Xu) and added that to flock02, you already merged that one. So you can review flock07.c patch just for the EINTR case. Thanks, Avinesh > > Kind regards, > Petr > > > Regards, > > Avinesh > > > > Kind regards, > > > Petr > > > > [1] https://lore.kernel.org/ltp/1934768.7Z3S40VBb9@localhost/ > > > > > >
Hi Avinesh, ... > > > Hi Petr, > > > I am sorry, I completely missed this. > > > I sent the patch for EWOULDBLOCK case now - > > > https://lore.kernel.org/ltp/20250114155013.7644-1-akumar@suse.de/T/#u > > Thanks for info. I'm closing this as rejected. Please let me know if I did not > > understand you and this is a valid patch. > Hi Petr, the above v3 patch (for new test of flock/EINTR errno) is still valid. > I took out the EWOULDBLOCK test from the original patch (by Yang Xu) and > added that to flock02, you already merged that one. > So you can review flock07.c patch just for the EINTR case. Thanks for info, I reopened it in patchwork and I'll have look later. I see, the crash is workarounded by forking child and verifying, that's why it's not in flock02.c (which has other errnos, which don't need a workaround with forking). Kind regards, Petr
Hi Avinesh, all, ... > +++ b/testcases/kernel/syscalls/flock/flock07.c ... > +static void handler(int sig) > +{ > + tst_res(TINFO, "Received signal: %d", sig); How about print a signal constant/name? tst_res(TINFO, "Received signal: %s (%d)", tst_strsig(sig), sig); > +} ... > +static void verify_flock(void) > +{ > + pid_t pid; > + int fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); > + int fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); How about to setup file descriptors in setup() and close them in cleanup()? I suggest to merge with the change below. > + > + TST_EXP_PASS(flock(fd1, LOCK_EX)); > + > + pid = SAFE_FORK(); > + if (!pid) { > + child_do(fd2); > + exit(0); > + } else { > + sleep(1); > + SAFE_KILL(pid, SIGUSR1); > + SAFE_WAITPID(pid, NULL, 0); > + } > + > + SAFE_CLOSE(fd1); > + SAFE_CLOSE(fd2); > +} Kind regards, Petr +++ testcases/kernel/syscalls/flock/flock07.c @@ -17,14 +17,27 @@ #define TEMPFILE "test_eintr" +static int fd1 = -1, fd2 = -1; + static void handler(int sig) { - tst_res(TINFO, "Received signal: %d", sig); + tst_res(TINFO, "Received signal: %s (%d)", tst_strsig(sig), sig); } static void setup(void) { SAFE_TOUCH(TEMPFILE, 0777, NULL); + fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); + fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); +} + +static void cleanup(void) +{ + if (fd1 >= 0) + SAFE_CLOSE(fd1); + + if (fd2 >= 0) + SAFE_CLOSE(fd2); } static void child_do(int fd) @@ -42,8 +55,6 @@ static void child_do(int fd) static void verify_flock(void) { pid_t pid; - int fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); - int fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); TST_EXP_PASS(flock(fd1, LOCK_EX)); @@ -56,13 +67,11 @@ static void verify_flock(void) SAFE_KILL(pid, SIGUSR1); SAFE_WAITPID(pid, NULL, 0); } - - SAFE_CLOSE(fd1); - SAFE_CLOSE(fd2); } static struct tst_test test = { .setup = setup, + .cleanup = cleanup, .test_all = verify_flock, .needs_tmpdir = 1, .needs_root = 1,
On Monday, February 3, 2025 3:22:13 PM CET Petr Vorel wrote: > Hi Avinesh, all, > > ... > > +++ b/testcases/kernel/syscalls/flock/flock07.c > ... > > +static void handler(int sig) > > +{ > > + tst_res(TINFO, "Received signal: %d", sig); > How about print a signal constant/name? > > tst_res(TINFO, "Received signal: %s (%d)", tst_strsig(sig), sig); > > +} > ... > > +static void verify_flock(void) > > +{ > > + pid_t pid; > > + int fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); > > + int fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); > How about to setup file descriptors in setup() and close them in cleanup()? > > I suggest to merge with the change below. Hi Petr, Thank you for the review, and I agree with your suggestions. Regards, Avinesh > > > + > > + TST_EXP_PASS(flock(fd1, LOCK_EX)); > > + > > + pid = SAFE_FORK(); > > + if (!pid) { > > + child_do(fd2); > > + exit(0); > > + } else { > > + sleep(1); > > + SAFE_KILL(pid, SIGUSR1); > > + SAFE_WAITPID(pid, NULL, 0); > > + } > > + > > + SAFE_CLOSE(fd1); > > + SAFE_CLOSE(fd2); > > +} > > Kind regards, > Petr > > +++ testcases/kernel/syscalls/flock/flock07.c > @@ -17,14 +17,27 @@ > > #define TEMPFILE "test_eintr" > > +static int fd1 = -1, fd2 = -1; > + > static void handler(int sig) > { > - tst_res(TINFO, "Received signal: %d", sig); > + tst_res(TINFO, "Received signal: %s (%d)", tst_strsig(sig), sig); > } > > static void setup(void) > { > SAFE_TOUCH(TEMPFILE, 0777, NULL); > + fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); > + fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); > +} > + > +static void cleanup(void) > +{ > + if (fd1 >= 0) > + SAFE_CLOSE(fd1); > + > + if (fd2 >= 0) > + SAFE_CLOSE(fd2); > } > > static void child_do(int fd) > @@ -42,8 +55,6 @@ static void child_do(int fd) > static void verify_flock(void) > { > pid_t pid; > - int fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); > - int fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); > > TST_EXP_PASS(flock(fd1, LOCK_EX)); > > @@ -56,13 +67,11 @@ static void verify_flock(void) > SAFE_KILL(pid, SIGUSR1); > SAFE_WAITPID(pid, NULL, 0); > } > - > - SAFE_CLOSE(fd1); > - SAFE_CLOSE(fd2); > } > > static struct tst_test test = { > .setup = setup, > + .cleanup = cleanup, > .test_all = verify_flock, > .needs_tmpdir = 1, > .needs_root = 1, >
Hi Avinesh, all, ... > Hi Petr, > Thank you for the review, and I agree with your suggestions. Good, merged. Thank you! Kind regards, Petr
diff --git a/runtest/syscalls b/runtest/syscalls index cf06ee563..091453fec 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -374,6 +374,7 @@ flock02 flock02 flock03 flock03 flock04 flock04 flock06 flock06 +flock07 flock07 fmtmsg01 fmtmsg01 diff --git a/testcases/kernel/syscalls/flock/.gitignore b/testcases/kernel/syscalls/flock/.gitignore index c8cb0fc54..9bac582e1 100644 --- a/testcases/kernel/syscalls/flock/.gitignore +++ b/testcases/kernel/syscalls/flock/.gitignore @@ -3,3 +3,4 @@ /flock03 /flock04 /flock06 +/flock07 diff --git a/testcases/kernel/syscalls/flock/flock07.c b/testcases/kernel/syscalls/flock/flock07.c new file mode 100644 index 000000000..b2de84905 --- /dev/null +++ b/testcases/kernel/syscalls/flock/flock07.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. + * Author: Yang Xu <xuyang2018.jy@fujitsu.com> + * Copyright (c) 2024 Linux Test Project + */ + +/*\ + * [Description] + * + * Verify that flock(2) fails with errno EINTR when waiting to acquire a lock, + * and the call is interrupted by a signal. + */ + +#include <sys/file.h> +#include "tst_test.h" + +#define TEMPFILE "test_eintr" + +static void handler(int sig) +{ + tst_res(TINFO, "Received signal: %d", sig); +} + +static void setup(void) +{ + SAFE_TOUCH(TEMPFILE, 0777, NULL); +} + +static void child_do(int fd) +{ + struct sigaction sa; + + sa.sa_handler = handler; + SAFE_SIGEMPTYSET(&sa.sa_mask); + SAFE_SIGACTION(SIGUSR1, &sa, NULL); + + tst_res(TINFO, "Trying to acquire exclusive lock from child"); + TST_EXP_FAIL(flock(fd, LOCK_EX), EINTR); +} + +static void verify_flock(void) +{ + pid_t pid; + int fd1 = SAFE_OPEN(TEMPFILE, O_RDWR); + int fd2 = SAFE_OPEN(TEMPFILE, O_RDWR); + + TST_EXP_PASS(flock(fd1, LOCK_EX)); + + pid = SAFE_FORK(); + if (!pid) { + child_do(fd2); + exit(0); + } else { + sleep(1); + SAFE_KILL(pid, SIGUSR1); + SAFE_WAITPID(pid, NULL, 0); + } + + SAFE_CLOSE(fd1); + SAFE_CLOSE(fd2); +} + +static struct tst_test test = { + .setup = setup, + .test_all = verify_flock, + .needs_tmpdir = 1, + .needs_root = 1, + .forks_child = 1, +};