[LTP] [PATCH] hugemmap/hugemmap05.c: cleanup && fix warnings and failures

Xiao Yang yangx.jy@cn.fujitsu.com
Tue Aug 15 08:40:56 CEST 2017


1) take use of safe macros
2) add missing check_hugepage()
3) fix warnings on system which doesn't support hugepages
4) fix failures caused by hugemmap05 -m -i n

We can get the following warnings on RHEL5.11GA:
   hugemmap05.c:263: CONF: file /proc/sys/vm/nr_overcommit_hugepages does not exist in the system
   hugemmap05.c:209: INFO: restore nr_hugepages to .
   hugemmap05.c:217: WARN: open: ENOENT
   hugemmap05.c:219: INFO: restore nr_overcommit_hugepages to .
   hugemmap05.c:222: WARN: write: EBADF
   hugemmap05.c:228: WARN: umount: ENOENT

We can get the following failures when running hugemmap05 -m -i n:
   hugemmap05.c:191: PASS: hugepages overcommit test pass
   hugemmap05.c:118: INFO: check sysfs before allocation.
   hugemmap05.c:378: INFO: HugePages_Total is 192.
   hugemmap05.c:378: INFO: HugePages_Free is 0.
   hugemmap05.c:381: FAIL: HugePages_Free is not 192 but 0.
   hugemmap05.c:118: INFO: check sysfs before allocation.
   hugemmap05.c:378: INFO: HugePages_Total is 192.
   hugemmap05.c:378: INFO: HugePages_Free is 0.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c | 364 ++++++++-------------
 1 file changed, 134 insertions(+), 230 deletions(-)

diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
index 7be7efb..2764106 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap05.c
@@ -26,20 +26,23 @@
  * them by mmap or shmget.
  */
 
-#include <sys/mount.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
 #include "mem.h"
 #include "hugetlb.h"
-#include "tst_safe_stdio.h"
+#include "tst_safe_sysv_ipc.h"
+#include "tst_test.h"
 
 #define PROTECTION		(PROT_READ | PROT_WRITE)
 #define PATH_MEMINFO		"/proc/meminfo"
 
-char path_sys_sz[BUFSIZ];
-char path_sys_sz_over[BUFSIZ];
-char path_sys_sz_free[BUFSIZ];
-char path_sys_sz_resv[BUFSIZ];
-char path_sys_sz_surp[BUFSIZ];
-char path_sys_sz_huge[BUFSIZ];
+static char path_sys_sz[BUFSIZ];
+static char path_sys_sz_over[BUFSIZ];
+static char path_sys_sz_free[BUFSIZ];
+static char path_sys_sz_resv[BUFSIZ];
+static char path_sys_sz_surp[BUFSIZ];
+static char path_sys_sz_huge[BUFSIZ];
 
 #define PATH_PROC_VM		"/proc/sys/vm/"
 #define PATH_PROC_OVER		PATH_PROC_VM "nr_overcommit_hugepages"
@@ -61,12 +64,10 @@ char path_sys_sz_huge[BUFSIZ];
 #define SHM_HUGETLB 04000
 #endif
 
-static char nr_hugepages[BUFSIZ], nr_overcommit_hugepages[BUFSIZ];
-static char buf[BUFSIZ], line[BUFSIZ], path[BUFSIZ], pathover[BUFSIZ];
-static char shmmax[BUFSIZ];
-static int hugepagesize;	/* in Bytes */
-static int shmid = -1;
-static int restore_shmmax;
+static long shmmax, hugepagesize, nr_hugepages, nr_overcommit_hugepages;
+static char mount_dir[BUFSIZ], tst_file[BUFSIZ], path[BUFSIZ], pathover[BUFSIZ];
+static int key = -1, shmid = -1, fd = -1;
+static int mounted, restore_shmmax, restore_nr_hgpgs, restore_overcomm_hgpgs;
 static size_t size = 128, length = 384;
 
 char *opt_sysfs;
