Skip to contentSkip to conversation

khugeepaged: Replace the usage of system(3) in the test.

PATCH In review Aneesh Kumar K.V 2 participants 2 messages Apr 29, 2020

Subsystems (2)
Memory management Kselftest
lore
Share & tools
lore

Aneesh Kumar K.V

author
#0 lore raw

PATCH 1/1 khugeepaged: Replace the usage of system(3) in the test.

Some glibc version doesn't use CLONE_VM | CLONE_VFORK for system(3) implementation and on such system, we find the test case fails. This is due to fork() marking all the parent page table pages read-only to do a COW.

Avoid the usage of system(3) in the test.

Signed-off-by Aneesh Kumar K.V <[email protected]>
---
 tools/testing/selftests/vm/khugepaged.c | 105 +++++++++++++++++-------
 1 file changed, 77 insertions(+), 28 deletions(-)
tools/testing/selftests/vm/khugepaged.c +77 -28
index 490055290d7f..9b2d675c0fd3 100644
@@ -21,6 +21,7 @@ static unsigned long page_size;
 static int hpage_pmd_nr;
 
 #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
+#define PID_SMAPS "/proc/self/smaps"
 
 enum thp_enabled {
 	THP_ALWAYS,

@@ -333,56 +334,104 @@ static void adjust_settings(void)
 	success("OK");
 }
 
-#define CHECK_HUGE_FMT "sed -ne " \
-	"'/^%lx/,/^AnonHugePages/{/^AnonHugePages:\\s*%ld kB/ q1}' " \
-	"/proc/%d/smaps"
+#define MAX_LINE_LENGTH 500
 
-static bool check_huge(void *p)
+static bool check_for_pattern(FILE *fp, char *pattern, char *buf)
 {
-	char *cmd;
+	while (fgets(buf, MAX_LINE_LENGTH, fp) != NULL) {
+		if (!strncmp(buf, pattern, strlen(pattern)))
+			return true;
+	}
+	return false;
+}
+
+static bool check_huge(char *addr)
+{
+	bool thp = false;
 	int ret;
+	FILE *fp;
+	char buffer[MAX_LINE_LENGTH];
+	char addr_pattern[MAX_LINE_LENGTH];
 
-	ret = asprintf(&cmd, CHECK_HUGE_FMT,
-			(unsigned long)p, hpage_pmd_size >> 10, getpid());
-	if (ret < 0) {
-		perror("asprintf(CHECK_FMT)");
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08llx-", addr);
+	if (ret >= MAX_LINE_LENGTH) {
+		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);
 	}
 
-	ret = system(cmd);
-	free(cmd);
-	if (ret < 0 || !WIFEXITED(ret)) {
-		perror("system(check_huge)");
+
+	fp = fopen(PID_SMAPS, "r");
+	if (!fp) {
+		printf("%s: Failed to open file %s\n", __func__, PID_SMAPS);
 		exit(EXIT_FAILURE);
 	}
+	if (!check_for_pattern(fp, addr_pattern, buffer))
+		goto err_out;
 
-	return WEXITSTATUS(ret);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "AnonHugePages:%10lld kB", hpage_pmd_size >> 10);
+	if (ret >= MAX_LINE_LENGTH) {
+		printf("%s: Pattern is too long\n", __func__);
+		exit(EXIT_FAILURE);
+	}
+	/*
+	 * Fetch the AnonHugePages: in the same block and check whether it got
+	 * the expected number of hugeepages next.
+	 */
+	if (!check_for_pattern(fp, "AnonHugePages:", buffer))
+		goto err_out;
+
+	if (strncmp(buffer, addr_pattern, strlen(addr_pattern)))
+		goto err_out;
+
+	thp = true;
+err_out:
+	fclose(fp);
+	return thp;
 }
 
-#define CHECK_SWAP_FMT "sed -ne " \
-	"'/^%lx/,/^Swap:/{/^Swap:\\s*%ld kB/ q1}' " \
-	"/proc/%d/smaps"
 
