Skip to contentSkip to conversation

ltp/numa: add new test11

PATCH New Li Wang 2 participants 4 messages Jan 20, 2017 – Feb 6, 2017

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH 1/1 ltp/numa: add new test11

Signed-off-by Li Wang <[email protected]>
---
 testcases/kernel/numa/README         |  2 ++
 testcases/kernel/numa/numa01.sh      | 40 ++++++++++++++++++++++++++--
 testcases/kernel/numa/support_numa.c | 51 +++++++++++++++++++++++++++++++++---
 3 files changed, 88 insertions(+), 5 deletions(-)
testcases/kernel/numa/README +2 -0
index 57b12f7..a5c3f1b 100644
@@ -54,6 +54,8 @@ Verifies the numa_node_size api with hardware checking.
 TestCase10:
 Verifieds the NUMA migratepages policy.
 
+TestCase11:
+Verifies the hugepage memory allocated from the node we specify.
 
 Pre-requisites
 ====================================================================================================================
testcases/kernel/numa/numa01.sh +38 -2
index f0f6139..8533c51 100755
@@ -31,11 +31,12 @@
 #               Test #8: Verifies memhog                                     #
 #               Test #9: Verifies numa_node_size api                         #
 #               Test #10:Verifies Migratepages                               #
+#               Test #11:Verifies hugepage alloacted on specified node       #
 #                                                                            #
 ##############################################################################
 
 TST_ID="numa01"
-TST_CNT=10
+TST_CNT=11
 TST_SETUP=setup
 TST_TESTFUNC=test
 TST_NEEDS_TMPDIR=1

@@ -83,11 +84,13 @@ setup()
 {
 	export MB=$((1024*1024))
 	export PAGE_SIZE=$(getconf PAGE_SIZE)
+	export HPAGE_SIZE=$(cat /proc/meminfo  |grep "Hugepagesize:" |awk '{print $2}')
 
 	# arguments to memory exercise program support_numa.c
 	ALLOC_1MB=1
 	SHARE_1MB=2
-	PAUSE=3
+	HUGE_PAGE=3
+	PAUSE=4
 
 	total_nodes=0
 

@@ -399,4 +402,37 @@ test10()
 	tst_res TPASS "NUMA MIGRATEPAGES policy"
 }
 
+# Verification of hugepage memory allocated on a node
+test11()
+{
+	Mem_huge=0
+
+	if [ ! -d "/sys/kernel/mm/hugepages/" ]; then
+		tst_brk TCONF "hugepage is not supported"
+	fi
+
+	for node in $nodes_list; do
+		Ori_hpgs=$(cat /sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages)
+		echo 1 >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages
+
+		numactl --cpunodebind=$node --membind=$node support_numa $HUGE_PAGE &
+		pid=$!
+		wait_for_support_numa $pid
+
+		Mem_huge=$(echo $(numastat -p $pid |grep '^Huge' |awk '{print $'$((node+2))'}'))
+		Mem_huge=$(echo "$Mem_huge * 1024" |bc)
+
+		if [ $(echo "$Mem_huge < $HPAGE_SIZE" | bc) -eq 1 ]; then
+			tst_res TFAIL \
+				"NUMA memory allocated in node$node is less than expected"
+			return
+		fi
+
+		kill -CONT $pid >/dev/null 2>&1
+		echo $Ori_hpgs >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages
+	done
+
+	tst_res TPASS "NUMA local node hugepage memory allocated"
+}
+
 tst_run
testcases/kernel/numa/support_numa.c +48 -3

77 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/1 ltp/numa: add new test11

Hi!

testcases/kernel/numa/numa01.sh test10() · line 411