@@ -79,113 +80,93 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
-static void write_bytes(void *addr);
-static void read_bytes(void *addr);
-static int lookup(char *line, char *pattern);
-static int checkproc(FILE * fp, char *string, int value);
-static int checksys(char *path, char *pattern, int value);
+static void check_wr_bytes(void *addr);
+static int checkproc(long act_val, char *string, long exp_val);
+static int checksys(char *path, char *pattern, long exp_val);
 static void init_sys_sz_paths(void);
 
 static void test_overcommit(void)
 {
 	void *addr = NULL, *shmaddr = NULL;
-	int fd = -1, key = -1;
-	char s[BUFSIZ];
-	FILE *fp;
 
-	if (shmid != -1) {
-		/* Use /proc/meminfo to generate an IPC key. */
-		key = ftok(PATH_MEMINFO, strlen(PATH_MEMINFO));
-		if (key == -1)
-			tst_brk(TBROK | TERRNO, "ftok");
-		shmid = shmget(key, (long)(length / 2 * hugepagesize),
-			       SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
-		if (shmid == -1)
-			tst_brk(TBROK | TERRNO, "shmget");
+	if (opt_shmid) {
+		shmid = SAFE_SHMGET(key, (long)(length / 2 * hugepagesize),
+				    SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
 	} else {
-		/* XXX (garrcoop): memory leak. */
-		snprintf(s, BUFSIZ, "%s/hugemmap05/file", tst_get_tmpdir());
-		fd = SAFE_OPEN(s, O_CREAT | O_RDWR, 0755);
-		addr = mmap(ADDR, (long)(length / 2 * hugepagesize), PROTECTION,
-			    FLAGS, fd, 0);
-		if (addr == MAP_FAILED) {
-			close(fd);
-			tst_brk(TBROK | TERRNO, "mmap");
-		}
+		fd = SAFE_OPEN(tst_file, O_CREAT | O_RDWR, 0755);
+		addr = SAFE_MMAP(ADDR, (long)(length / 2 * hugepagesize),
+				 PROTECTION, FLAGS, fd, 0);
 	}
 
 	if (opt_sysfs) {
 		tst_res(TINFO, "check sysfs before allocation.");
-		if (checksys(path_sys_sz_huge, "HugePages_Total",
-			     length / 2) != 0)
+		if (checksys(path_sys_sz_huge, "HugePages_Total", length / 2))
 			return;
-		if (checksys(path_sys_sz_free, "HugePages_Free",
-			     length / 2) != 0)
+		if (checksys(path_sys_sz_free, "HugePages_Free", length / 2))
 			return;
 		if (checksys(path_sys_sz_surp, "HugePages_Surp",
-			     length / 2 - size) != 0)
+			     length / 2 - size))
 			return;
-		if (checksys(path_sys_sz_resv, "HugePages_Rsvd",
-			     length / 2) != 0)
+		if (checksys(path_sys_sz_resv, "HugePages_Rsvd", length / 2))
 			return;
 	} else {
 		tst_res(TINFO, "check /proc/meminfo before allocation.");
-		fp = SAFE_FOPEN(PATH_MEMINFO, "r");
-		if (checkproc(fp, "HugePages_Total", length / 2) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Total:"),
+			      "HugePages_Total", length / 2))
 			return;
-		if (checkproc(fp, "HugePages_Free", length / 2) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Free:"),
+			      "HugePages_Free", length / 2))
 			return;
-		if (checkproc(fp, "HugePages_Surp", length / 2 - size) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Surp:"),
+			      "HugePages_Surp", length / 2 - size))
 			return;
-		if (checkproc(fp, "HugePages_Rsvd", length / 2) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Rsvd:"),
+			      "HugePages_Rsvd", length / 2))
 			return;
-		fclose(fp);
 	}
-	if (shmid != -1) {
+
+	if (opt_shmid) {
 		tst_res(TINFO, "shmid: 0x%x", shmid);
-		shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS);
-		if (shmaddr == (void *)-1)
-			tst_brk(TBROK | TERRNO, "shmat");
-		write_bytes(shmaddr);
-		read_bytes(shmaddr);
+		shmaddr = SAFE_SHMAT(shmid, ADDR, SHMAT_FLAGS);
+		check_wr_bytes(shmaddr);
 	} else {
-		write_bytes(addr);
-		read_bytes(addr);
+		check_wr_bytes(addr);
 	}
+
 	if (opt_sysfs) {
 		tst_res(TINFO, "check sysfs.");
-		if (checksys(path_sys_sz_huge, "HugePages_Total",
-			     length / 2) != 0)
+		if (checksys(path_sys_sz_huge, "HugePages_Total", length / 2))
 			return;
-		if (checksys(path_sys_sz_free, "HugePages_Free", 0)
-		    != 0)
+		if (checksys(path_sys_sz_free, "HugePages_Free", 0))
 			return;
 		if (checksys(path_sys_sz_surp, "HugePages_Surp",
-			     length / 2 - size) != 0)
+			     length / 2 - size))
 			return;
-		if (checksys(path_sys_sz_resv, "HugePages_Rsvd", 0)
-		    != 0)
+		if (checksys(path_sys_sz_resv, "HugePages_Rsvd", 0))
 			return;
 	} else {
 		tst_res(TINFO, "check /proc/meminfo.");
-		fp = SAFE_FOPEN(PATH_MEMINFO, "r");
-		if (checkproc(fp, "HugePages_Total", length / 2) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Total:"),
+			      "HugePages_Total", length / 2))
 			return;
-		if (checkproc(fp, "HugePages_Free", 0) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Free:"),
+			      "HugePages_Free", 0))
 			return;
-		if (checkproc(fp, "HugePages_Surp", length / 2 - size) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Surp:"),
+			      "HugePages_Surp", length / 2 - size))
 			return;
-		if (checkproc(fp, "HugePages_Rsvd", 0) != 0)
+		if (checkproc(SAFE_READ_MEMINFO("HugePages_Rsvd:"),
+			      "HugePages_Rsvd", 0))
 			return;
-		fclose(fp);
 	}
