public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems
@ 2017-11-14  2:11 Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH v2 1/4] mm: mallocstress: use new library and safe macros Sandeep Patil
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Sandeep Patil @ 2017-11-14  2:11 UTC (permalink / raw)
  To: ltp

Following series of patches fixes the review comments from the v1.
Each difference annotated in the patch.

Sandeep Patil (4):
  mm: mallocstress: use new library and safe macros
  checkpoint: add TST_CHECKPOINT_INIT() for new library headers
  mm: mallocstress: use futexes instead of SysV semaphores
  lib: mkfs: use 'mke2fs' for formatting extN filesystems

 include/tst_checkpoint.h                    |   9 +
 include/tst_fs.h                            |  13 ++
 lib/tst_mkfs.c                              |  17 +-
 lib/tst_supported_fs_types.c                |  38 +++-
 testcases/kernel/mem/mtest07/mallocstress.c | 310 ++++++++--------------------
 5 files changed, 150 insertions(+), 237 deletions(-)

-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH v2 1/4] mm: mallocstress: use new library and safe macros
  2017-11-14  2:11 [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems Sandeep Patil
@ 2017-11-14  2:11 ` Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH 2/4] checkpoint: add TST_CHECKPOINT_INIT() for new library headers Sandeep Patil
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Sandeep Patil @ 2017-11-14  2:11 UTC (permalink / raw)
  To: ltp

Make necessary changes to the test to start using the new ltp library
api instead of being a standalong program.  Use appropriate SAFE_ macros
in the process to reduce the error check paths.

Signed-off-by: Sandeep Patil <sspatil@google.com>
---
v1->v2
------
- Refactored test to use new library as suggested by Jan and then use the appropriate
  safe macros that do not need cleanup functions


 testcases/kernel/mem/mtest07/mallocstress.c | 317 +++++++++-------------------
 1 file changed, 100 insertions(+), 217 deletions(-)

diff --git a/testcases/kernel/mem/mtest07/mallocstress.c b/testcases/kernel/mem/mtest07/mallocstress.c
index 9588fb495..0f495a6b3 100644
--- a/testcases/kernel/mem/mtest07/mallocstress.c
+++ b/testcases/kernel/mem/mtest07/mallocstress.c
@@ -73,36 +73,22 @@
 #include <sys/ipc.h>
 #include <sys/sem.h>
 
-#define MAXL    100		/* default number of loops to do malloc and free      */
-#define MAXT     60		/* default number of threads to create.               */
+#include "tst_test.h"
+#include "tst_safe_pthread.h"
 
-#ifdef DEBUG
-#define dprt(args)	printf args
-#else
-#define dprt(args)
-#endif
+/* Number of loops per-thread */
+#define NUM_LOOPS	100
 
-#define OPT_MISSING(prog, opt)   do{\
-			       fprintf(stderr, "%s: option -%c ", prog, opt); \
-                               fprintf(stderr, "requires an argument\n"); \
-                               usage(prog); \
-                                   } while (0)
-
-int num_loop = MAXL;		/* number of loops to perform                     */
-int semid;
+/* Number of threads to create */
+#define NUM_THREADS     60
 
 /* Define SPEW_SIGNALS to tickle thread_create bug (it fails if interrupted). */
 #define SPEW_SIGNALS
 
-/******************************************************************************/
-/*								 	      */
-/* Function:	my_yield						      */
-/*									      */
-/* Description:	Yield control to another thread.                              */
-/*              Generate a signal, too.                                       */
-/*									      */
-/******************************************************************************/
-static void my_yield()
+static pthread_t *thread_id;	/* Spawned thread */
+int semid;
+
+static void my_yield(void)
 {
 #ifdef SPEW_SIGNALS
 	/* usleep just happens to use signals in glibc at moment.
@@ -119,60 +105,32 @@ static void my_yield()
 #endif
 }
 
-/******************************************************************************/
-/*								 	      */
-/* Function:	usage							      */
-/*									      */
-/* Description:	Print the usage message.				      */
-/*									      */
-/* Input:	char *progname - name of this program                         */
-/*									      */
-/* Return:	exits with -1						      */
-/*									      */
-/******************************************************************************/
-static void usage(char *progname)
-{				/* name of this program                       */
-	fprintf(stderr,
-		"Usage: %s -d NUMDIR -f NUMFILES -h -t NUMTHRD\n"
-		"\t -h Help!\n"
-		"\t -l Number of loops:               Default: 1000\n"
-		"\t -t Number of threads to generate: Default: 30\n", progname);
-	exit(-1);
-}
-
-/******************************************************************************/
-/* Function:	allocate_free				                      */
-/*								              */
-/* Description:	This function does the allocation and free by calling malloc  */
-/*		and free fuctions. The size of the memory to be malloced is   */
-/*		determined by the caller of this function. The size can be    */
-/*		a number from the fibannoaci series, power of 2 or 3 or 5     */
-/*									      */
-/* Input:	int repeat - number of times the alloc/free is repeated.      */
-/*		int scheme  - 0 to 3; selects how fast memory size grows      */
-/*								              */
-/* Return:	1 on failure						      */
-/*		0 on success						      */
-/******************************************************************************/
-int allocate_free(int repeat,	/* number of times to repeat allocate/free    */
-		  int scheme)
-{				/* how fast to increase block size            */
+/*
+ * allocate_free() - Allocate and free test called per-thread
+ *
+ * @scheme: 0-3; selects how fast memory size grows
+ *
+ * This function does the allocation and free by calling malloc
+ * and free functions. The size of the memory to be malloced is
+ * determined by the caller of this function. The size can be
+ * a number from the fibannoaci series, power of 2 or 3 or 5
+ *
+ * Return:
+ *  0: success
+ *  1: failure
+ */
+int allocate_free(int scheme)
+{
 	int loop;
 	const int MAXPTRS = 50;	/* only 42 or so get used on 32 bit machine */
 
-	dprt(("pid[%d]: allocate_free: repeat %d, scheme %d\n", getpid(),
-	      repeat, scheme));
-
-	for (loop = 0; loop < repeat; loop++) {
-		size_t oldsize = 5;	/* remember size for fibannoci series     */
-		size_t size = sizeof(long);	/* size of next block in ptrs[]           */
-		long *ptrs[MAXPTRS];	/* the pointers allocated in this loop    */
-		int num_alloc;	/* number of elements in ptrs[] so far    */
+	for (loop = 0; loop < NUM_LOOPS; loop++) {
+		size_t oldsize = 5;
+		size_t size = sizeof(long);
+		long *ptrs[MAXPTRS];
+		int num_alloc;
 		int i;
 
-		dprt(("pid[%d]: allocate_free: loop %d of %d\n", getpid(), loop,
-		      repeat));
-
 		/* loop terminates in one of three ways:
 		 * 1. after MAXPTRS iterations
 		 * 2. if malloc fails
@@ -181,15 +139,11 @@ int allocate_free(int repeat,	/* number of times to repeat allocate/free    */
 		for (num_alloc = 0; num_alloc < MAXPTRS; num_alloc++) {
 			size_t newsize = 0;
 
-			dprt(("pid[%d]: loop %d/%d; num_alloc=%d; size=%u\n",
-			      getpid(), loop, repeat, num_alloc, size));
-
 			/* Malloc the next block */
 			ptrs[num_alloc] = malloc(size);
-			if (ptrs[num_alloc] == NULL) {
-				/* terminate loop if malloc couldn't give us the memory we asked for */
+			/* terminate loop if malloc fails */
+			if (!ptrs[num_alloc])
 				break;
-			}
 			ptrs[num_alloc][0] = num_alloc;
 
 			/* Increase size according to one of four schedules. */
@@ -219,10 +173,8 @@ int allocate_free(int repeat,	/* number of times to repeat allocate/free    */
 		}
 
 		for (i = 0; i < num_alloc; i++) {
-			dprt(("pid[%d]: freeing ptrs[i] %p\n", getpid(),
-			      ptrs[i]));
 			if (ptrs[i][0] != i) {
-				fprintf(stderr,
+				tst_res(TFAIL,
 					"pid[%d]: fail: bad sentinel value\n",
 					getpid());
 				return 1;
@@ -233,175 +185,106 @@ int allocate_free(int repeat,	/* number of times to repeat allocate/free    */
 
 		my_yield();
 	}
+
 	/* Success! */
 	return 0;
 }
 
-/******************************************************************************/
-/* Function:	alloc_mem				                      */
-/*								              */
-/* Description:	Decide how fast to increase block sizes, then call            */
-/*              allocate_free() to actually to the test.                      */
-/*								              */
-/* Input:	threadnum is the thread number, 0...N-1                       */
-/*              global num_loop is how many iterations to run                 */
-/*								              */
-/* Return:	pthread_exit -1	on failure				      */
-/*		pthread_exit  0 on success			              */
-/*								              */
-/******************************************************************************/
 void *alloc_mem(void *threadnum)
 {
 	struct sembuf sop[1];
+	int err;
+
+	/* waiting for other threads starting */
 	sop[0].sem_num = 0;
 	sop[0].sem_op = 0;
 	sop[0].sem_flg = 0;
-	/* waiting for other threads starting */
-	if (semop(semid, sop, 1) == -1) {
-		if (errno != EIDRM)
-			perror("semop");
+	TEST(semop(semid, sop, 1));
+	if (TEST_RETURN == -1 && TEST_ERRNO != EIDRM) {
+		tst_res(TBROK | TTERRNO,
+			"Thread [%d] failed to wait on semaphore",
+			(int)(uintptr_t)threadnum);
 		return (void *)-1;
 	}
 
 	/* thread N will use growth scheme N mod 4 */
-	int err = allocate_free(num_loop, ((uintptr_t) threadnum) % 4);
-	fprintf(stdout,
+	err = allocate_free(((uintptr_t)threadnum) % 4);
+	tst_res(TINFO,
 		"Thread [%d]: allocate_free() returned %d, %s.  Thread exiting.\n",
-		(int)(uintptr_t) threadnum, err,
+		(int)(uintptr_t)threadnum, err,
 		(err ? "failed" : "succeeded"));
 	return (void *)(uintptr_t) (err ? -1 : 0);
 }
 
-/******************************************************************************/
-/*								 	      */
-/* Function:	main							      */
-/*									      */
-/* Description:	This is the entry point to the program. This function will    */
-/*		parse the input arguments and set the values accordingly. If  */
-/*		no arguments (or desired) are provided default values are used*/
-/*		refer the usage function for the arguments that this program  */
-/*		takes. It also creates the threads which do most of the dirty */
-/*		work. If the threads exits with a value '0' the program exits */
-/*		with success '0' else it exits with failure '-1'.             */
-/*									      */
-/* Return:	-1 on failure						      */
-/*		 0 on success						      */
-/*									      */
-/******************************************************************************/
-int main(int argc,		/* number of input parameters                 */
-	 char **argv)
-{				/* pointer to the command line arguments.     */
-	int c;			/* command line options                       */
-	int num_thrd = MAXT;	/* number of threads to create                */
-	int thrd_ndx;		/* index into the array of thread ids         */
-	pthread_t *thrdid;	/* the threads                                */
-	extern int optopt;	/* options to the program                     */
+static void stress_malloc(unsigned int n)
+{
 	struct sembuf sop[1];
-	int ret = 0;
-
-	while ((c = getopt(argc, argv, "hl:t:")) != -1) {
-		switch (c) {
-		case 'h':
-			usage(argv[0]);
-			break;
-		case 'l':
-			if ((num_loop = atoi(optarg)) == 0)
-				OPT_MISSING(argv[0], optopt);
-			else if (num_loop < 1) {
-				fprintf(stdout,
-					"WARNING: bad argument. Using default\n");
-				num_loop = MAXL;
-			}
-			break;
-		case 't':
-			if ((num_thrd = atoi(optarg)) == 0)
-				OPT_MISSING(argv[0], optopt);
-			else if (num_thrd < 1) {
-				fprintf(stdout,
-					"WARNING: bad argument. Using default\n");
-				num_thrd = MAXT;
-			}
-			break;
-		default:
-			usage(argv[0]);
-			break;
+	int thread_index;
+
+	(void)n;
+	/* kick off all the waiting threads */
+	sop[0].sem_num = 0;
+	sop[0].sem_op = -1;
+	sop[0].sem_flg = 0;
+	TEST(semop(semid, sop, 1));
+	if (TEST_RETURN == -1)
+		tst_res(TBROK | TTERRNO, "Failed to wakeup all threads");
+
+	/* wait for all threads to finish */
+	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
+		void *status;
+
+		SAFE_PTHREAD_JOIN(thread_id[thread_index], &status);
+		if ((intptr_t)status != 0) {
+			tst_res(TFAIL, "thread [%d] - exited with errors\n",
+				thread_index);
 		}
 	}
 
-	dprt(("number of times to loop in the thread = %d\n", num_loop));
+	tst_res(TPASS, "malloc stress test finished successfully");
+}
 
-	thrdid = malloc(sizeof(pthread_t) * num_thrd);
-	if (thrdid == NULL) {
-		perror("main(): allocating space for thrdid[] malloc()");
-		return 1;
-	}
+static void setup(void)
+{
+	struct sembuf sop[1];
+	int thread_index;
+
+	thread_id = SAFE_MALLOC(sizeof(pthread_t) * NUM_THREADS);
 
 	semid = semget(IPC_PRIVATE, 1, IPC_CREAT | 0666);
-	if (semid < 0) {
-		perror("Semaphore creation failed  Reason:");
-	}
+	if (semid < 0)
+		tst_res(TBROK | TTERRNO, "Semaphore creation failed");
 
 	sop[0].sem_num = 0;
 	sop[0].sem_op = 1;
 	sop[0].sem_flg = 0;
-	if (semop(semid, sop, 1) == -1) {
-		perror("semop");
-		ret = -1;
-		goto out;
-	}
+	TEST(semop(semid, sop, 1));
+	if (TEST_RETURN == -1)
+		tst_res(TBROK | TTERRNO, "Failed to initialize semaphore");
 
-	for (thrd_ndx = 0; thrd_ndx < num_thrd; thrd_ndx++) {
-		if (pthread_create(&thrdid[thrd_ndx], NULL, alloc_mem,
-				   (void *)(uintptr_t) thrd_ndx)) {
-			int err = errno;
-			if (err == EINTR) {
-				fprintf(stderr,
-					"main(): pthread_create failed with EINTR!\n");
-				ret = -1;
-				goto out;
-			}
-			perror("main(): pthread_create()");
-			ret = -11;
-			goto out;
-		}
+	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
+		SAFE_PTHREAD_CREATE(&thread_id[thread_index], NULL, alloc_mem,
+				    (void *)(uintptr_t)thread_index);
 	}
-	my_yield();
+}
 
-	sop[0].sem_op = -1;
-	if (semop(semid, sop, 1) == -1) {
-		perror("semop");
-		ret = -1;
-		goto out;
+static void cleanup(void)
+{
+	if (semid > 0) {
+		TEST(semctl(semid, 0, IPC_RMID));
+		if (TEST_RETURN == -1)
+			tst_res(TWARN, "Failed to destroy semaphore");
 	}
 
-	for (thrd_ndx = 0; thrd_ndx < num_thrd; thrd_ndx++) {
-		void *th_status;	/* exit status of LWP */
-		if (pthread_join(thrdid[thrd_ndx], &th_status) != 0) {
-			perror("main(): pthread_join()");
-			ret = -1;
-			goto out;
-		} else {
-			if ((intptr_t) th_status != 0) {
-				fprintf(stderr,
-					"main(): thread [%d] - exited with errors\n",
-					thrd_ndx);
-				ret = -1;
-				goto out;
-			}
-			dprt(("main(): thread [%d]: exited without errors\n",
-			      thrd_ndx));
-		}
-		my_yield();
-	}
-	printf("main(): test passed.\n");
-out:
-	if (semctl(semid, 0, IPC_RMID) == -1) {
-		perror("semctl\n");
-		ret = -1;
+	if (thread_id) {
+		free(thread_id);
+		thread_id = NULL;
 	}
-	if (thrdid) {
-		free(thrdid);
-		thrdid = NULL;
-	}
-	exit(ret);
 }
+
+static struct tst_test test = {
+	.tcnt = 1,
+	.cleanup = cleanup,
+	.setup = setup,
+	.test = stress_malloc,
+};
-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH 2/4] checkpoint: add TST_CHECKPOINT_INIT() for new library headers
  2017-11-14  2:11 [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH v2 1/4] mm: mallocstress: use new library and safe macros Sandeep Patil
@ 2017-11-14  2:11 ` Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems Sandeep Patil
  3 siblings, 0 replies; 11+ messages in thread
From: Sandeep Patil @ 2017-11-14  2:11 UTC (permalink / raw)
  To: ltp

The macro was missing unlike the new library equivalents of old
TST_SAFE_CHECKPOINT_{WAIT/WAKE} macros. So, add it, so it can be used
immediately in mm/mallocstress test conversion to checkpoint API.

Signed-off-by: Sandeep Patil <sspatil@google.com>
---
 include/tst_checkpoint.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/tst_checkpoint.h b/include/tst_checkpoint.h
index d6f7607ab..f936c698e 100644
--- a/include/tst_checkpoint.h
+++ b/include/tst_checkpoint.h
@@ -20,6 +20,15 @@
 
 #include "tst_checkpoint_fn.h"
 
+/*
+ * Checkpoint initialization, must be done first.
+ *
+ * Note: .needs_tmpdir must be set for the tests using checkpoints
+ */
+
+#define TST_CHECKPOINT_INIT() \
+	tst_checkpoint_init(__FILE__, __LINE__, NULL)
+
 #define TST_CHECKPOINT_WAIT(id) \
         tst_safe_checkpoint_wait(__FILE__, __LINE__, NULL, id, 0);
 
-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores
  2017-11-14  2:11 [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH v2 1/4] mm: mallocstress: use new library and safe macros Sandeep Patil
  2017-11-14  2:11 ` [LTP] [PATCH 2/4] checkpoint: add TST_CHECKPOINT_INIT() for new library headers Sandeep Patil
@ 2017-11-14  2:11 ` Sandeep Patil
  2017-11-20 12:36   ` Jan Stancek
  2017-11-14  2:11 ` [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems Sandeep Patil
  3 siblings, 1 reply; 11+ messages in thread
From: Sandeep Patil @ 2017-11-14  2:11 UTC (permalink / raw)
  To: ltp

Start using TST_CHECKPOINT_WAIT/WAKE macros that are based on futexes
instead of SysV semaphore. This allows the test to build and run on an
Android system.

Signed-off-by: Sandeep Patil <sspatil@google.com>
---

v1->v2
------
 - use the correct SAFE_ macros (that don't require cleanup functions)

 testcases/kernel/mem/mtest07/mallocstress.c | 59 +++++++----------------------
 1 file changed, 14 insertions(+), 45 deletions(-)

diff --git a/testcases/kernel/mem/mtest07/mallocstress.c b/testcases/kernel/mem/mtest07/mallocstress.c
index 0f495a6b3..63df7c70e 100644
--- a/testcases/kernel/mem/mtest07/mallocstress.c
+++ b/testcases/kernel/mem/mtest07/mallocstress.c
@@ -192,20 +192,10 @@ int allocate_free(int scheme)
 
 void *alloc_mem(void *threadnum)
 {
-	struct sembuf sop[1];
 	int err;
 
 	/* waiting for other threads starting */
-	sop[0].sem_num = 0;
-	sop[0].sem_op = 0;
-	sop[0].sem_flg = 0;
-	TEST(semop(semid, sop, 1));
-	if (TEST_RETURN == -1 && TEST_ERRNO != EIDRM) {
-		tst_res(TBROK | TTERRNO,
-			"Thread [%d] failed to wait on semaphore",
-			(int)(uintptr_t)threadnum);
-		return (void *)-1;
-	}
+	TST_CHECKPOINT_WAIT(0);
 
 	/* thread N will use growth scheme N mod 4 */
 	err = allocate_free(((uintptr_t)threadnum) % 4);
@@ -216,19 +206,21 @@ void *alloc_mem(void *threadnum)
 	return (void *)(uintptr_t) (err ? -1 : 0);
 }
 
+static void cleanup(void)
+{
+	if (thread_id) {
+		free(thread_id);
+		thread_id = NULL;
+	}
+}
+
 static void stress_malloc(unsigned int n)
 {
-	struct sembuf sop[1];
 	int thread_index;
-
 	(void)n;
-	/* kick off all the waiting threads */
-	sop[0].sem_num = 0;
-	sop[0].sem_op = -1;
-	sop[0].sem_flg = 0;
-	TEST(semop(semid, sop, 1));
-	if (TEST_RETURN == -1)
-		tst_res(TBROK | TTERRNO, "Failed to wakeup all threads");
+
+	/* Wake up all threads */
+	TST_CHECKPOINT_WAKE2(0, NUM_THREADS);
 
 	/* wait for all threads to finish */
 	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
@@ -246,21 +238,11 @@ static void stress_malloc(unsigned int n)
 
 static void setup(void)
 {
-	struct sembuf sop[1];
 	int thread_index;
 
 	thread_id = SAFE_MALLOC(sizeof(pthread_t) * NUM_THREADS);
 
-	semid = semget(IPC_PRIVATE, 1, IPC_CREAT | 0666);
-	if (semid < 0)
-		tst_res(TBROK | TTERRNO, "Semaphore creation failed");
-
-	sop[0].sem_num = 0;
-	sop[0].sem_op = 1;
-	sop[0].sem_flg = 0;
-	TEST(semop(semid, sop, 1));
-	if (TEST_RETURN == -1)
-		tst_res(TBROK | TTERRNO, "Failed to initialize semaphore");
+	TST_CHECKPOINT_INIT();
 
 	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
 		SAFE_PTHREAD_CREATE(&thread_id[thread_index], NULL, alloc_mem,
@@ -268,22 +250,9 @@ static void setup(void)
 	}
 }
 
-static void cleanup(void)
-{
-	if (semid > 0) {
-		TEST(semctl(semid, 0, IPC_RMID));
-		if (TEST_RETURN == -1)
-			tst_res(TWARN, "Failed to destroy semaphore");
-	}
-
-	if (thread_id) {
-		free(thread_id);
-		thread_id = NULL;
-	}
-}
-
 static struct tst_test test = {
 	.tcnt = 1,
+	.needs_tmpdir = 1,
 	.cleanup = cleanup,
 	.setup = setup,
 	.test = stress_malloc,
-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems
  2017-11-14  2:11 [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems Sandeep Patil
                   ` (2 preceding siblings ...)
  2017-11-14  2:11 ` [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores Sandeep Patil
@ 2017-11-14  2:11 ` Sandeep Patil
  2017-12-04 16:24   ` Cyril Hrubis
  2017-12-04 16:52   ` Jan Stancek
  3 siblings, 2 replies; 11+ messages in thread
From: Sandeep Patil @ 2017-11-14  2:11 UTC (permalink / raw)
  To: ltp

Android does not have mkfs.extN symlinks and only extN filesystems
images can be created on the device. So, make sure we use 'mke2fs -t
<fs_type>' when being built for Android and filter out any non-ext
filesystem type requests from tst_mkfs_() when built for Android.

In order to do this, we add a new "tst_get_mkfs()" api in the library
that checks if the given filesystem is supported and automatically
fills in the formatting tool's name in the buffer if it exists.

syscalls/mmap16, syscalls/rename11 are the tests the tests currently
known to start working after this change on Android systems

Signed-off-by: Sandeep Patil <sspatil@google.com>
---

v1->v2
-------
- Add the new tst_get_mkfs() API and use it in tst_mkfs_()
- Limit the __ANDROID__ ifdef to tst_supported_fs_types only
- Always use mke2fs for extN filesystem formatting instead of mkfs.extN
  (This allows all the tests that do filesystem formatting to run on Android
   systems)

 include/tst_fs.h             | 13 +++++++++++++
 lib/tst_mkfs.c               | 17 +++++++++++++++--
 lib/tst_supported_fs_types.c | 38 ++++++++++++++++++++++++++++++++------
 3 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 0ad535c2b..ad6979a58 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -151,6 +151,19 @@ int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
  */
 const char **tst_get_supported_fs_types(void);
 
+/*
+ * Returns 1 if given filesystem is supported, 0 otherwise.
+ */
+int tst_is_supported_fs_type(const char *fs_type);
+
+/*
+ * Test for the existence of filesystem formatting tool (mkfs.<fs_type>)
+ * and return the name if found.
+ *
+ * Returns 0 on Success, 1 if the buffer was too small or the tool wasn't found.
+ */
+int tst_get_mkfs(const char *fs_type, char *buf, size_t buf_len);
+
 /*
  * Creates and writes to files on given path until write fails with ENOSPC
  */
diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 7385a939f..99ab2cac7 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -15,6 +15,8 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <limits.h>
+
 #include "test.h"
 #include "ltp_priv.h"
 #include "tst_mkfs.h"
@@ -27,7 +29,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 	       const char *const fs_opts[], const char *extra_opt)
 {
 	int i, pos = 1, ret;
-	char mkfs[64];
+	char mkfs[NAME_MAX];
 	const char *argv[OPTS_MAX] = {mkfs};
 	char fs_opts_str[1024] = "";
 
@@ -43,7 +45,18 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 		return;
 	}
 
-	snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
+	if (tst_get_mkfs(fs_type, mkfs, NAME_MAX))
+		tst_brkm(TCONF, cleanup_fn,
+			 "mkfs.%s not found for %s", fs_type, fs_type);
+
+	if (!strcmp(mkfs, "mke2fs")) {
+		/* insert '-t <fs_type> in arguments here */
+		argv[pos++] = "-t";
+		strcat(fs_opts_str, "-t ");
+		argv[pos++] = fs_type;
+		strcat(fs_opts_str, fs_type);
+		strcat(fs_opts_str, " ");
+	}
 
 	if (fs_opts) {
 		for (i = 0; fs_opts[i]; i++) {
diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
index a23b1ed52..a62dda0da 100644
--- a/lib/tst_supported_fs_types.c
+++ b/lib/tst_supported_fs_types.c
@@ -29,31 +29,43 @@ static const char *const fs_type_whitelist[] = {
 	"ext2",
 	"ext3",
 	"ext4",
+#ifndef __ANDROID__
 	"xfs",
 	"btrfs",
 	"vfat",
 	"exfat",
 	"ntfs",
+#endif
 	NULL
 };
 
 static const char *fs_types[ARRAY_SIZE(fs_type_whitelist)];
 
+static void to_mkfs(char *buf, const char *fs_type)
+{
+	/* Use mke2fs instead of the mkfs.extN symlinks
+	 * so the same code works for Android systems
+	 */
+	if (!strncmp(fs_type, "ext", 3))
+		strcpy(buf, "mke2fs");
+	else
+		sprintf(buf, "mkfs.%s >/dev/null 2>&1", fs_type);
+}
+
 static int has_mkfs(const char *fs_type)
 {
-	char buf[128];
+	char buf[NAME_MAX];
 	int ret;
 
-	sprintf(buf, "mkfs.%s >/dev/null 2>&1", fs_type);
-
+	to_mkfs(buf, fs_type);
 	ret = tst_system(buf);
-
 	if (WEXITSTATUS(ret) == 127) {
-		tst_res(TINFO, "mkfs.%s does not exist", fs_type);
+		tst_res(TINFO, "%s for formatting %s does not exist",
+			buf, fs_type);
 		return 0;
 	}
 
-	tst_res(TINFO, "mkfs.%s does exist", fs_type);
+	tst_res(TINFO, "%s for formatting %s does exist", buf, fs_type);
 	return 1;
 }
 
@@ -116,3 +128,17 @@ const char **tst_get_supported_fs_types(void)
 
 	return fs_types;
 }
+
+int tst_get_mkfs(const char *fs_type, char *buf, size_t buf_len)
+{
+	tst_res(TINFO, "Checking for mkfs - %s", fs_type);
+	if (!is_supported(fs_type))
+		return 1;
+
+	/* make sure we have big enough buffer */
+	if (buf_len <= strlen(fs_type) + 5)
+		return 1;
+
+	to_mkfs(buf, fs_type);
+	return 0;
+}
-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores
  2017-11-14  2:11 ` [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores Sandeep Patil
@ 2017-11-20 12:36   ` Jan Stancek
  2017-11-28 14:25     ` Sandeep Patil
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Stancek @ 2017-11-20 12:36 UTC (permalink / raw)
  To: ltp

mallocstress set pushed with following changes:
- 2/3 patch dropped, replaced with ".needs_checkpoints = 1"
- int semid made static and initialized to -1
- changed .test to .test_all
- moved semop/checkpoint to stress_malloc(), so that -i parameter works
- 3/3 patch rebased

Regards,
Jan

----- Original Message -----
> Start using TST_CHECKPOINT_WAIT/WAKE macros that are based on futexes
> instead of SysV semaphore. This allows the test to build and run on an
> Android system.
> 
> Signed-off-by: Sandeep Patil <sspatil@google.com>
> ---
> 
> v1->v2
> ------
>  - use the correct SAFE_ macros (that don't require cleanup functions)
> 
>  testcases/kernel/mem/mtest07/mallocstress.c | 59
>  +++++++----------------------
>  1 file changed, 14 insertions(+), 45 deletions(-)
> 
> diff --git a/testcases/kernel/mem/mtest07/mallocstress.c
> b/testcases/kernel/mem/mtest07/mallocstress.c
> index 0f495a6b3..63df7c70e 100644
> --- a/testcases/kernel/mem/mtest07/mallocstress.c
> +++ b/testcases/kernel/mem/mtest07/mallocstress.c
> @@ -192,20 +192,10 @@ int allocate_free(int scheme)
>  
>  void *alloc_mem(void *threadnum)
>  {
> -	struct sembuf sop[1];
>  	int err;
>  
>  	/* waiting for other threads starting */
> -	sop[0].sem_num = 0;
> -	sop[0].sem_op = 0;
> -	sop[0].sem_flg = 0;
> -	TEST(semop(semid, sop, 1));
> -	if (TEST_RETURN == -1 && TEST_ERRNO != EIDRM) {
> -		tst_res(TBROK | TTERRNO,
> -			"Thread [%d] failed to wait on semaphore",
> -			(int)(uintptr_t)threadnum);
> -		return (void *)-1;
> -	}
> +	TST_CHECKPOINT_WAIT(0);
>  
>  	/* thread N will use growth scheme N mod 4 */
>  	err = allocate_free(((uintptr_t)threadnum) % 4);
> @@ -216,19 +206,21 @@ void *alloc_mem(void *threadnum)
>  	return (void *)(uintptr_t) (err ? -1 : 0);
>  }
>  
> +static void cleanup(void)
> +{
> +	if (thread_id) {
> +		free(thread_id);
> +		thread_id = NULL;
> +	}
> +}
> +
>  static void stress_malloc(unsigned int n)
>  {
> -	struct sembuf sop[1];
>  	int thread_index;
> -
>  	(void)n;
> -	/* kick off all the waiting threads */
> -	sop[0].sem_num = 0;
> -	sop[0].sem_op = -1;
> -	sop[0].sem_flg = 0;
> -	TEST(semop(semid, sop, 1));
> -	if (TEST_RETURN == -1)
> -		tst_res(TBROK | TTERRNO, "Failed to wakeup all threads");
> +
> +	/* Wake up all threads */
> +	TST_CHECKPOINT_WAKE2(0, NUM_THREADS);
>  
>  	/* wait for all threads to finish */
>  	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
> @@ -246,21 +238,11 @@ static void stress_malloc(unsigned int n)
>  
>  static void setup(void)
>  {
> -	struct sembuf sop[1];
>  	int thread_index;
>  
>  	thread_id = SAFE_MALLOC(sizeof(pthread_t) * NUM_THREADS);
>  
> -	semid = semget(IPC_PRIVATE, 1, IPC_CREAT | 0666);
> -	if (semid < 0)
> -		tst_res(TBROK | TTERRNO, "Semaphore creation failed");
> -
> -	sop[0].sem_num = 0;
> -	sop[0].sem_op = 1;
> -	sop[0].sem_flg = 0;
> -	TEST(semop(semid, sop, 1));
> -	if (TEST_RETURN == -1)
> -		tst_res(TBROK | TTERRNO, "Failed to initialize semaphore");
> +	TST_CHECKPOINT_INIT();
>  
>  	for (thread_index = 0; thread_index < NUM_THREADS; thread_index++) {
>  		SAFE_PTHREAD_CREATE(&thread_id[thread_index], NULL, alloc_mem,
> @@ -268,22 +250,9 @@ static void setup(void)
>  	}
>  }
>  
> -static void cleanup(void)
> -{
> -	if (semid > 0) {
> -		TEST(semctl(semid, 0, IPC_RMID));
> -		if (TEST_RETURN == -1)
> -			tst_res(TWARN, "Failed to destroy semaphore");
> -	}
> -
> -	if (thread_id) {
> -		free(thread_id);
> -		thread_id = NULL;
> -	}
> -}
> -
>  static struct tst_test test = {
>  	.tcnt = 1,
> +	.needs_tmpdir = 1,
>  	.cleanup = cleanup,
>  	.setup = setup,
>  	.test = stress_malloc,
> --
> 2.15.0.448.gf294e3d99a-goog
> 
> 

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

* [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores
  2017-11-20 12:36   ` Jan Stancek
@ 2017-11-28 14:25     ` Sandeep Patil
  0 siblings, 0 replies; 11+ messages in thread
From: Sandeep Patil @ 2017-11-28 14:25 UTC (permalink / raw)
  To: ltp

On Mon, Nov 20, 2017 at 4:36 AM, Jan Stancek <jstancek@redhat.com> wrote:

> mallocstress set pushed with following changes:
> - 2/3 patch dropped, replaced with ".needs_checkpoints = 1"
>

Ah, thanks for this. I suspected I was missing something
obvious.

<snip>

- ssp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20171128/63c7c27a/attachment.html>

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

* [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems
  2017-11-14  2:11 ` [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems Sandeep Patil
@ 2017-12-04 16:24   ` Cyril Hrubis
  2018-02-22 23:11     ` Sandeep Patil
  2017-12-04 16:52   ` Jan Stancek
  1 sibling, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2017-12-04 16:24 UTC (permalink / raw)
  To: ltp

Hi!
> @@ -43,7 +45,18 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
>  		return;
>  	}
>  
> -	snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
> +	if (tst_get_mkfs(fs_type, mkfs, NAME_MAX))
> +		tst_brkm(TCONF, cleanup_fn,
> +			 "mkfs.%s not found for %s", fs_type, fs_type);
> +
> +	if (!strcmp(mkfs, "mke2fs")) {
> +		/* insert '-t <fs_type> in arguments here */
> +		argv[pos++] = "-t";
> +		strcat(fs_opts_str, "-t ");
> +		argv[pos++] = fs_type;
> +		strcat(fs_opts_str, fs_type);
> +		strcat(fs_opts_str, " ");
> +	}

Hmm, this is kind of ugly, why do we have tst_get_mkfs() function and
then this if that fills it's arguments.

Well what about we rather exported the tst_fs_is_supported() function
and used it here before we even try to assemble the command to format
the filesystem.

I would have done it as following:

Change the is_supported() to tst_fs_is_supported() what would check for
mke2fs as a fallback command for ext*. Then the whole logic related to
mkfs would go into the tst_mkfs() function, including again the fallback
to mke2fs command for ext*.

Or we can move the ttst_supported_fs_types.c code to tst_mkfs.c (in a
separate patch) in order to get rid of the artificial boundary between
the two modules. That should free us to draft better code without a need
to draft publicly exposed interface between these two modules.

>  	if (fs_opts) {
>  		for (i = 0; fs_opts[i]; i++) {
> diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> index a23b1ed52..a62dda0da 100644
> --- a/lib/tst_supported_fs_types.c
> +++ b/lib/tst_supported_fs_types.c
> @@ -29,31 +29,43 @@ static const char *const fs_type_whitelist[] = {
>  	"ext2",
>  	"ext3",
>  	"ext4",
> +#ifndef __ANDROID__
>  	"xfs",
>  	"btrfs",
>  	"vfat",
>  	"exfat",
>  	"ntfs",
> +#endif
>  	NULL
>  };

What is the exact reason for this ifdef? Shouldn't the code handle the
missing support gracefully even without it?

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems
  2017-11-14  2:11 ` [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems Sandeep Patil
  2017-12-04 16:24   ` Cyril Hrubis
@ 2017-12-04 16:52   ` Jan Stancek
  2017-12-06 12:16     ` Cyril Hrubis
  1 sibling, 1 reply; 11+ messages in thread
From: Jan Stancek @ 2017-12-04 16:52 UTC (permalink / raw)
  To: ltp

Cyril,

when you get a chance, can you please also look at this one?
It's the last unpushed patch from Sandeep's series, that
touches newly added all_filesystems.

Regards,
Jan

----- Original Message -----
Android does not have mkfs.extN symlinks and only extN filesystems
images can be created on the device. So, make sure we use 'mke2fs -t
<fs_type>' when being built for Android and filter out any non-ext
filesystem type requests from tst_mkfs_() when built for Android.

In order to do this, we add a new "tst_get_mkfs()" api in the library
that checks if the given filesystem is supported and automatically
fills in the formatting tool's name in the buffer if it exists.

syscalls/mmap16, syscalls/rename11 are the tests the tests currently
known to start working after this change on Android systems

Signed-off-by: Sandeep Patil <sspatil@google.com>
---

v1->v2
-------
- Add the new tst_get_mkfs() API and use it in tst_mkfs_()
- Limit the __ANDROID__ ifdef to tst_supported_fs_types only
- Always use mke2fs for extN filesystem formatting instead of mkfs.extN
  (This allows all the tests that do filesystem formatting to run on Android
   systems)

 include/tst_fs.h             | 13 +++++++++++++
 lib/tst_mkfs.c               | 17 +++++++++++++++--
 lib/tst_supported_fs_types.c | 38 ++++++++++++++++++++++++++++++++------
 3 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 0ad535c2b..ad6979a58 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -151,6 +151,19 @@ int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
  */
 const char **tst_get_supported_fs_types(void);
 
+/*
+ * Returns 1 if given filesystem is supported, 0 otherwise.
+ */
+int tst_is_supported_fs_type(const char *fs_type);
+
+/*
+ * Test for the existence of filesystem formatting tool (mkfs.<fs_type>)
+ * and return the name if found.
+ *
+ * Returns 0 on Success, 1 if the buffer was too small or the tool wasn't found.
+ */
+int tst_get_mkfs(const char *fs_type, char *buf, size_t buf_len);
+
 /*
  * Creates and writes to files on given path until write fails with ENOSPC
  */
diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 7385a939f..99ab2cac7 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -15,6 +15,8 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <limits.h>
+
 #include "test.h"
 #include "ltp_priv.h"
 #include "tst_mkfs.h"
@@ -27,7 +29,7 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 	       const char *const fs_opts[], const char *extra_opt)
 {
 	int i, pos = 1, ret;
-	char mkfs[64];
+	char mkfs[NAME_MAX];
 	const char *argv[OPTS_MAX] = {mkfs};
 	char fs_opts_str[1024] = "";
 
@@ -43,7 +45,18 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
 		return;
 	}
 
-	snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
+	if (tst_get_mkfs(fs_type, mkfs, NAME_MAX))
+		tst_brkm(TCONF, cleanup_fn,
+			 "mkfs.%s not found for %s", fs_type, fs_type);
+
+	if (!strcmp(mkfs, "mke2fs")) {
+		/* insert '-t <fs_type> in arguments here */
+		argv[pos++] = "-t";
+		strcat(fs_opts_str, "-t ");
+		argv[pos++] = fs_type;
+		strcat(fs_opts_str, fs_type);
+		strcat(fs_opts_str, " ");
+	}
 
 	if (fs_opts) {
 		for (i = 0; fs_opts[i]; i++) {
diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
index a23b1ed52..a62dda0da 100644
--- a/lib/tst_supported_fs_types.c
+++ b/lib/tst_supported_fs_types.c
@@ -29,31 +29,43 @@ static const char *const fs_type_whitelist[] = {
 	"ext2",
 	"ext3",
 	"ext4",
+#ifndef __ANDROID__
 	"xfs",
 	"btrfs",
 	"vfat",
 	"exfat",
 	"ntfs",
+#endif
 	NULL
 };
 
 static const char *fs_types[ARRAY_SIZE(fs_type_whitelist)];
 
+static void to_mkfs(char *buf, const char *fs_type)
+{
+	/* Use mke2fs instead of the mkfs.extN symlinks
+	 * so the same code works for Android systems
+	 */
+	if (!strncmp(fs_type, "ext", 3))
+		strcpy(buf, "mke2fs");
+	else
+		sprintf(buf, "mkfs.%s >/dev/null 2>&1", fs_type);
+}
+
 static int has_mkfs(const char *fs_type)
 {
-	char buf[128];
+	char buf[NAME_MAX];
 	int ret;
 
-	sprintf(buf, "mkfs.%s >/dev/null 2>&1", fs_type);
-
+	to_mkfs(buf, fs_type);
 	ret = tst_system(buf);
-
 	if (WEXITSTATUS(ret) == 127) {
-		tst_res(TINFO, "mkfs.%s does not exist", fs_type);
+		tst_res(TINFO, "%s for formatting %s does not exist",
+			buf, fs_type);
 		return 0;
 	}
 
-	tst_res(TINFO, "mkfs.%s does exist", fs_type);
+	tst_res(TINFO, "%s for formatting %s does exist", buf, fs_type);
 	return 1;
 }
 
@@ -116,3 +128,17 @@ const char **tst_get_supported_fs_types(void)
 
 	return fs_types;
 }
+
+int tst_get_mkfs(const char *fs_type, char *buf, size_t buf_len)
+{
+	tst_res(TINFO, "Checking for mkfs - %s", fs_type);
+	if (!is_supported(fs_type))
+		return 1;
+
+	/* make sure we have big enough buffer */
+	if (buf_len <= strlen(fs_type) + 5)
+		return 1;
+
+	to_mkfs(buf, fs_type);
+	return 0;
+}
-- 
2.15.0.448.gf294e3d99a-goog


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

* [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems
  2017-12-04 16:52   ` Jan Stancek
@ 2017-12-06 12:16     ` Cyril Hrubis
  0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2017-12-06 12:16 UTC (permalink / raw)
  To: ltp

Hi!
> when you get a chance, can you please also look at this one?
> It's the last unpushed patch from Sandeep's series, that
> touches newly added all_filesystems.

I've looked at it on Monday (and replied). And I think that merging the
tst_supported_fs.c into tst_mkfs.c first would be the best solution
because otherwise we end up with a need to export some kind of API
between these and I do not see a clean solution for that.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems
  2017-12-04 16:24   ` Cyril Hrubis
@ 2018-02-22 23:11     ` Sandeep Patil
  0 siblings, 0 replies; 11+ messages in thread
From: Sandeep Patil @ 2018-02-22 23:11 UTC (permalink / raw)
  To: ltp

On Mon, Dec 04, 2017 at 05:24:28PM +0100, Cyril Hrubis wrote:
> Hi!
> > @@ -43,7 +45,18 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
> >  		return;
> >  	}
> >  
> > -	snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
> > +	if (tst_get_mkfs(fs_type, mkfs, NAME_MAX))
> > +		tst_brkm(TCONF, cleanup_fn,
> > +			 "mkfs.%s not found for %s", fs_type, fs_type);
> > +
> > +	if (!strcmp(mkfs, "mke2fs")) {
> > +		/* insert '-t <fs_type> in arguments here */
> > +		argv[pos++] = "-t";
> > +		strcat(fs_opts_str, "-t ");
> > +		argv[pos++] = fs_type;
> > +		strcat(fs_opts_str, fs_type);
> > +		strcat(fs_opts_str, " ");
> > +	}
> 
> Hmm, this is kind of ugly, why do we have tst_get_mkfs() function and
> then this if that fills it's arguments.
> 
> Well what about we rather exported the tst_fs_is_supported() function
> and used it here before we even try to assemble the command to format
> the filesystem.
> 
> I would have done it as following:
> 
> Change the is_supported() to tst_fs_is_supported() what would check for
> mke2fs as a fallback command for ext*. Then the whole logic related to
> mkfs would go into the tst_mkfs() function, including again the fallback
> to mke2fs command for ext*.
> 
> Or we can move the ttst_supported_fs_types.c code to tst_mkfs.c (in a
> separate patch) in order to get rid of the artificial boundary between
> the two modules. That should free us to draft better code without a need
> to draft publicly exposed interface between these two modules.
> 
> >  	if (fs_opts) {
> >  		for (i = 0; fs_opts[i]; i++) {
> > diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> > index a23b1ed52..a62dda0da 100644
> > --- a/lib/tst_supported_fs_types.c
> > +++ b/lib/tst_supported_fs_types.c
> > @@ -29,31 +29,43 @@ static const char *const fs_type_whitelist[] = {
> >  	"ext2",
> >  	"ext3",
> >  	"ext4",
> > +#ifndef __ANDROID__
> >  	"xfs",
> >  	"btrfs",
> >  	"vfat",
> >  	"exfat",
> >  	"ntfs",
> > +#endif
> >  	NULL
> >  };
> 
> What is the exact reason for this ifdef? Shouldn't the code handle the
> missing support gracefully even without it?

(I know this is ridiculously late, but ..)

Just wanted to report that we simply solved all of this trouble by making
mkfs.ext[234] symlinks available in Android and we pass 'ext4' as the
filesystem of preference using LTP_DEV_FS_TYPE :-)

Thanks for looking into this.

- ssp

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

end of thread, other threads:[~2018-02-22 23:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-14  2:11 [LTP] [PATCH v2 0/4] Miscellaneous fixes for Android systems Sandeep Patil
2017-11-14  2:11 ` [LTP] [PATCH v2 1/4] mm: mallocstress: use new library and safe macros Sandeep Patil
2017-11-14  2:11 ` [LTP] [PATCH 2/4] checkpoint: add TST_CHECKPOINT_INIT() for new library headers Sandeep Patil
2017-11-14  2:11 ` [LTP] [PATCH v2 3/4] mm: mallocstress: use futexes instead of SysV semaphores Sandeep Patil
2017-11-20 12:36   ` Jan Stancek
2017-11-28 14:25     ` Sandeep Patil
2017-11-14  2:11 ` [LTP] [PATCH v2 4/4] lib: mkfs: use 'mke2fs' for formatting extN filesystems Sandeep Patil
2017-12-04 16:24   ` Cyril Hrubis
2018-02-22 23:11     ` Sandeep Patil
2017-12-04 16:52   ` Jan Stancek
2017-12-06 12:16     ` Cyril Hrubis

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