-static bool check_swap(void *p, unsigned long size)
+static bool check_swap(void *addr, unsigned long size)
 {
-	char *cmd;
+	bool swap = false;
 	int ret;
+	FILE *fp;
+	char buffer[MAX_LINE_LENGTH];
+	char addr_pattern[MAX_LINE_LENGTH];
 
-	ret = asprintf(&cmd, CHECK_SWAP_FMT,
-			(unsigned long)p, size >> 10, getpid());
-	if (ret < 0) {
-		perror("asprintf(CHECK_SWAP)");
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08llx-", addr);
+	if (ret >= MAX_LINE_LENGTH) {
+		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);
 	}
 
-	ret = system(cmd);
-	free(cmd);
-	if (ret < 0 || !WIFEXITED(ret)) {
-		perror("system(check_swap)");
+
+	fp = fopen(PID_SMAPS, "r");
+	if (!fp) {
+		printf("%s: Failed to open file %s\n", __func__, PID_SMAPS);
 		exit(EXIT_FAILURE);
 	}
+	if (!check_for_pattern(fp, addr_pattern, buffer))
+		goto err_out;
 
-	return WEXITSTATUS(ret);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "Swap:%19lld kB", size >> 10);
+	if (ret >= MAX_LINE_LENGTH) {
+		printf("%s: Pattern is too long\n", __func__);
+		exit(EXIT_FAILURE);
+	}
+	/*
+	 * Fetch the Swap: in the same block and check whether it got
+	 * the expected number of hugeepages next.
+	 */
+	if (!check_for_pattern(fp, "Swap:", buffer))
+		goto err_out;
+
+	if (strncmp(buffer, addr_pattern, strlen(addr_pattern)))
+		goto err_out;
+
+	swap = true;
+err_out:
+	fclose(fp);
+	return swap;
 }
 
 static void *alloc_mapping(void)
Signature · 1 line
2.26.2
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Aneesh Kumar K.V

Some glibc version doesn't use CLONE_VM | CLONE_VFORK for system(3) implementation and on such system, we find the test case fails. This is due to fork() marking all the parent page table pages read-only to do a COW.

Avoid the usage of system(3) in the test.

Signed-off-by Aneesh Kumar K.V <[email protected]>

Below is fixup for issues I've noticed.

With them integrated the patch looks good to me and you can use my

Acked-by Kirill A. Shutemov <[email protected]>
tools/testing/selftests/vm/khugepaged.c +9 -5
index 9b2d675c0fd3..5d0698d4a101 100644
@@ -345,7 +345,7 @@ static bool check_for_pattern(FILE *fp, char *pattern, char *buf)
 	return false;
 }
 
-static bool check_huge(char *addr)
+static bool check_huge(void *addr)
 {
 	bool thp = false;
 	int ret;

@@ -353,7 +353,8 @@ static bool check_huge(char *addr)
 	char buffer[MAX_LINE_LENGTH];
 	char addr_pattern[MAX_LINE_LENGTH];
 
-	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08llx-", addr);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08lx-",
+		       (unsigned long) addr);
 	if (ret >= MAX_LINE_LENGTH) {
 		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);

@@ -368,7 +369,8 @@ static bool check_huge(char *addr)
 	if (!check_for_pattern(fp, addr_pattern, buffer))
 		goto err_out;
 
-	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "AnonHugePages:%10lld kB", hpage_pmd_size >> 10);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "AnonHugePages:%10ld kB",
+		       hpage_pmd_size >> 10);
 	if (ret >= MAX_LINE_LENGTH) {
 		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);

@@ -398,7 +400,8 @@ static bool check_swap(void *addr, unsigned long size)
 	char buffer[MAX_LINE_LENGTH];
 	char addr_pattern[MAX_LINE_LENGTH];
 
-	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08llx-", addr);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08lx-",
+		       (unsigned long) addr);
 	if (ret >= MAX_LINE_LENGTH) {
 		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);

@@ -413,7 +416,8 @@ static bool check_swap(void *addr, unsigned long size)
 	if (!check_for_pattern(fp, addr_pattern, buffer))
 		goto err_out;
 
-	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "Swap:%19lld kB", size >> 10);
+	ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "Swap:%19ld kB",
+		       size >> 10);
 	if (ret >= MAX_LINE_LENGTH) {
 		printf("%s: Pattern is too long\n", __func__);
 		exit(EXIT_FAILURE);
Signature · 1 line
 Kirill A. Shutemov