Linux Test Project
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code
@ 2022-02-09  9:42 Yang Xu
  2022-02-09  9:42 ` [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Yang Xu @ 2022-02-09  9:42 UTC (permalink / raw)
  To: ltp

1) make use of TST_EXP_FAIL2 or TST_EXP_PID_SILENT macros
2) remove min_kver check and use pidfd_open_supported()
3) Add docparse formatting

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/pidfd_open.h                     |  8 +++--
 .../kernel/syscalls/pidfd_open/pidfd_open01.c | 22 +++++++-----
 .../kernel/syscalls/pidfd_open/pidfd_open02.c | 34 +++++++------------
 .../kernel/syscalls/pidfd_open/pidfd_open03.c | 16 ++++++---
 4 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
index 9806c73d4..5cf10933e 100644
--- a/include/lapi/pidfd_open.h
+++ b/include/lapi/pidfd_open.h
@@ -9,11 +9,15 @@
 
 #include <sys/syscall.h>
 #include <sys/types.h>
-
 #include "lapi/syscalls.h"
-
 #include "config.h"
 
+static inline void pidfd_open_supported(void)
+{
+	/* allow the tests to fail early */
+	tst_syscall(__NR_pidfd_open);
+}
+
 #ifndef HAVE_PIDFD_OPEN
 static inline int pidfd_open(pid_t pid, unsigned int flags)
 {
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
index f40e9b624..ed9b82637 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
@@ -1,11 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
  *
- * Description:
  * Basic pidfd_open() test:
- * 1) Fetch the PID of the current process and try to get its file descriptor.
- * 2) Check that the close-on-exec flag is set on the file descriptor.
+ *
+ * - Fetch the PID of the current process and try to get its file descriptor.
+ * - Check that the close-on-exec flag is set on the file descriptor.
  */
 
 #include <unistd.h>
@@ -17,10 +21,7 @@ static void run(void)
 {
 	int flag;
 
-	TEST(pidfd_open(getpid(), 0));
-
-	if (TST_RET == -1)
-		tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed");
+	TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)");
 
 	flag = fcntl(TST_RET, F_GETFD);
 
@@ -35,7 +36,12 @@ static void run(void)
 	tst_res(TPASS, "pidfd_open(getpid(), 0) passed");
 }
 
+static void setup(void)
+{
+	pidfd_open_supported();
+}
+
 static struct tst_test test = {
-	.min_kver = "5.3",
+	.setup = setup,
 	.test_all = run,
 };
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
index dc86cae7a..93a61a51d 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
@@ -1,14 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
+ *
+ * Tests basic error handling of the pidfd_open syscall.
  *
- * Description:
- * Basic pidfd_open() test to test invalid arguments.
+ * - ESRCH the process specified by pid does not exist
+ * - EINVAL pid is not valid
+ * - EINVAL flags is not valid
  */
 #include "tst_test.h"
 #include "lapi/pidfd_open.h"
 
-pid_t expired_pid, my_pid, invalid_pid = -1;
+static pid_t expired_pid, my_pid, invalid_pid = -1;
 
 static struct tcase {
 	char *name;
@@ -23,6 +30,7 @@ static struct tcase {
 
 static void setup(void)
 {
+	pidfd_open_supported();
 	expired_pid = tst_get_unused_pid();
 	my_pid = getpid();
 }
@@ -31,27 +39,11 @@ static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
-	TEST(pidfd_open(*tc->pid, tc->flags));
-
-	if (TST_RET != -1) {
-		SAFE_CLOSE(TST_RET);
-		tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)",
-			tc->name, n);
-		return;
-	}
-
-	if (tc->exp_errno != TST_ERR) {
-		tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s",
-			tc->name, tst_strerrno(tc->exp_errno));
-		return;
-	}
-
-	tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected",
-		tc->name);
+	TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno,
+			"pidfd_open with %s", tc->name);
 }
 
 static struct tst_test test = {
-	.min_kver = "5.3",
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = run,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
index 48470e5e1..f41afa2fc 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
  *
- * Description:
  * This program opens the PID file descriptor of the child process created with
  * fork(). It then uses poll to monitor the file descriptor for process exit, as
  * indicated by an EPOLLIN event.
@@ -27,11 +30,9 @@ static void run(void)
 		exit(EXIT_SUCCESS);
 	}
 
-	TEST(pidfd_open(pid, 0));
+	TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid);
 
 	fd = TST_RET;
-	if (fd == -1)
-		tst_brk(TFAIL | TTERRNO, "pidfd_open() failed");
 
 	TST_CHECKPOINT_WAKE(0);
 
@@ -49,8 +50,13 @@ static void run(void)
 		tst_res(TPASS, "pidfd_open() passed");
 }
 
+static void setup(void)
+{
+	pidfd_open_supported();
+}
+
 static struct tst_test test = {
-	.min_kver = "5.3",
+	.setup = setup,
 	.test_all = run,
 	.forks_child = 1,
 	.needs_checkpoints = 1,
-- 
2.23.0


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-09  9:42 [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code Yang Xu
@ 2022-02-09  9:42 ` Yang Xu
  2022-02-09 13:47   ` Cyril Hrubis
  2022-02-09 10:00 ` [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code xuyang2018.jy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Yang Xu @ 2022-02-09  9:42 UTC (permalink / raw)
  To: ltp

As pidfd_open man-page said
"PIDFD_NONBLOCK (since Linux 5.10)
Return a nonblocking file descriptor.  If the process referred to by
the file descriptor has not yet terminated, then an attempt to wait
on the file descriptor using waitid(2) will immediately  return
the error EAGAIN rather than blocking."

Test this and also test whether set NONBLOCK flag in its pidfd.

Noticed that, don't introduce lapi/pidfd.h because linux/pidfd.h uses
kernel header fcntl.h but ltp api uses libc header. so it may
exist redefinition error of 'struct flock'[1].

[1]https://github.com/golang/go/issues/48221

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                              |  1 +
 .../kernel/syscalls/pidfd_open/.gitignore     |  1 +
 .../kernel/syscalls/pidfd_open/pidfd_open04.c | 78 +++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index c3e037f72..ce6f89f88 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -954,6 +954,7 @@ personality02 personality02
 pidfd_open01 pidfd_open01
 pidfd_open02 pidfd_open02
 pidfd_open03 pidfd_open03
+pidfd_open04 pidfd_open04
 
 pidfd_send_signal01 pidfd_send_signal01
 pidfd_send_signal02 pidfd_send_signal02
diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
index e0b8900c1..cebdc624d 100644
--- a/testcases/kernel/syscalls/pidfd_open/.gitignore
+++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
@@ -1,3 +1,4 @@
 pidfd_open01
 pidfd_open02
 pidfd_open03
+pidfd_open04
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
new file mode 100644
index 000000000..436351f23
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
+ * that waitid() with a non-blocking pidfd returns EAGAIN.
+ */
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+
+#ifndef PIDFD_NONBLOCK
+#define PIDFD_NONBLOCK O_NONBLOCK
+#endif
+
+#ifndef P_PIDFD
+#define P_PIDFD  3
+#endif
+
+static void run(void)
+{
+	int flag, pid, pidfd;
+	siginfo_t info;
+
+	pid = SAFE_FORK();
+	if (!pid)
+		pause();
+
+	TST_EXP_PID_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
+				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);
+
+	pidfd = TST_RET;
+	flag = fcntl(pidfd, F_GETFL);
+	if (flag == -1)
+		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
+
+	if (!(flag & O_NONBLOCK))
+		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
+
+	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
+
+	TST_EXP_FAIL(waitid(P_PIDFD, pidfd, &info, WEXITED), EAGAIN,
+			"waitid(P_PIDFD,...,WEXITED)");
+
+	SAFE_KILL(pid, SIGTERM);
+	SAFE_WAIT(NULL);
+	SAFE_CLOSE(pidfd);
+}
+
+static void setup(void)
+{
+	pidfd_open_supported();
+
+	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
+	if (TST_RET == -1) {
+		if (TST_ERR == EINVAL) {
+			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
+			return;
+		}
+		tst_brk(TFAIL | TTERRNO,
+			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
+	}
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.test_all = run,
+};
-- 
2.23.0


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code
  2022-02-09  9:42 [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code Yang Xu
  2022-02-09  9:42 ` [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
@ 2022-02-09 10:00 ` xuyang2018.jy
  2022-02-09 13:23 ` Cyril Hrubis
  2022-02-09 14:19 ` Petr Vorel
  3 siblings, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-02-09 10:00 UTC (permalink / raw)
  To: ltp@lists.linux.it

Hi All

It seems I should use TST_EXP_FD_SILENT instead of TST_EXP_PID_SILENT.

Best Regards
Yang Xu
> 1) make use of TST_EXP_FAIL2 or TST_EXP_PID_SILENT macros
> 2) remove min_kver check and use pidfd_open_supported()
> 3) Add docparse formatting
> 
> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> ---
>   include/lapi/pidfd_open.h                     |  8 +++--
>   .../kernel/syscalls/pidfd_open/pidfd_open01.c | 22 +++++++-----
>   .../kernel/syscalls/pidfd_open/pidfd_open02.c | 34 +++++++------------
>   .../kernel/syscalls/pidfd_open/pidfd_open03.c | 16 ++++++---
>   4 files changed, 44 insertions(+), 36 deletions(-)
> 
> diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
> index 9806c73d4..5cf10933e 100644
> --- a/include/lapi/pidfd_open.h
> +++ b/include/lapi/pidfd_open.h
> @@ -9,11 +9,15 @@
> 
>   #include<sys/syscall.h>
>   #include<sys/types.h>
> -
>   #include "lapi/syscalls.h"
> -
>   #include "config.h"
> 
> +static inline void pidfd_open_supported(void)
> +{
> +	/* allow the tests to fail early */
> +	tst_syscall(__NR_pidfd_open);
> +}
> +
>   #ifndef HAVE_PIDFD_OPEN
>   static inline int pidfd_open(pid_t pid, unsigned int flags)
>   {
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> index f40e9b624..ed9b82637 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> @@ -1,11 +1,15 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
>    *
> - * Description:
>    * Basic pidfd_open() test:
> - * 1) Fetch the PID of the current process and try to get its file descriptor.
> - * 2) Check that the close-on-exec flag is set on the file descriptor.
> + *
> + * - Fetch the PID of the current process and try to get its file descriptor.
> + * - Check that the close-on-exec flag is set on the file descriptor.
>    */
> 
>   #include<unistd.h>
> @@ -17,10 +21,7 @@ static void run(void)
>   {
>   	int flag;
> 
> -	TEST(pidfd_open(getpid(), 0));
> -
> -	if (TST_RET == -1)
> -		tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed");
> +	TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)");
> 
>   	flag = fcntl(TST_RET, F_GETFD);
> 
> @@ -35,7 +36,12 @@ static void run(void)
>   	tst_res(TPASS, "pidfd_open(getpid(), 0) passed");
>   }
> 
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +}
> +
>   static struct tst_test test = {
> -	.min_kver = "5.3",
> +	.setup = setup,
>   	.test_all = run,
>   };
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> index dc86cae7a..93a61a51d 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> @@ -1,14 +1,21 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Tests basic error handling of the pidfd_open syscall.
>    *
> - * Description:
> - * Basic pidfd_open() test to test invalid arguments.
> + * - ESRCH the process specified by pid does not exist
> + * - EINVAL pid is not valid
> + * - EINVAL flags is not valid
>    */
>   #include "tst_test.h"
>   #include "lapi/pidfd_open.h"
> 
> -pid_t expired_pid, my_pid, invalid_pid = -1;
> +static pid_t expired_pid, my_pid, invalid_pid = -1;
> 
>   static struct tcase {
>   	char *name;
> @@ -23,6 +30,7 @@ static struct tcase {
> 
>   static void setup(void)
>   {
> +	pidfd_open_supported();
>   	expired_pid = tst_get_unused_pid();
>   	my_pid = getpid();
>   }
> @@ -31,27 +39,11 @@ static void run(unsigned int n)
>   {
>   	struct tcase *tc =&tcases[n];
> 
> -	TEST(pidfd_open(*tc->pid, tc->flags));
> -
> -	if (TST_RET != -1) {
> -		SAFE_CLOSE(TST_RET);
> -		tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)",
> -			tc->name, n);
> -		return;
> -	}
> -
> -	if (tc->exp_errno != TST_ERR) {
> -		tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s",
> -			tc->name, tst_strerrno(tc->exp_errno));
> -		return;
> -	}
> -
> -	tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected",
> -		tc->name);
> +	TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno,
> +			"pidfd_open with %s", tc->name);
>   }
> 
>   static struct tst_test test = {
> -	.min_kver = "5.3",
>   	.tcnt = ARRAY_SIZE(tcases),
>   	.test = run,
>   	.setup = setup,
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> index 48470e5e1..f41afa2fc 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> @@ -1,8 +1,11 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) 2020 Viresh Kumar<viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
>    *
> - * Description:
>    * This program opens the PID file descriptor of the child process created with
>    * fork(). It then uses poll to monitor the file descriptor for process exit, as
>    * indicated by an EPOLLIN event.
> @@ -27,11 +30,9 @@ static void run(void)
>   		exit(EXIT_SUCCESS);
>   	}
> 
> -	TEST(pidfd_open(pid, 0));
> +	TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid);
> 
>   	fd = TST_RET;
> -	if (fd == -1)
> -		tst_brk(TFAIL | TTERRNO, "pidfd_open() failed");
> 
>   	TST_CHECKPOINT_WAKE(0);
> 
> @@ -49,8 +50,13 @@ static void run(void)
>   		tst_res(TPASS, "pidfd_open() passed");
>   }
> 
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +}
> +
>   static struct tst_test test = {
> -	.min_kver = "5.3",
> +	.setup = setup,
>   	.test_all = run,
>   	.forks_child = 1,
>   	.needs_checkpoints = 1,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code
  2022-02-09  9:42 [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code Yang Xu
  2022-02-09  9:42 ` [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
  2022-02-09 10:00 ` [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code xuyang2018.jy
@ 2022-02-09 13:23 ` Cyril Hrubis
  2022-02-09 14:19 ` Petr Vorel
  3 siblings, 0 replies; 12+ messages in thread
From: Cyril Hrubis @ 2022-02-09 13:23 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi!
> diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
> index 9806c73d4..5cf10933e 100644
> --- a/include/lapi/pidfd_open.h
> +++ b/include/lapi/pidfd_open.h
> @@ -9,11 +9,15 @@
>  
>  #include <sys/syscall.h>
>  #include <sys/types.h>
> -
>  #include "lapi/syscalls.h"
> -
>  #include "config.h"
>  
> +static inline void pidfd_open_supported(void)
> +{
> +	/* allow the tests to fail early */
> +	tst_syscall(__NR_pidfd_open);
> +}
> +
>  #ifndef HAVE_PIDFD_OPEN
>  static inline int pidfd_open(pid_t pid, unsigned int flags)
>  {
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> index f40e9b624..ed9b82637 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> @@ -1,11 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
>   *
> - * Description:
>   * Basic pidfd_open() test:
> - * 1) Fetch the PID of the current process and try to get its file descriptor.
> - * 2) Check that the close-on-exec flag is set on the file descriptor.
> + *
> + * - Fetch the PID of the current process and try to get its file descriptor.
> + * - Check that the close-on-exec flag is set on the file descriptor.
>   */
>  
>  #include <unistd.h>
> @@ -17,10 +21,7 @@ static void run(void)
>  {
>  	int flag;
>  
> -	TEST(pidfd_open(getpid(), 0));
> -
> -	if (TST_RET == -1)
> -		tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed");
> +	TST_EXP_PID_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)");

Techincally the return value is a fd so this should be
TST_EXP_FD_SILENT().

>  	flag = fcntl(TST_RET, F_GETFD);
>  
> @@ -35,7 +36,12 @@ static void run(void)
>  	tst_res(TPASS, "pidfd_open(getpid(), 0) passed");
>  }
>  
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +}
> +
>  static struct tst_test test = {
> -	.min_kver = "5.3",
> +	.setup = setup,
>  	.test_all = run,
>  };
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> index dc86cae7a..93a61a51d 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> @@ -1,14 +1,21 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Tests basic error handling of the pidfd_open syscall.
>   *
> - * Description:
> - * Basic pidfd_open() test to test invalid arguments.
> + * - ESRCH the process specified by pid does not exist
> + * - EINVAL pid is not valid
> + * - EINVAL flags is not valid
>   */
>  #include "tst_test.h"
>  #include "lapi/pidfd_open.h"
>  
> -pid_t expired_pid, my_pid, invalid_pid = -1;
> +static pid_t expired_pid, my_pid, invalid_pid = -1;
>  
>  static struct tcase {
>  	char *name;
> @@ -23,6 +30,7 @@ static struct tcase {
>  
>  static void setup(void)
>  {
> +	pidfd_open_supported();
>  	expired_pid = tst_get_unused_pid();
>  	my_pid = getpid();
>  }
> @@ -31,27 +39,11 @@ static void run(unsigned int n)
>  {
>  	struct tcase *tc = &tcases[n];
>  
> -	TEST(pidfd_open(*tc->pid, tc->flags));
> -
> -	if (TST_RET != -1) {
> -		SAFE_CLOSE(TST_RET);
> -		tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)",
> -			tc->name, n);
> -		return;
> -	}
> -
> -	if (tc->exp_errno != TST_ERR) {
> -		tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s",
> -			tc->name, tst_strerrno(tc->exp_errno));
> -		return;
> -	}
> -
> -	tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected",
> -		tc->name);
> +	TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno,
> +			"pidfd_open with %s", tc->name);
>  }
>  
>  static struct tst_test test = {
> -	.min_kver = "5.3",
>  	.tcnt = ARRAY_SIZE(tcases),
>  	.test = run,
>  	.setup = setup,
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> index 48470e5e1..f41afa2fc 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> @@ -1,8 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + */
> +
> +/*\
> + * [Description]
>   *
> - * Description:
>   * This program opens the PID file descriptor of the child process created with
>   * fork(). It then uses poll to monitor the file descriptor for process exit, as
>   * indicated by an EPOLLIN event.
> @@ -27,11 +30,9 @@ static void run(void)
>  		exit(EXIT_SUCCESS);
>  	}
>  
> -	TEST(pidfd_open(pid, 0));
> +	TST_EXP_PID_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid);

Here as well.

Otherwise it looks good:

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] 12+ messages in thread

* Re: [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-09  9:42 ` [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
@ 2022-02-09 13:47   ` Cyril Hrubis
  2022-02-10  1:49     ` xuyang2018.jy
  0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2022-02-09 13:47 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi!
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
> new file mode 100644
> index 000000000..436351f23
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
> + * that waitid() with a non-blocking pidfd returns EAGAIN.
> + */
> +
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/wait.h>
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +
> +#ifndef PIDFD_NONBLOCK
> +#define PIDFD_NONBLOCK O_NONBLOCK
> +#endif
> +
> +#ifndef P_PIDFD
> +#define P_PIDFD  3
> +#endif
> +
> +static void run(void)
> +{
> +	int flag, pid, pidfd;
> +	siginfo_t info;
> +
> +	pid = SAFE_FORK();
> +	if (!pid)
> +		pause();
> +
> +	TST_EXP_PID_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
> +				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);

Here as well FD_SILENT();

> +	pidfd = TST_RET;
> +	flag = fcntl(pidfd, F_GETFL);
> +	if (flag == -1)
> +		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
> +
> +	if (!(flag & O_NONBLOCK))
> +		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
> +
> +	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
> +
> +	TST_EXP_FAIL(waitid(P_PIDFD, pidfd, &info, WEXITED), EAGAIN,
> +			"waitid(P_PIDFD,...,WEXITED)");
> +
> +	SAFE_KILL(pid, SIGTERM);
> +	SAFE_WAIT(NULL);
> +	SAFE_CLOSE(pidfd);

I guess that we can also test that the waitid() succeds now, right?

> +}
> +
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +
> +	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
> +	if (TST_RET == -1) {
> +		if (TST_ERR == EINVAL) {
> +			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
> +			return;
> +		}
> +		tst_brk(TFAIL | TTERRNO,
> +			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.test_all = run,
> +};

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code
  2022-02-09  9:42 [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code Yang Xu
                   ` (2 preceding siblings ...)
  2022-02-09 13:23 ` Cyril Hrubis
@ 2022-02-09 14:19 ` Petr Vorel
  2022-02-10  1:50   ` xuyang2018.jy
  2022-02-10  3:41   ` [LTP] [PATCH v2 " Yang Xu
  3 siblings, 2 replies; 12+ messages in thread
From: Petr Vorel @ 2022-02-09 14:19 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Xu,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Nice cleanup.
+1 for Cyril's note to TST_EXP_FD_SILENT().

> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +}
> +
>  static struct tst_test test = {
> -	.min_kver = "5.3",
> +	.setup = setup,
>  	.test_all = run,
why not just .setup = pidfd_open_supported; ?

...

> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> index 48470e5e1..f41afa2fc 100644
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
...
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +}
> +
And here as well.

Kind regards,
Petr

>  static struct tst_test test = {
> -	.min_kver = "5.3",
> +	.setup = setup,
>  	.test_all = run,
>  	.forks_child = 1,
>  	.needs_checkpoints = 1,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-09 13:47   ` Cyril Hrubis
@ 2022-02-10  1:49     ` xuyang2018.jy
  0 siblings, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-02-10  1:49 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp@lists.linux.it

Hi Cyril
> Hi!
>> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
>> new file mode 100644
>> index 000000000..436351f23
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
>> @@ -0,0 +1,78 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
>> + * that waitid() with a non-blocking pidfd returns EAGAIN.
>> + */
>> +
>> +#include<unistd.h>
>> +#include<fcntl.h>
>> +#include<sys/wait.h>
>> +#include "tst_test.h"
>> +#include "lapi/pidfd_open.h"
>> +
>> +#ifndef PIDFD_NONBLOCK
>> +#define PIDFD_NONBLOCK O_NONBLOCK
>> +#endif
>> +
>> +#ifndef P_PIDFD
>> +#define P_PIDFD  3
>> +#endif
>> +
>> +static void run(void)
>> +{
>> +	int flag, pid, pidfd;
>> +	siginfo_t info;
>> +
>> +	pid = SAFE_FORK();
>> +	if (!pid)
>> +		pause();
>> +
>> +	TST_EXP_PID_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
>> +				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);
>
> Here as well FD_SILENT();
Yes.
>
>> +	pidfd = TST_RET;
>> +	flag = fcntl(pidfd, F_GETFL);
>> +	if (flag == -1)
>> +		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
>> +
>> +	if (!(flag&  O_NONBLOCK))
>> +		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
>> +
>> +	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
>> +
>> +	TST_EXP_FAIL(waitid(P_PIDFD, pidfd,&info, WEXITED), EAGAIN,
>> +			"waitid(P_PIDFD,...,WEXITED)");
>> +
>> +	SAFE_KILL(pid, SIGTERM);
>> +	SAFE_WAIT(NULL);
>> +	SAFE_CLOSE(pidfd);
>
> I guess that we can also test that the waitid() succeds now, right?
Yes, we can use TST_RETRY_FUNC(waitid(P_PIDFD, pidfd, &info, WEXITED), 
TST_RETVAL_EQ0) in here.

Best Regards
Yang Xu
>
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	pidfd_open_supported();
>> +
>> +	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
>> +	if (TST_RET == -1) {
>> +		if (TST_ERR == EINVAL) {
>> +			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
>> +			return;
>> +		}
>> +		tst_brk(TFAIL | TTERRNO,
>> +			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
>> +	}
>> +}
>> +
>> +static struct tst_test test = {
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.setup = setup,
>> +	.test_all = run,
>> +};
>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code
  2022-02-09 14:19 ` Petr Vorel
@ 2022-02-10  1:50   ` xuyang2018.jy
  2022-02-10  3:41   ` [LTP] [PATCH v2 " Yang Xu
  1 sibling, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-02-10  1:50 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp@lists.linux.it

Hi Petr
> Hi Xu,
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
> Nice cleanup.
> +1 for Cyril's note to TST_EXP_FD_SILENT().
>
>> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
>> +static void setup(void)
>> +{
>> +	pidfd_open_supported();
>> +}
>> +
>>   static struct tst_test test = {
>> -	.min_kver = "5.3",
>> +	.setup = setup,
>>   	.test_all = run,
> why not just .setup = pidfd_open_supported; ?
Yes. will remove this wrapper.

Best Regards
Yang Xu
>
> ...
>
>> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
>> index 48470e5e1..f41afa2fc 100644
>> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
>> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> ...
>> +static void setup(void)
>> +{
>> +	pidfd_open_supported();
>> +}
>> +
> And here as well.
>
> Kind regards,
> Petr
>
>>   static struct tst_test test = {
>> -	.min_kver = "5.3",
>> +	.setup = setup,
>>   	.test_all = run,
>>   	.forks_child = 1,
>>   	.needs_checkpoints = 1,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [LTP] [PATCH v2 1/2] syscalls/pidfd_open: Simplify code
  2022-02-09 14:19 ` Petr Vorel
  2022-02-10  1:50   ` xuyang2018.jy
@ 2022-02-10  3:41   ` Yang Xu
  2022-02-10  3:41     ` [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
  1 sibling, 1 reply; 12+ messages in thread
From: Yang Xu @ 2022-02-10  3:41 UTC (permalink / raw)
  To: ltp

1) make use of TST_EXP_FAIL2 or TST_EXP_FD_SILENT macros
2) remove min_kver check and use pidfd_open_supported()
3) Add docparse formatting

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/pidfd_open.h                     |  8 +++--
 .../kernel/syscalls/pidfd_open/pidfd_open01.c | 17 +++++-----
 .../kernel/syscalls/pidfd_open/pidfd_open02.c | 34 +++++++------------
 .../kernel/syscalls/pidfd_open/pidfd_open03.c | 11 +++---
 4 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
index 9806c73d4..5cf10933e 100644
--- a/include/lapi/pidfd_open.h
+++ b/include/lapi/pidfd_open.h
@@ -9,11 +9,15 @@
 
 #include <sys/syscall.h>
 #include <sys/types.h>
-
 #include "lapi/syscalls.h"
-
 #include "config.h"
 
+static inline void pidfd_open_supported(void)
+{
+	/* allow the tests to fail early */
+	tst_syscall(__NR_pidfd_open);
+}
+
 #ifndef HAVE_PIDFD_OPEN
 static inline int pidfd_open(pid_t pid, unsigned int flags)
 {
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
index f40e9b624..c0e88647f 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
@@ -1,11 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
  *
- * Description:
  * Basic pidfd_open() test:
- * 1) Fetch the PID of the current process and try to get its file descriptor.
- * 2) Check that the close-on-exec flag is set on the file descriptor.
+ *
+ * - Fetch the PID of the current process and try to get its file descriptor.
+ * - Check that the close-on-exec flag is set on the file descriptor.
  */
 
 #include <unistd.h>
@@ -17,10 +21,7 @@ static void run(void)
 {
 	int flag;
 
-	TEST(pidfd_open(getpid(), 0));
-
-	if (TST_RET == -1)
-		tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed");
+	TST_EXP_FD_SILENT(pidfd_open(getpid(), 0), "pidfd_open(getpid(), 0)");
 
 	flag = fcntl(TST_RET, F_GETFD);
 
@@ -36,6 +37,6 @@ static void run(void)
 }
 
 static struct tst_test test = {
-	.min_kver = "5.3",
+	.setup = pidfd_open_supported,
 	.test_all = run,
 };
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
index dc86cae7a..93a61a51d 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
@@ -1,14 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
+ *
+ * Tests basic error handling of the pidfd_open syscall.
  *
- * Description:
- * Basic pidfd_open() test to test invalid arguments.
+ * - ESRCH the process specified by pid does not exist
+ * - EINVAL pid is not valid
+ * - EINVAL flags is not valid
  */
 #include "tst_test.h"
 #include "lapi/pidfd_open.h"
 
-pid_t expired_pid, my_pid, invalid_pid = -1;
+static pid_t expired_pid, my_pid, invalid_pid = -1;
 
 static struct tcase {
 	char *name;
@@ -23,6 +30,7 @@ static struct tcase {
 
 static void setup(void)
 {
+	pidfd_open_supported();
 	expired_pid = tst_get_unused_pid();
 	my_pid = getpid();
 }
@@ -31,27 +39,11 @@ static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
-	TEST(pidfd_open(*tc->pid, tc->flags));
-
-	if (TST_RET != -1) {
-		SAFE_CLOSE(TST_RET);
-		tst_res(TFAIL, "%s: pidfd_open succeeded unexpectedly (index: %d)",
-			tc->name, n);
-		return;
-	}
-
-	if (tc->exp_errno != TST_ERR) {
-		tst_res(TFAIL | TTERRNO, "%s: pidfd_open() should fail with %s",
-			tc->name, tst_strerrno(tc->exp_errno));
-		return;
-	}
-
-	tst_res(TPASS | TTERRNO, "%s: pidfd_open() failed as expected",
-		tc->name);
+	TST_EXP_FAIL2(pidfd_open(*tc->pid, tc->flags), tc->exp_errno,
+			"pidfd_open with %s", tc->name);
 }
 
 static struct tst_test test = {
-	.min_kver = "5.3",
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = run,
 	.setup = setup,
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
index 48470e5e1..7c7c75cb1 100644
--- a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+/*\
+ * [Description]
  *
- * Description:
  * This program opens the PID file descriptor of the child process created with
  * fork(). It then uses poll to monitor the file descriptor for process exit, as
  * indicated by an EPOLLIN event.
@@ -27,11 +30,9 @@ static void run(void)
 		exit(EXIT_SUCCESS);
 	}
 
-	TEST(pidfd_open(pid, 0));
+	TST_EXP_FD_SILENT(pidfd_open(pid, 0), "pidfd_open(%d, 0)", pid);
 
 	fd = TST_RET;
-	if (fd == -1)
-		tst_brk(TFAIL | TTERRNO, "pidfd_open() failed");
 
 	TST_CHECKPOINT_WAKE(0);
 
@@ -50,7 +51,7 @@ static void run(void)
 }
 
 static struct tst_test test = {
-	.min_kver = "5.3",
+	.setup = pidfd_open_supported,
 	.test_all = run,
 	.forks_child = 1,
 	.needs_checkpoints = 1,
-- 
2.23.0


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-10  3:41   ` [LTP] [PATCH v2 " Yang Xu
@ 2022-02-10  3:41     ` Yang Xu
  2022-02-10 15:04       ` Cyril Hrubis
  0 siblings, 1 reply; 12+ messages in thread
From: Yang Xu @ 2022-02-10  3:41 UTC (permalink / raw)
  To: ltp

As pidfd_open man-page said
"PIDFD_NONBLOCK (since Linux 5.10)
Return a nonblocking file descriptor.  If the process referred to by
the file descriptor has not yet terminated, then an attempt to wait
on the file descriptor using waitid(2) will immediately  return
the error EAGAIN rather than blocking."

Test this and also test whether set NONBLOCK flag in its pidfd.

Noticed that, don't introduce lapi/pidfd.h because linux/pidfd.h uses
kernel header fcntl.h but ltp api uses libc header. so it may
exist redefinition error of 'struct flock'[1].

[1]https://github.com/golang/go/issues/48221

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                              |  1 +
 .../kernel/syscalls/pidfd_open/.gitignore     |  1 +
 .../kernel/syscalls/pidfd_open/pidfd_open04.c | 90 +++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index c3e037f72..ce6f89f88 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -954,6 +954,7 @@ personality02 personality02
 pidfd_open01 pidfd_open01
 pidfd_open02 pidfd_open02
 pidfd_open03 pidfd_open03
+pidfd_open04 pidfd_open04
 
 pidfd_send_signal01 pidfd_send_signal01
 pidfd_send_signal02 pidfd_send_signal02
diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
index e0b8900c1..cebdc624d 100644
--- a/testcases/kernel/syscalls/pidfd_open/.gitignore
+++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
@@ -1,3 +1,4 @@
 pidfd_open01
 pidfd_open02
 pidfd_open03
+pidfd_open04
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
new file mode 100644
index 000000000..91c7f26e1
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open04.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
+ * that waitid() with a non-blocking pidfd returns EAGAIN.
+ */
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#include <stdlib.h>
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+
+#ifndef PIDFD_NONBLOCK
+#define PIDFD_NONBLOCK O_NONBLOCK
+#endif
+
+#ifndef P_PIDFD
+#define P_PIDFD  3
+#endif
+
+static void run(void)
+{
+	int flag, pid, pidfd, ret;
+	siginfo_t info;
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		TST_CHECKPOINT_WAIT(0);
+		exit(EXIT_SUCCESS);
+	}
+
+	TST_EXP_FD_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
+				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);
+
+	pidfd = TST_RET;
+	flag = fcntl(pidfd, F_GETFL);
+	if (flag == -1)
+		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
+
+	if (!(flag & O_NONBLOCK))
+		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
+
+	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
+
+	TST_EXP_FAIL(waitid(P_PIDFD, pidfd, &info, WEXITED), EAGAIN,
+			"waitid(P_PIDFD,...,WEXITED)");
+
+	TST_CHECKPOINT_WAKE(0);
+
+	ret = TST_RETRY_FUNC(waitid(P_PIDFD, pidfd, &info, WEXITED), TST_RETVAL_EQ0);
+	if (ret == 0) {
+		tst_res(TPASS, "waitid(P_PIDFD) succeeded after child process terminated");
+	} else {
+		tst_res(TFAIL, "waitid(P_PIDFD) failed after child process terminated");
+		SAFE_WAIT(NULL);
+	}
+
+	SAFE_CLOSE(pidfd);
+}
+
+static void setup(void)
+{
+	pidfd_open_supported();
+
+	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
+	if (TST_RET == -1) {
+		if (TST_ERR == EINVAL) {
+			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
+			return;
+		}
+		tst_brk(TFAIL | TTERRNO,
+			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
+	}
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+	.setup = setup,
+	.test_all = run,
+};
-- 
2.23.0


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-10  3:41     ` [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
@ 2022-02-10 15:04       ` Cyril Hrubis
  2022-02-11  2:00         ` xuyang2018.jy
  0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2022-02-10 15:04 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi!
> +/*\
> + * [Description]
> + *
> + * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
> + * that waitid() with a non-blocking pidfd returns EAGAIN.
> + */
> +
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +
> +#ifndef PIDFD_NONBLOCK
> +#define PIDFD_NONBLOCK O_NONBLOCK
> +#endif
> +
> +#ifndef P_PIDFD
> +#define P_PIDFD  3
> +#endif
> +
> +static void run(void)
> +{
> +	int flag, pid, pidfd, ret;
> +	siginfo_t info;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		TST_CHECKPOINT_WAIT(0);
> +		exit(EXIT_SUCCESS);
> +	}
> +
> +	TST_EXP_FD_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
> +				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);
> +
> +	pidfd = TST_RET;
> +	flag = fcntl(pidfd, F_GETFL);
> +	if (flag == -1)
> +		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
> +
> +	if (!(flag & O_NONBLOCK))
> +		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
> +
> +	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
> +
> +	TST_EXP_FAIL(waitid(P_PIDFD, pidfd, &info, WEXITED), EAGAIN,
> +			"waitid(P_PIDFD,...,WEXITED)");
> +
> +	TST_CHECKPOINT_WAKE(0);
> +
> +	ret = TST_RETRY_FUNC(waitid(P_PIDFD, pidfd, &info, WEXITED), TST_RETVAL_EQ0);
> +	if (ret == 0) {
> +		tst_res(TPASS, "waitid(P_PIDFD) succeeded after child process terminated");
> +	} else {
> +		tst_res(TFAIL, "waitid(P_PIDFD) failed after child process terminated");
> +		SAFE_WAIT(NULL);
> +	}
> +
> +	SAFE_CLOSE(pidfd);
> +}
> +
> +static void setup(void)
> +{
> +	pidfd_open_supported();
> +
> +	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
> +	if (TST_RET == -1) {
> +		if (TST_ERR == EINVAL) {
> +			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
> +			return;
> +		}
> +		tst_brk(TFAIL | TTERRNO,
> +			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
> +	}

I guess that we should do SAFE_CLOSE(TST_RET); here. Otherwise:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.needs_checkpoints = 1,
> +	.setup = setup,
> +	.test_all = run,
> +};
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag
  2022-02-10 15:04       ` Cyril Hrubis
@ 2022-02-11  2:00         ` xuyang2018.jy
  0 siblings, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-02-11  2:00 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp@lists.linux.it

Hi Cyril
> Hi!
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that the PIDFD_NONBLOCK flag works with pidfd_open() and
>> + * that waitid() with a non-blocking pidfd returns EAGAIN.
>> + */
>> +
>> +#include<unistd.h>
>> +#include<fcntl.h>
>> +#include<sys/wait.h>
>> +#include<stdlib.h>
>> +#include "tst_test.h"
>> +#include "lapi/pidfd_open.h"
>> +
>> +#ifndef PIDFD_NONBLOCK
>> +#define PIDFD_NONBLOCK O_NONBLOCK
>> +#endif
>> +
>> +#ifndef P_PIDFD
>> +#define P_PIDFD  3
>> +#endif
>> +
>> +static void run(void)
>> +{
>> +	int flag, pid, pidfd, ret;
>> +	siginfo_t info;
>> +
>> +	pid = SAFE_FORK();
>> +	if (!pid) {
>> +		TST_CHECKPOINT_WAIT(0);
>> +		exit(EXIT_SUCCESS);
>> +	}
>> +
>> +	TST_EXP_FD_SILENT(pidfd_open(pid, PIDFD_NONBLOCK),
>> +				"pidfd_open(%d,  PIDFD_NONBLOCK)", pid);
>> +
>> +	pidfd = TST_RET;
>> +	flag = fcntl(pidfd, F_GETFL);
>> +	if (flag == -1)
>> +		tst_brk(TFAIL | TERRNO, "fcntl(F_GETFL) failed");
>> +
>> +	if (!(flag&  O_NONBLOCK))
>> +		tst_brk(TFAIL, "pidfd_open(%d, O_NONBLOCK) didn't set O_NONBLOCK flag", pid);
>> +
>> +	tst_res(TPASS, "pidfd_open(%d, O_NONBLOCK) sets O_NONBLOCK flag", pid);
>> +
>> +	TST_EXP_FAIL(waitid(P_PIDFD, pidfd,&info, WEXITED), EAGAIN,
>> +			"waitid(P_PIDFD,...,WEXITED)");
>> +
>> +	TST_CHECKPOINT_WAKE(0);
>> +
>> +	ret = TST_RETRY_FUNC(waitid(P_PIDFD, pidfd,&info, WEXITED), TST_RETVAL_EQ0);
>> +	if (ret == 0) {
>> +		tst_res(TPASS, "waitid(P_PIDFD) succeeded after child process terminated");
>> +	} else {
>> +		tst_res(TFAIL, "waitid(P_PIDFD) failed after child process terminated");
>> +		SAFE_WAIT(NULL);
>> +	}
>> +
>> +	SAFE_CLOSE(pidfd);
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	pidfd_open_supported();
>> +
>> +	TEST(pidfd_open(getpid(), PIDFD_NONBLOCK));
>> +	if (TST_RET == -1) {
>> +		if (TST_ERR == EINVAL) {
>> +			tst_brk(TCONF, "PIDFD_NONBLOCK was supported since linux 5.10");
>> +			return;
>> +		}
>> +		tst_brk(TFAIL | TTERRNO,
>> +			"pidfd_open(getpid(),PIDFD_NONBLOCK) failed unexpectedly");
>> +	}
>
> I guess that we should do SAFE_CLOSE(TST_RET); here. Otherwise:
>
> Reviewed-by: Cyril Hrubis<chrubis@suse.cz>
Thanks for your review, I add this and push this patchset.

Best Regards
Yang Xu
>
>> +}
>> +
>> +static struct tst_test test = {
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.needs_checkpoints = 1,
>> +	.setup = setup,
>> +	.test_all = run,
>> +};
>> --
>> 2.23.0
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-02-11  2:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-09  9:42 [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code Yang Xu
2022-02-09  9:42 ` [LTP] [PATCH 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
2022-02-09 13:47   ` Cyril Hrubis
2022-02-10  1:49     ` xuyang2018.jy
2022-02-09 10:00 ` [LTP] [PATCH 1/2] syscalls/pidfd_open: Simplify code xuyang2018.jy
2022-02-09 13:23 ` Cyril Hrubis
2022-02-09 14:19 ` Petr Vorel
2022-02-10  1:50   ` xuyang2018.jy
2022-02-10  3:41   ` [LTP] [PATCH v2 " Yang Xu
2022-02-10  3:41     ` [LTP] [PATCH v2 2/2] syscalls/pidfd_open04: Add new test with PIDFD_NONBLOCK flag Yang Xu
2022-02-10 15:04       ` Cyril Hrubis
2022-02-11  2:00         ` xuyang2018.jy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox