diff mbox series

[v2,2/2] syscalls/mkdir09: rewrite in new LTP API

Message ID c59ce5a253c3c8022fe465027c9310c29827ffa6.1643980361.git.jstancek@redhat.com
State Accepted, archived
Headers show
Series mkdir09: rewrite in new LTP API | expand

Commit Message

Jan Stancek Feb. 4, 2022, 1:14 p.m. UTC
'jump' variable is not initialized, which I suspect is behind
rare failures of this test. The original test is using longjmp,
processes and signals to sychronize couple processes that
exercise mkdir/rmdir calls.

Rewrite it using threads and new LTP API, drop all parameters,
because no runtest is using them, and make new default test time
just 1 second.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/mkdir/Makefile  |   2 +
 testcases/kernel/syscalls/mkdir/mkdir09.c | 494 ++++------------------
 2 files changed, 93 insertions(+), 403 deletions(-)

Comments

Li Wang Feb. 5, 2022, 4:19 a.m. UTC | #1
Jan Stancek <jstancek@redhat.com> wrote:


> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
>
> +#define MNTPOINT       "mntpoint"
>  #define MODE_RWX       07770
>


> +#define DIR_NAME       MNTPOINT "/X.%d"
> +#define DIR_NAME_GROUP MNTPOINT "/X.%d.%d"
> +#define NCHILD         3
>


>  static int child_groups = 2;
> -static int test_time = 5;
> +static int test_time = 1;
>  static int nfiles = 5;
>

I think initialize 'nfiles' to 10 will be better.

As we can see the test3 starts from 'j=2' and only creates
one directory for testing, but the comment says many
directories. Also, it will test more dirs in test1 and test2.

