All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/3 v2] Cleanup and bugfix for getrusage03
@ 2021-06-17 10:55 Xie Ziyao
  2021-06-17 10:55 ` [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment Xie Ziyao
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Xie Ziyao @ 2021-06-17 10:55 UTC (permalink / raw)
  To: ltp

1. Cleanup and add tst_process_exit_wait() for tst_process_state.
2. Cleanup and bugfix for getrusage03.

Xie Ziyao (3):
  lib: tst_process_state: Use SPDX and format alignment
  lib: tst_process_state: Add tst_process_exit_wait()
  getrusage: Cleanup and bugfix for getrusage03

 include/tst_process_state.h                   |  35 +-
 lib/tst_process_state.c                       |  52 +--
 .../kernel/syscalls/getrusage/getrusage03.c   | 398 +++++-------------
 .../kernel/syscalls/getrusage/getrusage03.h   |  33 ++
 .../syscalls/getrusage/getrusage03_child.c    | 196 +++------
 5 files changed, 242 insertions(+), 472 deletions(-)
 create mode 100644 testcases/kernel/syscalls/getrusage/getrusage03.h

--
2.17.1


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

* [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment
  2021-06-17 10:55 [LTP] [PATCH 0/3 v2] Cleanup and bugfix for getrusage03 Xie Ziyao
@ 2021-06-17 10:55 ` Xie Ziyao
  2021-06-18 11:46   ` Cyril Hrubis
  2021-06-17 10:55 ` [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait() Xie Ziyao
  2021-06-17 10:55 ` [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03 Xie Ziyao
  2 siblings, 1 reply; 9+ messages in thread
From: Xie Ziyao @ 2021-06-17 10:55 UTC (permalink / raw)
  To: ltp

Use SPDX and format alignment.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
1. Move changes in license and whitespaces to a separate patch.

 include/tst_process_state.h | 23 +++++++++--------------
 lib/tst_process_state.c     | 31 +++++++------------------------
 2 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/include/tst_process_state.h b/include/tst_process_state.h
index c32aa58e6..32eaf46d9 100644
--- a/include/tst_process_state.h
+++ b/include/tst_process_state.h
@@ -2,13 +2,10 @@
  * Copyright (C) 2012-2014 Cyril Hrubis chrubis@suse.cz
  */

- /*
-
-   These functions helps you wait till a process with given pid changes state.
-   This is for example useful when you need to wait in parent until child
-   blocks.
-
-  */
+/*
+ * These functions helps you wait till a process with given pid changes state.
+ * This is for example useful when you need to wait in parent until child blocks.
+ */

 #ifndef TST_PROCESS_STATE__
 #define TST_PROCESS_STATE__
@@ -29,8 +26,7 @@
 #ifdef TST_TEST_H__

 #define TST_PROCESS_STATE_WAIT(pid, state, msec_timeout) \
-	tst_process_state_wait(__FILE__, __LINE__, NULL, \
-	                       (pid), (state), (msec_timeout))
+	tst_process_state_wait(__FILE__, __LINE__, NULL, (pid), (state), (msec_timeout))
 #else
 /*
  * The same as above but does not use tst_brkm() interface.
@@ -41,13 +37,12 @@
  */
 int tst_process_state_wait2(pid_t pid, const char state);

-# define TST_PROCESS_STATE_WAIT(cleanup_fn, pid, state) \
-	 tst_process_state_wait(__FILE__, __LINE__, (cleanup_fn), \
-	                        (pid), (state), 0)
+#define TST_PROCESS_STATE_WAIT(cleanup_fn, pid, state) \
+	tst_process_state_wait(__FILE__, __LINE__, (cleanup_fn), (pid), (state), 0)
 #endif

 int tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void), pid_t pid,
-			    const char state, unsigned int msec_timeout);
+			   void (*cleanup_fn)(void), pid_t pid,
+			   const char state, unsigned int msec_timeout);

 #endif /* TST_PROCESS_STATE__ */
diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
index 11790c947..2f42895b7 100644
--- a/lib/tst_process_state.c
+++ b/lib/tst_process_state.c
@@ -1,24 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2012-2014 Cyril Hrubis chrubis@suse.cz
- *
- * 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.
- *
- * Further, this software is distributed without any warranty that it is
- * free of the rightful claim of any third person regarding infringement
- * or the like.  Any license provided herein, whether implied or
- * otherwise, applies only to this software file.  Patent licenses, if
- * any, provided herein do not apply to combinations of this program with
- * other software, or any other product whatsoever.
- *
- * 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.
  */

 #include <stdio.h>
@@ -29,8 +11,8 @@
 #include "tst_process_state.h"

 int tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void), pid_t pid,
-			    const char state, unsigned int msec_timeout)
+			   void (*cleanup_fn)(void), pid_t pid,
+			   const char state, unsigned int msec_timeout)
 {
 	char proc_path[128], cur_state;
 	unsigned int msecs = 0;
@@ -39,7 +21,7 @@ int tst_process_state_wait(const char *file, const int lineno,

 	for (;;) {
 		safe_file_scanf(file, lineno, cleanup_fn, proc_path,
-		                "%*i %*s %c", &cur_state);
+				"%*i %*s %c", &cur_state);

 		if (state == cur_state)
 			break;
@@ -64,16 +46,17 @@ int tst_process_state_wait2(pid_t pid, const char state)

 	for (;;) {
 		FILE *f = fopen(proc_path, "r");
+
 		if (!f) {
 			fprintf(stderr, "Failed to open '%s': %s\n",
-			        proc_path, strerror(errno));
+				proc_path, strerror(errno));
 			return 1;
 		}

 		if (fscanf(f, "%*i %*s %c", &cur_state) != 1) {
 			fclose(f);
 			fprintf(stderr, "Failed to read '%s': %s\n",
-			        proc_path, strerror(errno));
+				proc_path, strerror(errno));
 			return 1;
 		}
 		fclose(f);
--
2.17.1


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

* [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait()
  2021-06-17 10:55 [LTP] [PATCH 0/3 v2] Cleanup and bugfix for getrusage03 Xie Ziyao
  2021-06-17 10:55 ` [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment Xie Ziyao
@ 2021-06-17 10:55 ` Xie Ziyao
  2021-06-18 11:53   ` Cyril Hrubis
  2021-06-17 10:55 ` [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03 Xie Ziyao
  2 siblings, 1 reply; 9+ messages in thread
From: Xie Ziyao @ 2021-06-17 10:55 UTC (permalink / raw)
  To: ltp

The tst_process_exit_wait() checks a given pid is present on the system.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
1. Instead of checking /proc/$PID, do kill(2) with signal == 0 to check
for the existence of a $PID.

 include/tst_process_state.h | 12 ++++++++++--
 lib/tst_process_state.c     | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/include/tst_process_state.h b/include/tst_process_state.h
index 32eaf46d9..457ff6056 100644
--- a/include/tst_process_state.h
+++ b/include/tst_process_state.h
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later
  * Copyright (C) 2012-2014 Cyril Hrubis chrubis@suse.cz
+ * Copyright (C) 2021 Xie Ziyao <xieziyao@huawei.com>
  */

 /*
@@ -12,6 +13,8 @@

 #include <unistd.h>

+#ifdef TST_TEST_H__
+
 /*
  * Waits for process state change.
  *
@@ -23,10 +26,14 @@
  * Z - zombie process
  * T - process is traced
  */
-#ifdef TST_TEST_H__
-
 #define TST_PROCESS_STATE_WAIT(pid, state, msec_timeout) \
 	tst_process_state_wait(__FILE__, __LINE__, NULL, (pid), (state), (msec_timeout))
+
+/*
+ * Check that a given pid is present on the system
+ */
+#define TST_PROCESS_EXIT_WAIT(pid, msec_timeout) \
+	tst_process_exit_wait((pid), (msec_timeout))
 #else
 /*
  * The same as above but does not use tst_brkm() interface.
@@ -44,5 +51,6 @@ int tst_process_state_wait2(pid_t pid, const char state);
 int tst_process_state_wait(const char *file, const int lineno,
 			   void (*cleanup_fn)(void), pid_t pid,
 			   const char state, unsigned int msec_timeout);
+int tst_process_exit_wait(pid_t pid, unsigned int msec_timeout);

 #endif /* TST_PROCESS_STATE__ */
diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
index 2f42895b7..08a9d0966 100644
--- a/lib/tst_process_state.c
+++ b/lib/tst_process_state.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2012-2014 Cyril Hrubis chrubis@suse.cz
+ * Copyright (c) 2021 Xie Ziyao <xieziyao@huawei.com>
  */

 #include <stdio.h>
@@ -67,3 +68,23 @@ int tst_process_state_wait2(pid_t pid, const char state)
 		usleep(10000);
 	}
 }
+
+int tst_process_exit_wait(pid_t pid, unsigned int msec_timeout)
+{
+	unsigned int msecs = 0;
+
+	for (;;) {
+		if (kill(pid, 0) && errno == ESRCH)
+			break;
+
+		usleep(1000);
+		msecs += 1;
+
+		if (msec_timeout && msecs >= msec_timeout) {
+			errno = ETIMEDOUT;
+			return 0;
+		}
+	}
+
+	return 1;
+}
--
2.17.1


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

* [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03
  2021-06-17 10:55 [LTP] [PATCH 0/3 v2] Cleanup and bugfix for getrusage03 Xie Ziyao
  2021-06-17 10:55 ` [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment Xie Ziyao
  2021-06-17 10:55 ` [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait() Xie Ziyao
@ 2021-06-17 10:55 ` Xie Ziyao
  2021-06-18 12:20   ` Cyril Hrubis
  2 siblings, 1 reply; 9+ messages in thread
From: Xie Ziyao @ 2021-06-17 10:55 UTC (permalink / raw)
  To: ltp

1. Remove fork_malloc(), which is covered by inherit_fork{1, 2}().
2. Double the value of DELTA_MAX for machines with 64k pages, e.g. ppc64le.
3. Convert getrusage03 to the new API.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
1. Report PASS/FAIL from child processes.
2. Print more details in output.
3. Cleanup and correct some format.

 .../kernel/syscalls/getrusage/getrusage03.c   | 398 +++++-------------
 .../kernel/syscalls/getrusage/getrusage03.h   |  33 ++
 .../syscalls/getrusage/getrusage03_child.c    | 196 +++------
 3 files changed, 195 insertions(+), 432 deletions(-)
 create mode 100644 testcases/kernel/syscalls/getrusage/getrusage03.h

diff --git a/testcases/kernel/syscalls/getrusage/getrusage03.c b/testcases/kernel/syscalls/getrusage/getrusage03.c
index 125acfc64..f72f908dc 100644
--- a/testcases/kernel/syscalls/getrusage/getrusage03.c
+++ b/testcases/kernel/syscalls/getrusage/getrusage03.c
@@ -1,353 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- * getrusage03 - test ru_maxrss behaviors in struct rusage
- *
- * This test program is backported from upstream commit:
- * 1f10206cf8e945220f7220a809d8bfc15c21f9a5, which fills ru_maxrss
- * value in struct rusage according to rss hiwater mark. To make sure
- * this feature works correctly, a series of tests are executed in
- * this program.
- *
  * Copyright (C) 2011  Red Hat, Inc.
- * 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.
+ * Copyright (C) 2021 Xie Ziyao <xieziyao@huawei.com>
+ */
+
+/*\
+ * [Description]
  *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
+ * Test ru_maxrss behaviors in struct rusage.
  *
- * 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.
+ * This test program is backported from upstream commit: 1f10206cf8e9, which
+ * fills ru_maxrss value in struct rusage according to rss hiwater mark. To
+ * make sure this feature works correctly, a series of tests are executed in
+ * this program.
  */
-#include <sys/types.h>
-#include <sys/mman.h>
-#include <sys/resource.h>
-#include <sys/time.h>
-#include <sys/wait.h>
-#include <unistd.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>

-#include "test.h"
-#include "safe_macros.h"
+#include <stdlib.h>
+#include <stdio.h>

-char *TCID = "getrusage03";
-int TST_TOTAL = 1;
+#include "tst_test.h"
+#include "getrusage03.h"

-#define DELTA_MAX	10240
+#define TESTBIN "getrusage03_child"

 static struct rusage ru;
 static long maxrss_init;
-static int retval, status;
-static pid_t pid;
-
-static void inherit_fork(void);
-static void inherit_fork2(void);
-static void fork_malloc(void);
-static void grandchild_maxrss(void);
-static void zombie(void);
-static void sig_ign(void);
-static void exec_without_fork(void);
-static void check_return(int status, char *pass_msg, char *fail_msg);
-static int is_in_delta(long value);
-static void consume(int mega);
-static void setup(void);
-static void cleanup(void);
-
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);

-	setup();
+static const char *const resource[] = {
+	TESTBIN,
+	NULL,
+};

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		tst_resm(TINFO, "allocate 100MB");
-		consume(100);
+static void inherit_fork1(void)
+{
+	SAFE_GETRUSAGE(RUSAGE_SELF, &ru);
+	maxrss_init = ru.ru_maxrss;

-		inherit_fork();
-		inherit_fork2();
-		fork_malloc();
-		grandchild_maxrss();
-		zombie();
-		sig_ign();
-		exec_without_fork();
-	}
-	cleanup();
-	tst_exit();
-}
+	if (!SAFE_FORK()) {
+		SAFE_GETRUSAGE(RUSAGE_SELF, &ru);

-/* Testcase #01: fork inherit
- * expect: initial.self ~= child.self */
-static void inherit_fork(void)
-{
-	tst_resm(TINFO, "Testcase #01: fork inherit");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-	tst_resm(TINFO, "initial.self = %ld", ru.ru_maxrss);
-
-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork #1");
-	case 0:
-		maxrss_init = ru.ru_maxrss;
-		SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-		tst_resm(TINFO, "child.self = %ld", ru.ru_maxrss);
-		exit(is_in_delta(maxrss_init - ru.ru_maxrss));
-	default:
-		break;
+		if (is_in_delta(maxrss_init - ru.ru_maxrss))
+			tst_res(TPASS, "initial.self ~= child.self");
+		else
+			tst_res(TFAIL, "child.self = %li, expected %li",
+				ru.ru_maxrss, maxrss_init);
+		exit(0);
 	}
-
-	SAFE_WAITPID(cleanup, pid, &status, WUNTRACED | WCONTINUED);
-	check_return(WEXITSTATUS(status), "initial.self ~= child.self",
-		     "initial.self !~= child.self");
+	tst_reap_children();
 }

-/* Testcase #02: fork inherit (cont.)
- * expect: initial.children ~= 100MB, child.children = 0 */
 static void inherit_fork2(void)
 {
-	tst_resm(TINFO, "Testcase #02: fork inherit(cont.)");
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);

-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
 	if (is_in_delta(ru.ru_maxrss - 102400))
-		tst_resm(TPASS, "initial.children ~= 100MB");
+		tst_res(TPASS, "initial.children ~= 100MB");
 	else
-		tst_resm(TFAIL, "initial.children !~= 100MB");
-
-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork #2");
-	case 0:
-		SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-		tst_resm(TINFO, "child.children = %ld", ru.ru_maxrss);
-		exit(ru.ru_maxrss == 0);
-	default:
-		break;
-	}
+		tst_res(TFAIL, "initial.children = %li, expected %i",
+			ru.ru_maxrss, 102400);

-	SAFE_WAITPID(cleanup, pid, &status, WUNTRACED | WCONTINUED);
-	check_return(WEXITSTATUS(status), "child.children == 0",
-		     "child.children != 0");
-}
+	if (!SAFE_FORK()) {
+		SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);

-/* Testcase #03: fork + malloc
- * expect: initial.self + 50MB ~= child.self */
-static void fork_malloc(void)
-{
-	tst_resm(TINFO, "Testcase #03: fork + malloc");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-	tst_resm(TINFO, "initial.self = %ld", ru.ru_maxrss);
-
-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork #3");
-	case 0:
-		maxrss_init = ru.ru_maxrss;
-		tst_resm(TINFO, "child allocate +50MB");
-		consume(50);
-		SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-		tst_resm(TINFO, "child.self = %ld", ru.ru_maxrss);
-		exit(is_in_delta(maxrss_init + 51200 - ru.ru_maxrss));
-	default:
-		break;
+		if (!ru.ru_maxrss)
+			tst_res(TPASS, "child.children == 0");
+		else
+			tst_res(TFAIL, "child.children = %li, expected %i",
+				ru.ru_maxrss, 0);
+		exit(0);
 	}
-
-	SAFE_WAITPID(cleanup, pid, &status, WUNTRACED | WCONTINUED);
-	check_return(WEXITSTATUS(status), "initial.self + 50MB ~= child.self",
-		     "initial.self + 50MB !~= child.self");
+	tst_reap_children();
 }

-/* Testcase #04: grandchild maxrss
- * expect: post_wait.children ~= 300MB */
 static void grandchild_maxrss(void)
 {
-	tst_resm(TINFO, "Testcase #04: grandchild maxrss");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
-
-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork #4");
-	case 0:
-		retval = system("getrusage03_child -g 300");
-		if ((WIFEXITED(retval) && WEXITSTATUS(retval) != 0))
-			tst_brkm(TBROK | TERRNO, cleanup, "system");
-		exit(0);
-	default:
-		break;
-	}
-
-	SAFE_WAITPID(cleanup, pid, &status, WUNTRACED | WCONTINUED);
-	if (WEXITSTATUS(status) != 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "child exit status is not 0");
+	if (!SAFE_FORK())
+		SAFE_EXECLP("getrusage03_child", "getrusage03_child",
+			    "grand_consume", "300", NULL);
+	tst_reap_children();
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);

-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "post_wait.children = %ld", ru.ru_maxrss);
 	if (is_in_delta(ru.ru_maxrss - 307200))
-		tst_resm(TPASS, "child.children ~= 300MB");
+		tst_res(TPASS, "child.children ~= 300MB");
 	else
-		tst_resm(TFAIL, "child.children !~= 300MB");
+		tst_res(TFAIL, "child.children = %li, expected %i",
+			ru.ru_maxrss, 307200);
 }

-/* Testcase #05: zombie
- * expect: initial ~= pre_wait, post_wait ~= 400MB */
 static void zombie(void)
 {
-	tst_resm(TINFO, "Testcase #05: zombie");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
 	maxrss_init = ru.ru_maxrss;

-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK, cleanup, "fork #5");
-	case 0:
-		retval = system("getrusage03_child -n 400");
-		if ((WIFEXITED(retval) && WEXITSTATUS(retval) != 0))
-			tst_brkm(TBROK | TERRNO, cleanup, "system");
-		exit(0);
-	default:
-		break;
-	}
+	pid_t pid = SAFE_FORK();

-	sleep(1);		/* children become zombie */
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "pre_wait.children = %ld", ru.ru_maxrss);
+	if (!pid)
+		SAFE_EXECLP("getrusage03_child", "getrusage03_child",
+			    "consume", "400", NULL);
+
+	TST_PROCESS_STATE_WAIT(pid, 'Z', 0);
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
 	if (is_in_delta(ru.ru_maxrss - maxrss_init))
-		tst_resm(TPASS, "initial.children ~= pre_wait.children");
+		tst_res(TPASS, "initial.children ~= pre_wait.children");
 	else
-		tst_resm(TFAIL, "initial.children !~= pre_wait.children");
-
-	SAFE_WAITPID(cleanup, pid, &status, WUNTRACED | WCONTINUED);
-	if (WEXITSTATUS(status) != 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "child exit status is not 0");
+		tst_res(TFAIL, "pre_wait.children = %li, expected %li",
+			ru.ru_maxrss, maxrss_init);

-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "post_wait.children = %ld", ru.ru_maxrss);
+	tst_reap_children();
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
 	if (is_in_delta(ru.ru_maxrss - 409600))
-		tst_resm(TPASS, "post_wait.children ~= 400MB");
+		tst_res(TPASS, "post_wait.children ~= 400MB");
 	else
-		tst_resm(TFAIL, "post_wait.children !~= 400MB");
+		tst_res(TFAIL, "post_wait.children = %li, expected %i",
+			ru.ru_maxrss, 409600);
 }

-/* Testcase #06: SIG_IGN
- * expect: initial ~= after_zombie */
 static void sig_ign(void)
 {
-	tst_resm(TINFO, "Testcase #06: SIG_IGN");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
-	signal(SIGCHLD, SIG_IGN);
+	SAFE_SIGNAL(SIGCHLD, SIG_IGN);
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
 	maxrss_init = ru.ru_maxrss;

-	switch (pid = fork()) {
-	case -1:
-		tst_brkm(TBROK, cleanup, "fork #6");
-	case 0:
-		retval = system("getrusage03_child -n 500");
-		if ((WIFEXITED(retval) && WEXITSTATUS(retval) != 0))
-			tst_brkm(TBROK | TERRNO, cleanup, "system");
-		exit(0);
-	default:
-		break;
-	}
+	pid_t pid = SAFE_FORK();
+
+	if (!pid)
+		SAFE_EXECLP("getrusage03_child", "getrusage03_child",
+			    "consume", "500", NULL);

-	sleep(1);		/* children become zombie */
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	tst_resm(TINFO, "after_zombie.children = %ld", ru.ru_maxrss);
+	TST_PROCESS_EXIT_WAIT(pid, 0);
+	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
 	if (is_in_delta(ru.ru_maxrss - maxrss_init))
-		tst_resm(TPASS, "initial.children ~= after_zombie.children");
+		tst_res(TPASS, "initial.children ~= after_zombie.children");
 	else
-		tst_resm(TFAIL, "initial.children !~= after_zombie.children");
-	signal(SIGCHLD, SIG_DFL);
-}
+		tst_res(TFAIL, "after_zombie.children = %li, expected %li",
+			ru.ru_maxrss, maxrss_init);

-/* Testcase #07: exec without fork
- * expect: initial ~= fork */
-static void exec_without_fork(void)
-{
-	char str_maxrss_self[BUFSIZ], str_maxrss_child[BUFSIZ];
-	long maxrss_self, maxrss_child;
-
-	tst_resm(TINFO, "Testcase #07: exec without fork");
-
-	SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-	maxrss_self = ru.ru_maxrss;
-	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-	maxrss_child = ru.ru_maxrss;
-	tst_resm(TINFO, "initial.self = %ld, initial.children = %ld",
-		 maxrss_self, maxrss_child);
-
-	sprintf(str_maxrss_self, "%ld", maxrss_self);
-	sprintf(str_maxrss_child, "%ld", maxrss_child);
-	if (execlp("getrusage03_child", "getrusage03_child", "-v",
-		   "-s", str_maxrss_self, "-l", str_maxrss_child, NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "execlp");
+	SAFE_SIGNAL(SIGCHLD, SIG_DFL);
 }

-static int is_in_delta(long value)
+static void inherit_exec(void)
 {
-	return (value >= -DELTA_MAX && value <= DELTA_MAX);
-}
+	if (!SAFE_FORK()) {
+		char str_maxrss_self[BUFSIZ], str_maxrss_child[BUFSIZ];

-static void check_return(int status, char *pass_msg, char *fail_msg)
-{
-	switch (status) {
-	case 1:
-		tst_resm(TPASS, "%s", pass_msg);
-		break;
-	case 0:
-		tst_resm(TFAIL, "%s", fail_msg);
-		break;
-	default:
-		tst_resm(TFAIL, "child exit status is %d", status);
-		break;
+		SAFE_GETRUSAGE(RUSAGE_SELF, &ru);
+		sprintf(str_maxrss_self, "%ld", ru.ru_maxrss);
+		SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
+		sprintf(str_maxrss_child, "%ld", ru.ru_maxrss);
+
+		SAFE_EXECLP("getrusage03_child", "getrusage03_child",
+			    "compare", str_maxrss_self, str_maxrss_child, NULL);
 	}
+	tst_reap_children();
 }

-static void consume(int mega)
-{
-	size_t sz;
-	void *ptr;
+void (*testfunc_list[])(void) = {
+	inherit_fork1, inherit_fork2, grandchild_maxrss,
+	zombie, sig_ign, inherit_exec
+};

-	sz = mega * 1024 * 1024;
-	ptr = SAFE_MALLOC(cleanup, sz);
-	memset(ptr, 0, sz);
+static void run(unsigned int i)
+{
+	testfunc_list[i]();
 }

 static void setup(void)
 {
-	/* Disable test if the version of the kernel is less than 2.6.32 */
-	if ((tst_kvercmp(2, 6, 32)) < 0) {
-		tst_resm(TCONF, "This ru_maxrss field is not supported");
-		tst_brkm(TCONF, NULL, "before kernel 2.6.32");
-	}
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	consume_mb(100);
 }

-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.forks_child = 1,
+	.child_needs_reinit = 1,
+	.resource_files = resource,
+	.min_kver = "2.6.32",
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "1f10206cf8e9"},
+	},
+	.setup = setup,
+	.test = run,
+	.tcnt = ARRAY_SIZE(testfunc_list),
+};
diff --git a/testcases/kernel/syscalls/getrusage/getrusage03.h b/testcases/kernel/syscalls/getrusage/getrusage03.h
new file mode 100644
index 000000000..f1bbe9be5
--- /dev/null
+++ b/testcases/kernel/syscalls/getrusage/getrusage03.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Copyright (C) 2011  Red Hat, Inc.
+ * Copyright (C) 2021 Xie Ziyao <xieziyao@huawei.com>
+ */
+
+#ifndef LTP_GETRUSAGE03_H
+#define LTP_GETRUSAGE03_H
+
+#include "tst_test.h"
+
+#define DELTA_MAX 20480
+
+static void consume_mb(int consume_nr)
+{
+	void *ptr;
+	size_t size;
+	unsigned long vmswap_size;
+
+	size = consume_nr * 1024 * 1024;
+	ptr = SAFE_MALLOC(size);
+	memset(ptr, 0, size);
+
+	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmSwap: %lu", &vmswap_size);
+	if (vmswap_size > 0)
+		tst_brk(TBROK, "VmSwap is not zero");
+}
+
+static int is_in_delta(long value)
+{
+	return (value >= -DELTA_MAX && value <= DELTA_MAX);
+}
+
+#endif //LTP_GETRUSAGE03_H
diff --git a/testcases/kernel/syscalls/getrusage/getrusage03_child.c b/testcases/kernel/syscalls/getrusage/getrusage03_child.c
index 972c38e4e..e8252084f 100644
--- a/testcases/kernel/syscalls/getrusage/getrusage03_child.c
+++ b/testcases/kernel/syscalls/getrusage/getrusage03_child.c
@@ -1,168 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- * getrusage03_child.c - a child program executed by getrusage03
- *
  * Copyright (C) 2011  Red Hat, Inc.
- * 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.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * 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.
+ * Copyright (C) 2021 Xie Ziyao <xieziyao@huawei.com>
  */
-#include <sys/types.h>
-#include <sys/resource.h>
-#include <sys/time.h>
-#include <sys/wait.h>
-#include <errno.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <limits.h>
-
-#include "test.h"
-#include "safe_macros.h"

-char *TCID = "getrusage03_child";
-int TST_TOTAL = 1;
-
-#define DELTA_MAX	10240
+/*\
+ * [Description]
+ *
+ * Child program executed by getrusage03.
+ */

-static int opt_consume, opt_grand, opt_show, opt_self, opt_child;
-static char *consume_str, *grand_consume_str, *self_str, *child_str;
+#define TST_NO_DEFAULT_MAIN

-option_t child_options[] = {
-	{"n:", &opt_consume, &consume_str},
-	{"g:", &opt_grand, &grand_consume_str},
-	{"v", &opt_show, NULL},
-	{"s:", &opt_self, &self_str},
-	{"l:", &opt_child, &child_str},
-	{NULL, NULL, NULL}
-};
+#include <stdlib.h>

-static void usage(void);
-static void consume(int mega);
-static void setup(void);
-static void cleanup(void);
+#include "tst_test.h"
+#include "getrusage03.h"

 int main(int argc, char *argv[])
 {
-	int lc;
+	if (argc < 3)
+		tst_brk(TFAIL, "argc is %d, expected more than two", argc);
+
 	pid_t pid;
-	long maxrss_self, maxrss_children, delta;
-	long consume_nr, grand_consume_nr, self_nr, child_nr;
 	struct rusage ru;
+	long maxrss_self, maxrss_children;
+	long consume_nr, grand_consume_nr, self_nr, child_nr;

-	tst_parse_opts(argc, argv, child_options, usage);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	tst_reinit();

-		if (opt_consume) {
-			consume_nr = SAFE_STRTOL(cleanup,
-						 consume_str, 0, LONG_MAX);
-			tst_resm(TINFO, "child allocate %ldMB", consume_nr);
-			consume(consume_nr);
-		}
+	if (!strcmp(argv[1], "consume")) {
+		consume_nr = SAFE_STRTOL(argv[2], 0, LONG_MAX);
+		consume_mb(consume_nr);
+	} else if (!strcmp(argv[1], "grand_consume")) {
+		grand_consume_nr = SAFE_STRTOL(argv[2], 0, LONG_MAX);

-		if (opt_grand) {
-			grand_consume_nr = SAFE_STRTOL(cleanup,
-						       grand_consume_str, 0,
-						       LONG_MAX);
-			tst_resm(TINFO, "grandchild allocate %ldMB",
-				 grand_consume_nr);
-			switch (pid = fork()) {
-			case -1:
-				tst_brkm(TBROK, cleanup, "fork");
-			case 0:
-				consume(grand_consume_nr);
-				exit(0);
-			default:
-				break;
-			}
-			while (waitpid(-1, &pid, WUNTRACED | WCONTINUED) > 0)
-				if (WEXITSTATUS(pid) != 0)
-					tst_brkm(TBROK | TERRNO, cleanup,
-						 "child exit status is not 0");
+		pid = fork();
+		if (pid == -1)
+			tst_brk(TBROK, "fork failed");
+		else if (pid == 0) {
+			consume_mb(grand_consume_nr);
+			exit(0);
 		}

-		if (opt_show) {
-			SAFE_GETRUSAGE(cleanup, RUSAGE_SELF, &ru);
-			maxrss_self = ru.ru_maxrss;
-			SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
-			maxrss_children = ru.ru_maxrss;
-			tst_resm(TINFO, "exec.self = %ld, exec.children = %ld",
-				 maxrss_self, maxrss_children);
-			if (opt_self) {
-				self_nr = SAFE_STRTOL(cleanup,
-						      self_str, 0, LONG_MAX);
-				delta = maxrss_self - self_nr;
-				if (delta >= -DELTA_MAX && delta <= DELTA_MAX)
-					tst_resm(TPASS,
-						 "initial.self ~= exec.self");
-				else
-					tst_resm(TFAIL,
-						 "initial.self !~= exec.self");
-			}
-			if (opt_child) {
-				child_nr = SAFE_STRTOL(cleanup,
-						       child_str, 0, LONG_MAX);
-				delta = maxrss_children - child_nr;
-				if (delta >= -DELTA_MAX && delta <= DELTA_MAX)
-					tst_resm(TPASS,
-						 "initial.children ~= exec.children");
-				else
-					tst_resm(TFAIL,
-						 "initial.children !~= exec.children");
-			}
-		}
+		tst_reap_children();
+	} else if (!strcmp(argv[1], "compare")) {
+		self_nr = SAFE_STRTOL(argv[2], 0, LONG_MAX);
+		child_nr = SAFE_STRTOL(argv[3], 0, LONG_MAX);
+
+		SAFE_GETRUSAGE(RUSAGE_SELF, &ru);
+		maxrss_self = ru.ru_maxrss;
+		SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
+		maxrss_children = ru.ru_maxrss;
+
+		if (is_in_delta(maxrss_self - self_nr))
+			tst_res(TPASS, "initial.self ~= exec.self");
+		else
+			tst_res(TFAIL, "initial.self !~= exec.self");
+
+		if (is_in_delta(maxrss_children - child_nr))
+			tst_res(TPASS, "initial.children ~= exec.children");
+		else
+			tst_res(TFAIL, "initial.children !~= exec.children");
 	}

-	cleanup();
-	tst_exit();
-}
-
-static void usage(void)
-{
-	printf("  -n NUM  consume NUM MB size\n");
-	printf("  -g NUM  grandchild consume NUM MB size\n");
-	printf("  -v      verbose mode, show rusage info\n");
-	printf("  -s NUM  compare rusage_self.maxrss with given NUM\n");
-	printf("  -l NUM  compare rusage_children.maxrss with given NUM\n");
-}
-
-static void consume(int mega)
-{
-	size_t sz;
-	void *ptr;
-
-	sz = mega * 1024 * 1024;
-	ptr = SAFE_MALLOC(cleanup, sz);
-	memset(ptr, 0, sz);
-}
-
-static void setup(void)
-{
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-}
-
-static void cleanup(void)
-{
+	return 0;
 }
--
2.17.1


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

* [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment
  2021-06-17 10:55 ` [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment Xie Ziyao
@ 2021-06-18 11:46   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2021-06-18 11:46 UTC (permalink / raw)
  To: ltp

Hi!
> ---
> v1->v2:
> 1. Move changes in license and whitespaces to a separate patch.
> 
>  include/tst_process_state.h | 23 +++++++++--------------
>  lib/tst_process_state.c     | 31 +++++++------------------------
>  2 files changed, 16 insertions(+), 38 deletions(-)
> 
> diff --git a/include/tst_process_state.h b/include/tst_process_state.h
> index c32aa58e6..32eaf46d9 100644
> --- a/include/tst_process_state.h
> +++ b/include/tst_process_state.h
> @@ -2,13 +2,10 @@
>   * Copyright (C) 2012-2014 Cyril Hrubis chrubis@suse.cz
>   */
> 
> - /*
> -
> -   These functions helps you wait till a process with given pid changes state.
> -   This is for example useful when you need to wait in parent until child
> -   blocks.
> -
> -  */
> +/*
> + * These functions helps you wait till a process with given pid changes state.
> + * This is for example useful when you need to wait in parent until child blocks.
> + */
> 
>  #ifndef TST_PROCESS_STATE__
>  #define TST_PROCESS_STATE__
> @@ -29,8 +26,7 @@
>  #ifdef TST_TEST_H__
> 
>  #define TST_PROCESS_STATE_WAIT(pid, state, msec_timeout) \
> -	tst_process_state_wait(__FILE__, __LINE__, NULL, \
> -	                       (pid), (state), (msec_timeout))
> +	tst_process_state_wait(__FILE__, __LINE__, NULL, (pid), (state), (msec_timeout))

This actually creates a line over 80 chars, so I guess that the proper
fix would be keeping the pid, state and timeout on a separate line but
using tabs instead of spaces for indentation.

>  #else
>  /*
>   * The same as above but does not use tst_brkm() interface.
> @@ -41,13 +37,12 @@
>   */
>  int tst_process_state_wait2(pid_t pid, const char state);
> 
> -# define TST_PROCESS_STATE_WAIT(cleanup_fn, pid, state) \
> -	 tst_process_state_wait(__FILE__, __LINE__, (cleanup_fn), \
> -	                        (pid), (state), 0)
> +#define TST_PROCESS_STATE_WAIT(cleanup_fn, pid, state) \
> +	tst_process_state_wait(__FILE__, __LINE__, (cleanup_fn), (pid), (state), 0)

And here as well.


Apart from these two minor issues the rest looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait()
  2021-06-17 10:55 ` [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait() Xie Ziyao
@ 2021-06-18 11:53   ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2021-06-18 11:53 UTC (permalink / raw)
  To: ltp

Hi!
Looks good.

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03
  2021-06-17 10:55 ` [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03 Xie Ziyao
@ 2021-06-18 12:20   ` Cyril Hrubis
  2021-06-18 13:16     ` xieziyao
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2021-06-18 12:20 UTC (permalink / raw)
  To: ltp

Hi!
>  static void sig_ign(void)
>  {
> -	tst_resm(TINFO, "Testcase #06: SIG_IGN");
> -
> -	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
> -	tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
> -	signal(SIGCHLD, SIG_IGN);
> +	SAFE_SIGNAL(SIGCHLD, SIG_IGN);
> +	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
>  	maxrss_init = ru.ru_maxrss;
> 
> -	switch (pid = fork()) {
> -	case -1:
> -		tst_brkm(TBROK, cleanup, "fork #6");
> -	case 0:
> -		retval = system("getrusage03_child -n 500");
> -		if ((WIFEXITED(retval) && WEXITSTATUS(retval) != 0))
> -			tst_brkm(TBROK | TERRNO, cleanup, "system");
> -		exit(0);
> -	default:
> -		break;
> -	}
> +	pid_t pid = SAFE_FORK();
> +
> +	if (!pid)
> +		SAFE_EXECLP("getrusage03_child", "getrusage03_child",
> +			    "consume", "500", NULL);
> 
> -	sleep(1);		/* children become zombie */
> -	SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
> -	tst_resm(TINFO, "after_zombie.children = %ld", ru.ru_maxrss);
> +	TST_PROCESS_EXIT_WAIT(pid, 0);
> +	SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
>  	if (is_in_delta(ru.ru_maxrss - maxrss_init))
> -		tst_resm(TPASS, "initial.children ~= after_zombie.children");
> +		tst_res(TPASS, "initial.children ~= after_zombie.children");
>  	else
> -		tst_resm(TFAIL, "initial.children !~= after_zombie.children");
> -	signal(SIGCHLD, SIG_DFL);
> -}
> +		tst_res(TFAIL, "after_zombie.children = %li, expected %li",
> +			ru.ru_maxrss, maxrss_init);

I guess that these messages could be better, techincally it's not a
zombie but rather ignored child, so I would change this to:

"initial.children ~= ignored_child.children"

Other than this the rest is good.

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


Also if you agree I can fix the two minor issues in the patchset before
merge, no need to send v3.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03
  2021-06-18 12:20   ` Cyril Hrubis
@ 2021-06-18 13:16     ` xieziyao
  2021-06-18 18:04       ` Cyril Hrubis
  0 siblings, 1 reply; 9+ messages in thread
From: xieziyao @ 2021-06-18 13:16 UTC (permalink / raw)
  To: ltp

Yes, please do.

Feel fine with your modification and thanks for your review.

Kind Regards,
Ziyao

________________________________

Xie Ziyao
Email: xieziyao@huawei.com<mailto:xieziyao@huawei.com>


From: Cyril Hrubis<chrubis@suse.cz<mailto:chrubis@suse.cz>>
To: xieziyao<xieziyao@huawei.com<mailto:xieziyao@huawei.com>>
Cc: ltp<ltp@lists.linux.it<mailto:ltp@lists.linux.it>>;mdoucha<mdoucha@suse.cz<mailto:mdoucha@suse.cz>>
Subject: Re: [LTP][PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03
Time: 2021-06-18 20:46:14

Hi!
>  static void sig_ign(void)
>  {
> -     tst_resm(TINFO, "Testcase #06: SIG_IGN");
> -
> -     SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
> -     tst_resm(TINFO, "initial.children = %ld", ru.ru_maxrss);
> -     signal(SIGCHLD, SIG_IGN);
> +     SAFE_SIGNAL(SIGCHLD, SIG_IGN);
> +     SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
>        maxrss_init = ru.ru_maxrss;
>
> -     switch (pid = fork()) {
> -     case -1:
> -             tst_brkm(TBROK, cleanup, "fork #6");
> -     case 0:
> -             retval = system("getrusage03_child -n 500");
> -             if ((WIFEXITED(retval) && WEXITSTATUS(retval) != 0))
> -                     tst_brkm(TBROK | TERRNO, cleanup, "system");
> -             exit(0);
> -     default:
> -             break;
> -     }
> +     pid_t pid = SAFE_FORK();
> +
> +     if (!pid)
> +             SAFE_EXECLP("getrusage03_child", "getrusage03_child",
> +                         "consume", "500", NULL);
>
> -     sleep(1);               /* children become zombie */
> -     SAFE_GETRUSAGE(cleanup, RUSAGE_CHILDREN, &ru);
> -     tst_resm(TINFO, "after_zombie.children = %ld", ru.ru_maxrss);
> +     TST_PROCESS_EXIT_WAIT(pid, 0);
> +     SAFE_GETRUSAGE(RUSAGE_CHILDREN, &ru);
>        if (is_in_delta(ru.ru_maxrss - maxrss_init))
> -             tst_resm(TPASS, "initial.children ~= after_zombie.children");
> +             tst_res(TPASS, "initial.children ~= after_zombie.children");
>        else
> -             tst_resm(TFAIL, "initial.children !~= after_zombie.children");
> -     signal(SIGCHLD, SIG_DFL);
> -}
> +             tst_res(TFAIL, "after_zombie.children = %li, expected %li",
> +                     ru.ru_maxrss, maxrss_init);

I guess that these messages could be better, techincally it's not a
zombie but rather ignored child, so I would change this to:

"initial.children ~= ignored_child.children"

Other than this the rest is good.

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


Also if you agree I can fix the two minor issues in the patchset before
merge, no need to send v3.

--
Cyril Hrubis
chrubis@suse.cz
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210618/2fbe3251/attachment.htm>

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

* [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03
  2021-06-18 13:16     ` xieziyao
@ 2021-06-18 18:04       ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2021-06-18 18:04 UTC (permalink / raw)
  To: ltp

Hi!
Pushed, thanks.

I had to add another fix for the -i parameter.

When the test was executed in a loop it was failing on subsequent
iterations because it expects that the initiali maxrss is 100MB.

So in order to fix that I had to change the run() function, now each
test runs in its own process and starts with a child that consumes
100MB.

Diff:

diff --git a/testcases/kernel/syscalls/getrusage/getrusage03.c b/testcases/kernel/syscalls/getrusage/getrusage03.c
index f72f908dc..ef2f62a18 100644
--- a/testcases/kernel/syscalls/getrusage/getrusage03.c
+++ b/testcases/kernel/syscalls/getrusage/getrusage03.c
@@ -161,12 +161,16 @@ void (*testfunc_list[])(void) = {

 static void run(unsigned int i)
 {
-       testfunc_list[i]();
-}
+       if (!SAFE_FORK()) {
+               if (!SAFE_FORK()) {
+                       consume_mb(100);
+                       exit(0);
+               }

-static void setup(void)
-{
-       consume_mb(100);
+               SAFE_WAIT(NULL);
+
+               testfunc_list[i]();
+       }
 }

 static struct tst_test test = {
@@ -177,7 +181,6 @@ static struct tst_test test = {
        .tags = (const struct tst_tag[]) {
                {"linux-git", "1f10206cf8e9"},
        },
-       .setup = setup,
        .test = run,
        .tcnt = ARRAY_SIZE(testfunc_list),
 };

-- 
Cyril Hrubis
chrubis@suse.cz

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

end of thread, other threads:[~2021-06-18 18:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 10:55 [LTP] [PATCH 0/3 v2] Cleanup and bugfix for getrusage03 Xie Ziyao
2021-06-17 10:55 ` [LTP] [PATCH 1/3 v2] lib: tst_process_state: Use SPDX and format alignment Xie Ziyao
2021-06-18 11:46   ` Cyril Hrubis
2021-06-17 10:55 ` [LTP] [PATCH 2/3 v2] lib: tst_process_state: Add tst_process_exit_wait() Xie Ziyao
2021-06-18 11:53   ` Cyril Hrubis
2021-06-17 10:55 ` [LTP] [PATCH 3/3 v2] getrusage: Cleanup and bugfix for getrusage03 Xie Ziyao
2021-06-18 12:20   ` Cyril Hrubis
2021-06-18 13:16     ` xieziyao
2021-06-18 18:04       ` Cyril Hrubis

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.