-	if (shmid != -1) {
-		if (shmdt(shmaddr) != 0)
-			tst_brk(TBROK | TERRNO, "shmdt");
+
+	if (opt_shmid) {
+		SAFE_SHMDT(shmaddr);
+		SAFE_SHMCTL(shmid, IPC_RMID, NULL);
 	} else {
-		munmap(addr, (long)(length / 2 * hugepagesize));
-		close(fd);
-		unlink(s);
+		SAFE_MUNMAP(addr, (long)(length / 2 * hugepagesize));
+		SAFE_CLOSE(fd);
+		SAFE_UNLINK(tst_file);
 	}
 
 	tst_res(TPASS, "hugepages overcommit test pass");
@@ -193,57 +174,38 @@ static void test_overcommit(void)
 
 static void cleanup(void)
 {
-	int fd;
-
-	if (restore_shmmax) {
-		fd = open(PATH_SHMMAX, O_WRONLY);
-		if (fd == -1)
-			tst_res(TWARN | TERRNO, "open");
-		if (write(fd, shmmax, strlen(shmmax)) != (ssize_t)strlen(shmmax))
-			tst_res(TWARN | TERRNO, "write");
+	if (opt_shmid) {
+		shmctl(shmid, IPC_RMID, NULL);
+	} else {
 		close(fd);
+		unlink(tst_file);
 	}
-	fd = open(path, O_WRONLY);
-	if (fd == -1)
-		tst_res(TWARN | TERRNO, "open");
-	tst_res(TINFO, "restore nr_hugepages to %s.", nr_hugepages);
-	if (write(fd, nr_hugepages,
-		  strlen(nr_hugepages)) != (ssize_t)strlen(nr_hugepages))
-		tst_res(TWARN | TERRNO, "write");
-	close(fd);
-
-	fd = open(pathover, O_WRONLY);
-	if (fd == -1)
-		tst_res(TWARN | TERRNO, "open");
-	tst_res(TINFO, "restore nr_overcommit_hugepages to %s.",
-		 nr_overcommit_hugepages);
-	if (write(fd, nr_overcommit_hugepages, strlen(nr_overcommit_hugepages))
-	    != (ssize_t)strlen(nr_overcommit_hugepages))
-		tst_res(TWARN | TERRNO, "write");
-	close(fd);
 
 	/* XXX (garrcoop): memory leak. */
-	snprintf(buf, BUFSIZ, "%s/hugemmap05", tst_get_tmpdir());
-	if (umount(buf) == -1)
-		tst_res(TWARN | TERRNO, "umount");
-	if (shmid != -1) {
-		tst_res(TINFO, "shmdt cleaning");
-		shmctl(shmid, IPC_RMID, NULL);
+	if (mounted)
+		tst_umount(mount_dir);
+
+	if (restore_nr_hgpgs) {
+		tst_res(TINFO, "restore nr_hugepages to %ld.", nr_hugepages);
+		SAFE_FILE_PRINTF(path, "%ld", nr_hugepages);
+	}
+
+	if (restore_shmmax)
+		SAFE_FILE_PRINTF(PATH_SHMMAX, "%ld", shmmax);
+
+	if (restore_overcomm_hgpgs) {
+		tst_res(TINFO, "restore nr_overcommit_hugepages to %ld.",
+			nr_overcommit_hugepages);
+		SAFE_FILE_PRINTF(pathover, "%ld", nr_overcommit_hugepages);
 	}
 }
 
 static void setup(void)
 {
-	FILE *fp;
-	int fd;
-	struct stat stat_buf;
-
 	hugepagesize = SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
+	check_hugepage();
 	init_sys_sz_paths();
 
-	if (opt_shmid)
-		shmid = 0;
-
 	if (opt_sysfs) {
 		strncpy(path, path_sys_sz_huge, strlen(path_sys_sz_huge) + 1);
 		strncpy(pathover, path_sys_sz_over,
@@ -252,97 +214,64 @@ static void setup(void)
 		strncpy(path, PATH_PROC_HUGE, strlen(PATH_PROC_HUGE) + 1);
 		strncpy(pathover, PATH_PROC_OVER, strlen(PATH_PROC_OVER) + 1);
 	}
+
 	if (opt_alloc) {
 		size = atoi(opt_alloc);
 		length = (int)(size + size * 0.5) * 2;
 	}
-	if (stat(pathover, &stat_buf) == -1) {
-		if (errno == ENOENT || errno == ENOTDIR)
-			tst_brk(TCONF,
-				 "file %s does not exist in the system",
-				 pathover);
-	}
 
-	if (shmid != -1) {
-		fp = fopen(PATH_SHMMAX, "r");
-		if (fp == NULL)
-			tst_brk(TBROK | TERRNO, "fopen");
-		if (fgets(shmmax, BUFSIZ, fp) == NULL)
-			tst_brk(TBROK | TERRNO, "fgets");
-		fclose(fp);
-
-		if (atol(shmmax) < (long)(length / 2 * hugepagesize)) {
+	if (opt_shmid) {
+		SAFE_FILE_SCANF(PATH_SHMMAX, "%ld", &shmmax);
+		if (shmmax < (long)(length / 2 * hugepagesize)) {
 			restore_shmmax = 1;
-			fd = open(PATH_SHMMAX, O_RDWR);
-			if (fd == -1)
-				tst_brk(TBROK | TERRNO, "open");
-			snprintf(buf, BUFSIZ, "%ld",
-				 (long)(length / 2 * hugepagesize));
-			if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
-				tst_brk(TBROK | TERRNO,
-					 "failed to change shmmax.");
+			SAFE_FILE_PRINTF(PATH_SHMMAX, "%ld",
+					(length / 2 * hugepagesize));
 		}
 	}
-	fp = SAFE_FOPEN(path, "r+");
-	if (fgets(nr_hugepages, BUFSIZ, fp) == NULL)
-		tst_brk(TBROK | TERRNO, "fgets");
-	fclose(fp);
-	/* Remove trailing newline. */
-	nr_hugepages[strlen(nr_hugepages) - 1] = '\0';
-	tst_res(TINFO, "original nr_hugepages is %s", nr_hugepages);
-
-	fd = SAFE_OPEN(path, O_RDWR);
+
+	SAFE_FILE_SCANF(path, "%ld", &nr_hugepages);
+	tst_res(TINFO, "original nr_hugepages is %ld", nr_hugepages);
+
 	/* Reset. */
-	SAFE_WRITE(1, fd, "0", 1);
-	SAFE_LSEEK(fd, 0, SEEK_SET);
-	snprintf(buf, BUFSIZ, "%zd", size);
-	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
-		tst_brk(TBROK | TERRNO,
-			"failed to change nr_hugepages.");
-	close(fd);
-
-	fp = SAFE_FOPEN(pathover, "r+");
-	if (fgets(nr_overcommit_hugepages, BUFSIZ, fp) == NULL)
-		tst_brk(TBROK | TERRNO, "fgets");
-	fclose(fp);
-	nr_overcommit_hugepages[strlen(nr_overcommit_hugepages) - 1] = '\0';
-	tst_res(TINFO, "original nr_overcommit_hugepages is %s",
-		 nr_overcommit_hugepages);
-
-	fd = open(pathover, O_RDWR);
-	if (fd == -1)
-		tst_brk(TBROK | TERRNO, "open");
+	SAFE_FILE_PRINTF(path, "%ld", size);
+	restore_nr_hgpgs = 1;
+
+	if (access(pathover, F_OK)) {
+		tst_brk(TCONF, "file %s does not exist in the system",
+			pathover);
+	}
+
+	SAFE_FILE_SCANF(pathover, "%ld", &nr_overcommit_hugepages);
+	tst_res(TINFO, "original nr_overcommit_hugepages is %ld",
+		nr_overcommit_hugepages);
+
 	/* Reset. */
-	if (write(fd, "0", 1) != 1)
-		tst_brk(TBROK | TERRNO, "write");
-	if (lseek(fd, 0, SEEK_SET) == -1)
-		tst_brk(TBROK | TERRNO, "lseek");
-	snprintf(buf, BUFSIZ, "%zd", size);
-	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
-		tst_brk(TBROK | TERRNO,
-			 "failed to change nr_hugepages.");
-	close(fd);
+	SAFE_FILE_PRINTF(pathover, "%ld", size);
+	restore_overcomm_hgpgs = 1;
 
 	/* XXX (garrcoop): memory leak. */
-	snprintf(buf, BUFSIZ, "%s/hugemmap05", tst_get_tmpdir());
-	if (mkdir(buf, 0700) == -1)
-		tst_brk(TBROK | TERRNO, "mkdir");
-	if (mount(NULL, buf, "hugetlbfs", 0, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "mount");
-}
+	sprintf(mount_dir, "%s/hugemmap05", tst_get_tmpdir());
+	SAFE_MKDIR(mount_dir, 0700);
+	SAFE_MOUNT(NULL, mount_dir, "hugetlbfs", 0, NULL);
+	mounted = 1;
 
-static void write_bytes(void *addr)
-{
-	long i;
-
-	for (i = 0; i < (long)(length / 2 * hugepagesize); i++)
-		((char *)addr)[i] = '\a';
+	if (opt_shmid) {
+		/* Use /proc/meminfo to generate an IPC key. */
+		key = ftok(PATH_MEMINFO, strlen(PATH_MEMINFO));
+		if (key == -1)
+			tst_brk(TBROK | TERRNO, "ftok");
+	} else {
+		/* XXX (garrcoop): memory leak. */
+		sprintf(tst_file, "%s/hugemmap05/file", tst_get_tmpdir());
+	}
 }
 
-static void read_bytes(void *addr)
+static void check_wr_bytes(void *addr)
 {
 	long i;
 
+	memset((char *)addr, '\a', (long)(length / 2 * hugepagesize));
+
 	tst_res(TINFO, "First hex is %x", *((unsigned int *)addr));
 	for (i = 0; i < (long)(length / 2 * hugepagesize); i++) {
 		if (((char *)addr)[i] != '\a') {
@@ -352,52 +281,27 @@ static void read_bytes(void *addr)
 	}
 }
 
-/* Lookup a pattern and get the value from file */
-static int lookup(char *line, char *pattern)
+static int checksys(char *path, char *string, long exp_val)
 {
-	char buf2[BUFSIZ];
-
-	/* empty line */
-	if (line[0] == '\0')
-		return 0;
+	long act_val;
 
-	snprintf(buf2, BUFSIZ, "%s: %%s", pattern);
-	if (sscanf(line, buf2, buf) != 1)
-		return 0;
-
-	return 1;
-}
-
-static int checksys(char *path, char *string, int value)
-{
-	FILE *fp;
-
-	fp = SAFE_FOPEN(path, "r");
-	if (fgets(buf, BUFSIZ, fp) == NULL)
-		tst_brk(TBROK | TERRNO, "fgets");
-	tst_res(TINFO, "%s is %d.", string, atoi(buf));
-	if (atoi(buf) != value) {
-		tst_res(TFAIL, "%s is not %d but %d.", string, value,
-			 atoi(buf));
-		fclose(fp);
+	SAFE_FILE_SCANF(path, "%ld", &act_val);
+	tst_res(TINFO, "%s is %ld.", string, act_val);
+	if (act_val != exp_val) {
+		tst_res(TFAIL, "%s is not %ld but %ld.", string, exp_val,
+			act_val);
 		return 1;
 	}
-	fclose(fp);
 	return 0;
 }
 
-static int checkproc(FILE * fp, char *pattern, int value)
+static int checkproc(long act_val, char *pattern, long exp_val)
 {
-	memset(buf, -1, BUFSIZ);
-	rewind(fp);
-	while (fgets(line, BUFSIZ, fp) != NULL)
-		if (lookup(line, pattern))
-			break;
 
-	tst_res(TINFO, "%s is %d.", pattern, atoi(buf));
-	if (atoi(buf) != value) {
-		tst_res(TFAIL, "%s is not %d but %d.", pattern, value,
-			 atoi(buf));
+	tst_res(TINFO, "%s is %ld.", pattern, act_val);
+	if (act_val != exp_val) {
+		tst_res(TFAIL, "%s is not %ld but %ld.",
+			pattern, exp_val, act_val);
 		return 1;
 	}
 	return 0;
@@ -405,7 +309,7 @@ static int checkproc(FILE * fp, char *pattern, int value)
 
 static void init_sys_sz_paths(void)
 {
-	sprintf(path_sys_sz, "/sys/kernel/mm/hugepages/hugepages-%dkB",
+	sprintf(path_sys_sz, "/sys/kernel/mm/hugepages/hugepages-%ldkB",
 		hugepagesize / 1024);
 	sprintf(path_sys_sz_over, "%s/nr_overcommit_hugepages", path_sys_sz);
 	sprintf(path_sys_sz_free, "%s/free_hugepages", path_sys_sz);
-- 
1.8.3.1





More information about the ltp mailing list