Quoted 7 lines
testcases/kernel/numa/numa01.sh +7 -0
+# Verification of hugepage memory allocated on a node
+test11()
+{
+	Mem_huge=0
+
+	if [ ! -d "/sys/kernel/mm/hugepages/" ]; then
+		tst_brk TCONF "hugepage is not supported"

This should be tst_res followed by a return on a next line. The tst_brk will exit the whole test just because hugepages are not supported.

testcases/kernel/numa/numa01.sh test10() · line 416

Quoted 5 lines
testcases/kernel/numa/numa01.sh +5 -0
+	fi
+
+	for node in $nodes_list; do
+		Ori_hpgs=$(cat /sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages)
+		echo 1 >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages

Uh, so this sysfs knob controlls how much hugepages can be allocated on a node right? What if it was set to nonzero and some hugepages are allocated already? Shouldn't we rather set it to current value + 1 ?

testcases/kernel/numa/numa01.sh test10() · line 423

Quoted 6 lines
testcases/kernel/numa/numa01.sh +6 -0
+		numactl --cpunodebind=$node --membind=$node support_numa $HUGE_PAGE &
+		pid=$!
+		wait_for_support_numa $pid
+
+		Mem_huge=$(echo $(numastat -p $pid |grep '^Huge' |awk '{print $'$((node+2))'}'))
+		Mem_huge=$(echo "$Mem_huge * 1024" |bc)

Eh, why not just $((Mem_huge * 1024)) ?

testcases/kernel/numa/numa01.sh test10() · line 425

Quoted 1 line
testcases/kernel/numa/numa01.sh +1 -0
+		if [ $(echo "$Mem_huge < $HPAGE_SIZE" | bc) -eq 1 ]; then

Here as well, why not just if [ "$Mem_huge" -lt "$HPAGE_SIZE" ]; then

Quoted 54 lines
testcases/kernel/numa/numa01.sh +12 -0
+			tst_res TFAIL \
+				"NUMA memory allocated in node$node is less than expected"
+			return
+		fi
+
+		kill -CONT $pid >/dev/null 2>&1
+		echo $Ori_hpgs >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages
+	done
+
+	tst_res TPASS "NUMA local node hugepage memory allocated"
+}
+
 tst_run
testcases/kernel/numa/support_numa.c +18 -2
index eaf63e3..0241a19 100644
@@ -22,7 +22,7 @@
 /*                                                                            */
 /* File:        support_numa.c                                                */
 /*                                                                            */
-/* Description: Allocates 1MB of memory and touches it to verify numa         */
+/* Description: Allocates memory and touches it to verify numa                */
 /*                                                                            */
 /* Author:      Sivakumar Chinnaiah  [email protected]                   */
 /*                                                                            */

@@ -52,16 +52,43 @@ static void help(void)
 	printf("Input:	Describe input arguments to this program\n");
 	printf("	argv[1] == 1 then allocate 1MB of memory\n");
 	printf("	argv[1] == 2 then allocate 1MB of share memory\n");
-	printf("	argv[1] == 3 then pause the program to catch sigint\n");
+	printf("        argv[1] == 3 then allocate 1HUGE PAGE SIZE of memory\n");
+	printf("        argv[1] == 4 then pause the program to catch sigint\n");
 	printf("Exit:	On failure - Exits with non-zero value\n");
 	printf("	On success - exits with 0 exit value\n");
 
 	exit(1);
 }
 
+static int read_hugepagesize(void)
+{
+       FILE *fp;
+       char line[BUFSIZ], buf[BUFSIZ];
+       int val;
+
+       fp = fopen("/proc/meminfo", "r");
+       if (fp == NULL) {
+               fprintf(stderr, "Failed to open /proc/meminfo");
+               return 1;
+       }
+
+       while (fgets(line, BUFSIZ, fp) != NULL) {
+               if (sscanf(line, "%64s %d", buf, &val) == 2)
+                       if (strcmp(buf, "Hugepagesize:") == 0) {

testcases/kernel/numa/support_numa.c static void help(void) · line 87

Quoted 10 lines
testcases/kernel/numa/support_numa.c +10 -0
+                               fclose(fp);
+                               return 1024 * val;
+                       }
+       }
+
+       fclose(fp);
+       fprintf(stderr, "can't find \"%s\" in %s", "Hugepagesize:", "/proc/meminfo");
+
+       return 1;
+}

We should rather return 0 on a failure so that we can easily check hpsz != 0 in the main()

testcases/kernel/numa/support_numa.c int main(int argc, char *argv[]) · line 144