Otherwise, this patch looks pretty good.
Reviewed-by: Li Wang <liwang@redhat.com>
Cyril Hrubis Feb. 7, 2022, 12:55 p.m. UTC | #2
Hi!
> +/*
> + * Child routine which attempts to remove directories from the
> + * test directory which do not exist.
> + */
> +static void test2(int child_num)
>  {
> -	/* Child routine which attempts to remove directories from the
> -	 * test directory which do not exist. Runs until a SIGTERM
> -	 * signal is received. Exits with an error if the proper
> -	 * error is not detected or if the remove operation is
> -	 * successful.
> -	 */
>  

This empty line here looks a bit strange and the same for the test3().
Other than that and the suggestion from Li it looks good.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Jan Stancek Feb. 7, 2022, 3:04 p.m. UTC | #3
On Mon, Feb 7, 2022 at 1:53 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > +/*
> > + * Child routine which attempts to remove directories from the
> > + * test directory which do not exist.
> > + */
> > +static void test2(int child_num)
> >  {
> > -     /* Child routine which attempts to remove directories from the
> > -      * test directory which do not exist. Runs until a SIGTERM
> > -      * signal is received. Exits with an error if the proper
> > -      * error is not detected or if the remove operation is
> > -      * successful.
> > -      */
> >
>
> This empty line here looks a bit strange and the same for the test3().
> Other than that and the suggestion from Li it looks good.
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thanks, pushed with suggested changes.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/mkdir/Makefile b/testcases/kernel/syscalls/mkdir/Makefile
index 044619fb8724..881b087c6c1a 100644
--- a/testcases/kernel/syscalls/mkdir/Makefile
+++ b/testcases/kernel/syscalls/mkdir/Makefile
@@ -3,6 +3,8 @@ 
 
 top_srcdir		?= ../../../..
 
+mkdir09: CFLAGS += -pthread
+
 include $(top_srcdir)/include/mk/testcases.mk
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mkdir/mkdir09.c b/testcases/kernel/syscalls/mkdir/mkdir09.c
index 88034d29ed89..a2870ae94df8 100644
--- a/testcases/kernel/syscalls/mkdir/mkdir09.c
+++ b/testcases/kernel/syscalls/mkdir/mkdir09.c
@@ -1,460 +1,148 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2002
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines Corp., 2001
+ * Copyright (c) Linux Test Project, 2022
  */
 
-/* 06/30/2001	Port to Linux	nsharoff@us.ibm.com */
-/* 10/30/2002	Port to LTP	dbarrera@us.ibm.com */
-
-/*
- * Stress test of mkdir call.
- *
- * ALGORITHM
- *	Create multiple processes which create subdirectories in the
- *	same directory multiple times. On exit of all child processes,
- *	make sure all subdirectories can be removed.
- *
- *      USAGE: mkdir09 -c # -t # -d #
- *              -c = number of children groups
- *              -t = number of seconds to run test
- *              -d = number of directories created in test directory
+/*\
+ * [Description]
  *
+ * Create multiple processes which create subdirectories in the
+ * same directory multiple times within test time.
  */
 
 #include <stdio.h>
-#include <sys/wait.h>
-#include <sys/types.h>
 #include <sys/param.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-#include <setjmp.h>
-#include "test.h"
-
-#include <stdlib.h>
-#include <stdlib.h>
-#include <string.h>
-
-#define NCHILD		3
+#include "tst_test.h"
+#include "tst_safe_pthread.h"
 
+#define MNTPOINT	"mntpoint"
 #define MODE_RWX	07770
-#define DIR_NAME	"./X.%d"
-
-char *TCID = "mkdir09";
-int TST_TOTAL = 1;
-
-char testdir[MAXPATHLEN];
-int parent_pid, sigchld, sigterm, jump;
-void term(int sig);
-void chld(int sig);
-int *pidlist, child_count;
-jmp_buf env_buf;
-
-int getchild(int group, int child, int children);
-int dochild1(void);
-int dochild2(void);
-int dochild3(int group);
-int massmurder(void);
-int runtest(void);
-void setup(void);
-void cleanup(void);
+#define DIR_NAME	MNTPOINT "/X.%d"
+#define DIR_NAME_GROUP	MNTPOINT "/X.%d.%d"
+#define NCHILD		3
 
 static int child_groups = 2;
-static int test_time = 5;
+static int test_time = 1;
 static int nfiles = 5;
+static volatile int done;
 
-static char *opt_child_groups;
-static char *opt_test_time;
-static char *opt_nfiles;
-
-static option_t options[] = {
-	{"c:", NULL, &opt_child_groups},
-	{"t:", NULL, &opt_test_time},
-	{"d:", NULL, &opt_nfiles},
-	{NULL, NULL, NULL}
-};
-
-static void usage(void)
-{
-	printf("  -c      Child groups\n");
-	printf("  -t      Test runtime\n");
-	printf("  -d      Directories\n");
-}
-
-int main(int argc, char *argv[])
-{
-	tst_parse_opts(argc, argv, options, usage);
-
-	if (opt_child_groups)
-		child_groups = atoi(opt_child_groups);
-
-	if (opt_test_time)
-		test_time = atoi(opt_test_time);
-
-	if (opt_nfiles)
-		nfiles = atoi(opt_nfiles);
-
-	setup();
-
-	if (signal(SIGTERM, term) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup,
-			 "Error setting up SIGTERM signal, ERRNO = %d", errno);
-
-	}
-
-	if (signal(SIGCHLD, chld) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup,
-			 "Error setting up SIGCHLD signal, ERRNO = %d", errno);
-
-	}
-
-	runtest();
-	cleanup();
-	tst_exit();
-}
-
-int runtest(void)
-{
-	int i, j;
-	int count, child, status;
-	char tmpdir[MAXPATHLEN];
-
-	/* Create permanent directories with holes in directory structure */
-
-	for (j = 0; j < nfiles; j++) {
-		sprintf(tmpdir, DIR_NAME, j);
-		TEST(mkdir(tmpdir, MODE_RWX));
-
-		if (TEST_RETURN < 0) {
-			tst_brkm(TFAIL, cleanup,
-				 "Error creating permanent directories, ERRNO = %d",
-				 TEST_ERRNO);
-		}
-		if ((j % NCHILD) != 0) {
-			if (rmdir(tmpdir) < 0) {
-				tst_brkm(TFAIL, cleanup,
-					 "Error removing directory, ERRNO = %d",
-					 errno);
-			}
-		}
-	}
-
-	parent_pid = getpid();
-
-	/* allocate space for list of child pid's */
-
-	if ((pidlist = malloc((child_groups * NCHILD) * sizeof(int))) ==
-	    NULL) {
-		tst_brkm(TWARN, NULL,
-			 "\tMalloc failed (may be OK if under stress)");
-	}
-
-	child_count = 0;
-	for (j = 0; j < child_groups; j++) {
-		for (i = 0; i < NCHILD; i++) {
-			getchild(j, i, child_count);
-			child_count++;
-		}
-	}
-
-	/* If signal already received, skip to cleanup */
-
-	if (!sigchld && !sigterm) {
-		if (test_time) {
-			/* To get out of sleep if signal caught */
-			if (!setjmp(env_buf)) {
-				jump++;
-				sleep(test_time);
-			}
-		} else {
-			pause();
-		}
-	}
-
-	/* Reset signals since we are about to clean-up and to avoid
-	 * problem with wait call *               $
-	 * */
-
-	if (signal(SIGTERM, SIG_IGN) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup,
-			 "Error resetting SIGTERM signal, ERRNO = %d", errno);
-	}
-	if (signal(SIGCHLD, SIG_DFL) == SIG_ERR) {
-		tst_brkm(TFAIL, cleanup,
-			 "Error resetting SIGCHLD signal, ERRNO = %d", errno);
-	}
-
-	if (test_time) {
-		sleep(test_time);
-	}
-
-	/* Clean up children */
-	massmurder();
-	/*
-	 * Watch children finish and show returns.
-	 */
-
-	count = 0;
-	while (1) {
-		if ((child = wait(&status)) > 0) {
-			if (status != 0) {
-				tst_brkm(TWARN,
-					 NULL,
-					 "\tChild{%d} exited status = %0x",
-					 child, status);
-			}
-			count++;
-		} else {
-			if (errno != EINTR) {
-				break;
-			}
-			tst_resm(TINFO, "\tSignal detected during wait");
-		}
-	}
-
-	/*
-	 * Make sure correct number of children exited.
-	 */
-
-	if (count != child_count) {
-		tst_resm(TWARN, "\tWrong number of children waited on!");
-		tst_brkm(TWARN, NULL, "\tSaw %d, expected %d", count,
-			 NCHILD);
-	}
-
-	/* Check for core file in test directory. */
-
-	if (access("core", 0) == 0) {
-		tst_brkm(TWARN, NULL, "\tCore file found in test directory.");
-	}
-
-	/* Remove expected files */
-
-	for (j = 0; j < nfiles; j += NCHILD) {
-		sprintf(tmpdir, DIR_NAME, j);
-		if (rmdir(tmpdir) < 0) {
-			tst_brkm(TWARN,
-				 NULL,
-				 "\tError removing expected directory, ERRNO = %d",
-				 errno);
-		}
-	}
-
-	tst_resm(TPASS, "PASS");
-
-	return 0;
-}
-
-int getchild(int group, int child, int children)
-{
-	int pid;
-
-	pid = FORK_OR_VFORK();
-
-	if (pid < 0) {
-
-		massmurder();	/* kill the kids */
-		tst_brkm(TBROK, cleanup,
-			 "\tFork failed (may be OK if under stress)");
-	} else if (pid == 0) {	/* child does this */
-		switch (children % NCHILD) {
-		case 0:
-			dochild1();	/* create existing directories */
-			break;	/* so lint won't complain */
-		case 1:
-			dochild2();	/* remove nonexistant directories */
-			break;
-		case 2:
-			dochild3(group);	/* create/delete directories */
-			break;
-		default:
-			tst_brkm(TFAIL, cleanup,
-				 "Test not inplemented for child %d", child);
-			exit(1);
-			break;
-		}
-		exit(1);	/* If child gets here, something wrong */
-	}
-	pidlist[children] = pid;
-	return 0;
-}
-
-void term(int sig)
-{
-	/* Routine to handle SIGTERM signal. */
-
-	if (parent_pid == getpid()) {
-		tst_brkm(TWARN, NULL, "\tsignal SIGTERM received by parent.");
-	}
-	sigterm++;
-	if (jump) {
-		longjmp(env_buf, 1);
-	}
-}
-
-void chld(int sig)
-{
-	/* Routine to handle SIGCHLD signal. */
-
-	sigchld++;
-	if (jump) {
-		longjmp(env_buf, 1);
-	}
-}
-
-int dochild1(void)
+/*
+ * Routine which attempts to create directories in the test
+ * directory that already exist.
+ */
+static void test1(int child_num)
 {
-	/* Child routine which attempts to create directories in the test
-	 * directory that already exist. Runs until a SIGTERM signal is
-	 * received. Will exit with an error if it is able to create the
-	 * directory or if the expected error is not received.
-	 */
-
 	int j;
 	char tmpdir[MAXPATHLEN];
 
-	while (!sigterm) {
+	while (!done) {
 		for (j = 0; j < nfiles; j += NCHILD) {
 			sprintf(tmpdir, DIR_NAME, j);
-			TEST(mkdir(tmpdir, MODE_RWX));
-
-			if (TEST_RETURN < 0) {
-
-				if (TEST_ERRNO != EEXIST) {
-					tst_brkm(TFAIL, cleanup,
-						 "MKDIR %s, errno = %d; Wrong error detected.",
-						 tmpdir, TEST_ERRNO);
-					exit(1);
-				}
-			} else {
-				tst_brkm(TFAIL, cleanup,
-					 "MKDIR %s succeded when it shoud have failed.",
-					 tmpdir);
-				exit(1);
-			}
+			TST_EXP_FAIL_SILENT(mkdir(tmpdir, MODE_RWX), EEXIST);
+			if (!TST_PASS)
+				break;
 		}
 	}
-	exit(0);
+	tst_res(TPASS, "[%d] create dirs that already exist", child_num);
 }
 
-int dochild2(void)
+/*
+ * Child routine which attempts to remove directories from the
+ * test directory which do not exist.
+ */
+static void test2(int child_num)
 {
-	/* Child routine which attempts to remove directories from the
-	 * test directory which do not exist. Runs until a SIGTERM
-	 * signal is received. Exits with an error if the proper
-	 * error is not detected or if the remove operation is
-	 * successful.
-	 */
 
 	int j;
 	char tmpdir[MAXPATHLEN];
 
-	while (!sigterm) {
+	while (!done) {
 		for (j = 1; j < nfiles; j += NCHILD) {
 			sprintf(tmpdir, DIR_NAME, j);
-			if (rmdir(tmpdir) < 0) {
-				if (errno != ENOENT) {
-					tst_brkm(TFAIL, cleanup,
-						 "RMDIR %s, errno = %d; Wrong error detected.",
-						 tmpdir, errno);
-					exit(1);
-				}
-			} else {
-				tst_brkm(TFAIL, cleanup,
-					 "RMDIR %s succeded when it should have failed.",
-					 tmpdir);
-				exit(1);
-			}
+			TST_EXP_FAIL_SILENT(rmdir(tmpdir), ENOENT);
+			if (!TST_PASS)
+				break;
 		}
 	}
-	exit(0);
-	return 0;
+	tst_res(TPASS, "[%d] remove dirs that do not exist", child_num);
 }
 
-int dochild3(int group)
+/*
+ * Child routine which creates and deletes directories in the
+ * test directory.
+ */
+static void test3(int child_num)
 {
-	/* Child routine which creates and deletes directories in the
-	 * test directory. Runs until a SIGTERM signal is received, then
-	 * cleans up and exits. Detects error if the expected condition
-	 * is not encountered.
-	 */
 
 	int j;
-
 	char tmpdir[MAXPATHLEN];
-	char tmp[MAXPATHLEN];
 
-	while (!sigterm) {
+	while (!done) {
 		for (j = 2; j < nfiles; j += NCHILD) {
-			strcpy(tmp, DIR_NAME);
-			strcat(tmp, ".%d");
-			sprintf(tmpdir, tmp, j, group);
-
-			TEST(mkdir(tmpdir, MODE_RWX));
-
-			if (TEST_RETURN < 0) {
-				tst_brkm(TFAIL, cleanup,
-					 "MKDIR %s, errno = %d; Wrong error detected.",
-					 tmpdir, TEST_ERRNO);
-				exit(1);
-			}
+			sprintf(tmpdir, DIR_NAME_GROUP, j, child_num / NCHILD);
+			TST_EXP_PASS_SILENT(mkdir(tmpdir, MODE_RWX));
+			if (!TST_PASS)
+				break;
 		}
 		for (j = 2; j < nfiles; j += NCHILD) {
-			strcpy(tmp, DIR_NAME);
-			strcat(tmp, ".%d");
-			sprintf(tmpdir, tmp, j, group);
-			if (rmdir(tmpdir) < 0) {
-				tst_brkm(TFAIL, cleanup,
-					 "RMDIR %s, errno = %d; Wrong error detected.",
-					 tmpdir, errno);
-				exit(1);
-			}
+			sprintf(tmpdir, DIR_NAME_GROUP, j, child_num / NCHILD);
+			TST_EXP_PASS_SILENT(rmdir(tmpdir));
+			if (!TST_PASS)
+				break;
 		}
 	}
-	exit(0);
+	tst_res(TPASS, "[%d] create/remove dirs", child_num);
 }
 
-int massmurder(void)
+static void *child_thread_func(void *arg)
 {
-	register int j;
-	for (j = 0; j < child_count; j++) {
-		if (pidlist[j] > 0) {
-			if (kill(pidlist[j], SIGTERM) < 0) {
-				tst_brkm(TFAIL, cleanup,
-					 "Error killing child %d, ERRNO = %d",
-					 j, errno);
-			}
-		}
-	}
-	return 0;
+	void (*tests[NCHILD])(int) = { test1, test2, test3 };
+	int child_num = (long)arg;
+
+	tests[child_num % NCHILD](child_num);
+
+	/* if any thread failed, make other finish as well */
+	done = 1;
+
+	return NULL;
 }
 
-void setup(void)
+static void verify_mkdir(void)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	pthread_t child_thread[NCHILD * child_groups];
+	long i;
+
+	done = 0;
+	for (i = 0; i < child_groups * NCHILD; i++) {
+		SAFE_PTHREAD_CREATE(&child_thread[i], NULL,
+			child_thread_func, (void *)i);
+	}
 
-	TEST_PAUSE;
+	sleep(test_time);
+	done = 1;
 
-	tst_tmpdir();
+	for (i = 0; i < child_groups * NCHILD; i++)
+		SAFE_PTHREAD_JOIN(child_thread[i], NULL);
 }
 
-void cleanup(void)
+static void setup(void)
 {
-	tst_rmdir();
+	int j;
+	char tmpdir[MAXPATHLEN];
+
+	for (j = 0; j < nfiles; j += NCHILD) {
+		sprintf(tmpdir, DIR_NAME, j);
+		SAFE_MKDIR(tmpdir, MODE_RWX);
+	}
 }
+
+static struct tst_test test = {
+	.test_all = verify_mkdir,
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.setup = setup,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+};