Quoted 12 lines
testcases/kernel/numa/support_numa.c +2 -1
 int main(int argc, char *argv[])
 {
-	int i, fd, rc;
+	int i, fd, rc, hpsz;
 	char *buf = NULL;
 	struct stat sb;
 

@@ -114,6 +141,24 @@ int main(int argc, char *argv[])
 		remove(TEST_SFILE);
 		break;
 	case 3:
+		hpsz = read_hugepagesize();

Eh, we do not check for a failure and gracefully try to allocate 1 byte here? That does not sound right.

testcases/kernel/numa/support_numa.c int main(int argc, char *argv[]) · line 151

Quoted 6 lines
testcases/kernel/numa/support_numa.c +6 -0
+		buf = mmap(NULL, hpsz, PROT_READ | PROT_WRITE,
+				MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB,
+				-1, 0);
+
+		if (buf == MAP_FAILED){
+			fprintf(stderr, "mmap failed\n");

What about errno? We should rather use perror() here. Also coding style is wrong, you should use checkpatch.pl to check for common mistakes.

Quoted 16 lines
testcases/kernel/numa/support_numa.c +10 -0
+			exit(1);
+		}
+
+		memset(buf, 'a', hpsz);
+
+		raise(SIGSTOP);
+
+		munmap(buf, hpsz);
+		break;
+	case 4:
 		raise(SIGSTOP);
 		break;
 	default:
Signature · 1 line
1.8.3.1
Signature · 2 lines
Cyril Hrubis
[email protected]
author
#2 lore raw

PATCH 1/1 ltp/numa: add new test11

Quoted 12 lines · Cyril Hrubis

Hi!

testcases/kernel/numa/numa01.sh test10() · line 411

Quoted 7 lines
testcases/kernel/numa/numa01.sh +7 -0
+# Verification of hugepage memory allocated on a node
+test11()
+{
+     Mem_huge=0
+
+     if [ ! -d "/sys/kernel/mm/hugepages/" ]; then
+             tst_brk TCONF "hugepage is not supported"

This should be tst_res followed by a return on a next line. The tst_brk will exit the whole test just because hugepages are not supported.

Good catch, that would probably happened when we have new testcase12 in next.

Quoted 10 lines

testcases/kernel/numa/numa01.sh test10() · line 416

Quoted 6 lines
testcases/kernel/numa/numa01.sh +5 -0
+     fi
+
+     for node in $nodes_list; do
+             Ori_hpgs=$(cat /sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages)
+             echo 1 >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages

Uh, so this sysfs knob controlls how much hugepages can be allocated on a node right? What if it was set to nonzero and some hugepages are allocated already? Shouldn't we rather set it to current value + 1 ?

Yes, we should consider that situation.

echo '$Ori_hpgs + 1' > /sys/...

And, if it failed to allocate new hugepage, exit is necessary.

Quoted 8 lines

testcases/kernel/numa/numa01.sh test10() · line 423

Quoted 7 lines
testcases/kernel/numa/numa01.sh +6 -0
+             numactl --cpunodebind=$node --membind=$node support_numa $HUGE_PAGE &
+             pid=$!
+             wait_for_support_numa $pid
+
+             Mem_huge=$(echo $(numastat -p $pid |grep '^Huge' |awk '{print $'$((node+2))'}'))
+             Mem_huge=$(echo "$Mem_huge * 1024" |bc)
Quoted 5 lines

Eh, why not just $((Mem_huge * 1024)) ?

Quoted 1 line

+ if [ $(echo "$Mem_huge < $HPAGE_SIZE" | bc) -eq 1 ]; then

Here as well, why not just if [ "$Mem_huge" -lt "$HPAGE_SIZE" ]; then

agree!

Quoted 60 lines
Quoted 60 lines
testcases/kernel/numa/numa01.sh +12 -0
+                     tst_res TFAIL \
+                             "NUMA memory allocated in node$node is less than expected"
+                     return
+             fi
+
+             kill -CONT $pid >/dev/null 2>&1
+             echo $Ori_hpgs >/sys/devices/system/node/node${node}/hugepages/hugepages-${HPAGE_SIZE}kB/nr_hugepages
+     done
+
+     tst_res TPASS "NUMA local node hugepage memory allocated"
+}
+
 tst_run
testcases/kernel/numa/support_numa.c +23 -2
index eaf63e3..0241a19 100644
@@ -22,7 +22,7 @@
 /*                                                                            */
 /* File:        support_numa.c                                                */
 /*                                                                            */
-/* Description: Allocates 1MB of memory and touches it to verify numa         */
+/* Description: Allocates memory and touches it to verify numa                */
 /*                                                                            */
 /* Author:      Sivakumar Chinnaiah  [email protected]                   */
 /*                                                                            */

@@ -52,16 +52,43 @@ static void help(void)
      printf("Input:  Describe input arguments to this program\n");
      printf("        argv[1] == 1 then allocate 1MB of memory\n");
      printf("        argv[1] == 2 then allocate 1MB of share memory\n");
-     printf("        argv[1] == 3 then pause the program to catch sigint\n");
+     printf("        argv[1] == 3 then allocate 1HUGE PAGE SIZE of memory\n");
+     printf("        argv[1] == 4 then pause the program to catch sigint\n");
      printf("Exit:   On failure - Exits with non-zero value\n");
      printf("        On success - exits with 0 exit value\n");
 
      exit(1);
 }
 
+static int read_hugepagesize(void)
+{
+       FILE *fp;
+       char line[BUFSIZ], buf[BUFSIZ];
+       int val;
+
+       fp = fopen("/proc/meminfo", "r");
+       if (fp == NULL) {
+               fprintf(stderr, "Failed to open /proc/meminfo");
+               return 1;
+       }
+
+       while (fgets(line, BUFSIZ, fp) != NULL) {
+               if (sscanf(line, "%64s %d", buf, &val) == 2)
+                       if (strcmp(buf, "Hugepagesize:") == 0) {
+                               fclose(fp);
+                               return 1024 * val;
+                       }
+       }
+
Quoted 8 lines

testcases/kernel/numa/support_numa.c static void help(void) · line 87

Quoted 5 lines
testcases/kernel/numa/support_numa.c +5 -0
+       fclose(fp);
+       fprintf(stderr, "can't find \"%s\" in %s", "Hugepagesize:", "/proc/meminfo");
+
+       return 1;
+}

We should rather return 0 on a failure so that we can easily check hpsz != 0 in the main()

Sure.

Quoted 8 lines
Quoted 8 lines
testcases/kernel/numa/support_numa.c +1 -1
 int main(int argc, char *argv[])
 {
-     int i, fd, rc;
+     int i, fd, rc, hpsz;
      char *buf = NULL;
      struct stat sb;
Quoted 8 lines

testcases/kernel/numa/support_numa.c int main(int argc, char *argv[]) · line 144

Quoted 5 lines
testcases/kernel/numa/support_numa.c +1 -0
@@ -114,6 +141,24 @@ int main(int argc, char *argv[])
              remove(TEST_SFILE);
              break;
      case 3:
+             hpsz = read_hugepagesize();

Eh, we do not check for a failure and gracefully try to allocate 1 byte here? That does not sound right.

Yes, let me do that in next PATCH version.

Quoted 10 lines

testcases/kernel/numa/support_numa.c int main(int argc, char *argv[]) · line 151

Quoted 7 lines
testcases/kernel/numa/support_numa.c +6 -0
+             buf = mmap(NULL, hpsz, PROT_READ | PROT_WRITE,
+                             MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB,
+                             -1, 0);
+
+             if (buf == MAP_FAILED){
+                     fprintf(stderr, "mmap failed\n");

What about errno? We should rather use perror() here. Also coding style is wrong, you should use checkpatch.pl to check for common mistakes.

sure, thanks!

Quoted 21 lines
Quoted 15 lines
testcases/kernel/numa/support_numa.c +10 -1
+                     exit(1);
+             }
+
+             memset(buf, 'a', hpsz);
+
+             raise(SIGSTOP);
+
+             munmap(buf, hpsz);
+             break;
+     case 4:
              raise(SIGSTOP);
              break;
      default:
--
Quoted 2 lines

1.8.3.1

--
Cyril Hrubis
[email protected]

Signature · 3 lines
Regards,
Li Wang
Email: [email protected]
author
#3 lore raw

PATCH 1/1 ltp/numa: add new test11

In reply to Cyril Hrubis
Quoted 9 lines · Cyril Hrubis

testcases/kernel/numa/numa01.sh test10() · line 423

Quoted 7 lines
testcases/kernel/numa/numa01.sh +6 -0
+             numactl --cpunodebind=$node --membind=$node support_numa $HUGE_PAGE &
+             pid=$!
+             wait_for_support_numa $pid
+
+             Mem_huge=$(echo $(numastat -p $pid |grep '^Huge' |awk '{print $'$((node+2))'}'))
+             Mem_huge=$(echo "$Mem_huge * 1024" |bc)

Eh, why not just $((Mem_huge * 1024)) ?

Hmm, from what I test this is not a proper form here.

The Mem_huge is a Floating-point data(as: 2.00), so I prefer to keep the original operate style.

Quoted 5 lines
Quoted 2 lines

+ if [ $(echo "$Mem_huge < $HPAGE_SIZE" | bc) -eq 1 ]; then

Here as well, why not just if [ "$Mem_huge" -lt "$HPAGE_SIZE" ]; then

Here likewise.

Quoted 4 lines
Quoted 4 lines
testcases/kernel/numa/numa01.sh +4 -0
+                     tst_res TFAIL \
+                             "NUMA memory allocated in node$node is less than expected"
+                     return
+             fi
Signature · 3 lines
Regards,
Li Wang
Email: [email protected]