[LTP] [PATCH v2] Migrating the libhugetlbfs/testcases/slbpacaflush.c test

[LTP] [PATCH v2] Migrating the libhugetlbfs/testcases/slbpacaflush.c test

[Shirisha G]

We are verifying ppc64 kernels prior to 2.6.15-rc5 exhibit a bug in the
hugepage SLB flushing path. When opening new hugetlb areas, updating masks
in the thread_struct and copying to the PACA only occurs on the CPU where
segments are opened, leading to potential stale copies in other CPUs.
This bug can be triggered by multiple threads sharing the mm or a single thread
migrating between CPUs, particularly evident in a close-to-idle system,
as other processes may flush the SLB and prevent the bug from manifesting.

Original test originates from https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/slbpacaflush.c

Signed-off-by: Shirisha G <shirisha@linux.ibm.com>
---
---
V2:
 -Addressed below requested changes
 1. Removed the blank lines
 2. Ran make-check and fixed the issues
 3. As suggested used TERRNO
 4. Defined CPU_SETSIZE/8 at the top
 5. use get_nprocs_conf() directly as suggested
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 137 ++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 299c07ac9..d956866ac 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -35,6 +35,7 @@ hugemmap29 hugemmap29
 hugemmap30 hugemmap30
 hugemmap31 hugemmap31
 hugemmap32 hugemmap32
+hugemmap41 hugemmap41
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index c96fe8bfc..b7e108956 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -34,6 +34,7 @@
 /hugetlb/hugemmap/hugemmap30
 /hugetlb/hugemmap/hugemmap31
 /hugetlb/hugemmap/hugemmap32
+/hugetlb/hugemmap/hugemmap41
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
new file mode 100644
index 000000000..5c2d4f77c
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2005-2006 IBM Corporation.
+ * Author: David Gibson & Adam Litke
+ */
+/*\
+ * [Description]
+ *
+ * ppc64 kernels (prior to 2.6.15-rc5) have a bug in the hugepage SLB
+ * flushing path.  After opening new hugetlb areas, we update the
+ * masks in the thread_struct, copy to the PACA, then do slbies on
+ * each CPU.  The trouble is we only copy to the PACA on the CPU where
+ * we're opening the segments, which can leave a stale copy in the
+ * PACAs on other CPUs.
+ *
+ * This can be triggered either with multiple threads sharing the mm,
+ * or with a single thread which is migrated from one CPU, to another
+ * (where the mapping occurs), then back again (where we touch the
+ * stale SLB).  We use the second method in this test, since it's
+ * easier to force (using sched_setaffinity).  However it relies on a
+ * close-to-idle system, if any process other than a kernel thread
+ * runs on the first CPU between runs of the test process, the SLB
+ * will be flushed and we won't trigger the bug, hence the
+ * PASS_INCONCLUSIVE().  Obviously, this test won't work on a 1-cpu
+ * system (should get CONFIG() on the sched_setaffinity)
+ *
+ */
+#define _GNU_SOURCE
+#define SYSFS_CPU_ONLINE_FMT "/sys/devices/system/cpu/cpu%d/online"
+#define MNTPOINT "hugetlbfs/"
+#define SINGLE_CPU (CPU_SETSIZE/8)
+#include "hugetlb.h"
+
+
+#include <stdio.h>
+#include <sched.h>
+
+
+long hpage_size;
+int fd;
+void *p;
+volatile unsigned long *q;
+int online_cpus[2], err;
+cpu_set_t cpu0, cpu1;
+
+void check_online_cpus(int online_cpus[], int nr_cpus_needed)
+{
+	char cpu_state, path_buf[64];
+	int cpu_idx, fd, ret, i;
+
+	cpu_idx = 0;
+
+	if (get_nprocs() < nr_cpus_needed)
+		tst_brk(TCONF, "minimum  %d online cpus are required", nr_cpus_needed);
+
+	for (i = 0; i < get_nprocs_conf() && cpu_idx < nr_cpus_needed; i++) {
+		errno = 0;
+		sprintf(path_buf, SYSFS_CPU_ONLINE_FMT, i);
+		fd = open(path_buf, O_RDONLY);
+		if (fd < 0) {
+			/* If 'online' is absent, the cpu cannot be offlined */
+			if (errno == ENOENT) {
+				online_cpus[cpu_idx] = i;
+				cpu_idx++;
+				continue;
+			} else {
+				tst_res(TFAIL | TERRNO, "Unable to open %s", path_buf);
+			}
+		}
+
+		ret = read(fd, &cpu_state, 1);
+		if (ret < 1)
+			tst_res(TFAIL | TERRNO, "Unable to read %s", path_buf);
+
+		if (cpu_state == '1') {
+			online_cpus[cpu_idx] = i;
+			cpu_idx++;
+		}
+
+		if (fd >= 0)
+			SAFE_CLOSE(fd);
+	}
+
+	if (cpu_idx < nr_cpus_needed)
+		tst_brk(TCONF, "minimum %d online cpus were not found", nr_cpus_needed);
+}
+
+
+static void run_test(void)
+{
+	check_online_cpus(online_cpus, 2);
+	CPU_ZERO(&cpu0);
+	CPU_SET(online_cpus[0], &cpu0);
+	CPU_ZERO(&cpu1);
+	CPU_SET(online_cpus[1], &cpu1);
+
+	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
+	if (err != 0)
+		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)", online_cpus[0]);
+
+	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu1);
+
+	if (err != 0)
+		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)", online_cpus[1]);
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+
+	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
+	if (err != 0)
+		tst_res(TFAIL, "sched_setaffinity(cpu%d)", online_cpus[0]);
+	q = (volatile unsigned long *)(p + getpagesize());
+	*q = 0xdeadbeef;
+
+	tst_res(TPASS, "Test Passed inconclusive");
+}
+
+static void setup(void)
+{
+	hpage_size = tst_get_hugepage_size();
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+}
+
+void cleanup(void)
+{
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {1, TST_NEEDS},
+};
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] Migrating the libhugetlbfs/testcases/slbpacaflush.c test

[Cyril Hrubis]

Hi!
> +#define _GNU_SOURCE
> +#define SYSFS_CPU_ONLINE_FMT "/sys/devices/system/cpu/cpu%d/online"
> +#define MNTPOINT "hugetlbfs/"
> +#define SINGLE_CPU (CPU_SETSIZE/8)
> +#include "hugetlb.h"
> +
> +
> +#include <stdio.h>
> +#include <sched.h>
> +
> +
> +long hpage_size;
> +int fd;
> +void *p;
> +volatile unsigned long *q;
> +int online_cpus[2], err;
> +cpu_set_t cpu0, cpu1;

All these should be static.

> +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> +{
> +	char cpu_state, path_buf[64];
> +	int cpu_idx, fd, ret, i;
> +
> +	cpu_idx = 0;
> +
> +	if (get_nprocs() < nr_cpus_needed)
> +		tst_brk(TCONF, "minimum  %d online cpus are required", nr_cpus_needed);
> +
> +	for (i = 0; i < get_nprocs_conf() && cpu_idx < nr_cpus_needed; i++) {
> +		errno = 0;
> +		sprintf(path_buf, SYSFS_CPU_ONLINE_FMT, i);
> +		fd = open(path_buf, O_RDONLY);
> +		if (fd < 0) {
> +			/* If 'online' is absent, the cpu cannot be offlined */
> +			if (errno == ENOENT) {
> +				online_cpus[cpu_idx] = i;
> +				cpu_idx++;
> +				continue;
> +			} else {
> +				tst_res(TFAIL | TERRNO, "Unable to open %s", path_buf);
> +			}
> +		}
> +
> +		ret = read(fd, &cpu_state, 1);
> +		if (ret < 1)
> +			tst_res(TFAIL | TERRNO, "Unable to read %s", path_buf);
> +
> +		if (cpu_state == '1') {
> +			online_cpus[cpu_idx] = i;
> +			cpu_idx++;
> +		}
> +
> +		if (fd >= 0)
> +			SAFE_CLOSE(fd);
> +	}
> +
> +	if (cpu_idx < nr_cpus_needed)
> +		tst_brk(TCONF, "minimum %d online cpus were not found", nr_cpus_needed);
> +}

There seems to be an easier method, recently we needed to find online
CPU for the startvation tests and all you need to do is to get the
current thread affinity and then look for non-zero bits in that, see
commit:

commit 1800e635783b69cacdce9f654ecd730a8f30915b
Author: Edward Liaw via ltp <ltp@lists.linux.it>
Date:   Wed Jun 19 16:28:07 2024 +0000

And I suppose that it would make sense to put this function in the test
library so taht we do not have to repeat it over in tests, but we would
have to make it return the actual number of CPUs found and do the
tst_brk(TCONF, ...) in the tests instead, so we would add:

unsigned int tst_get_online_cpus(int online_cpus[], unsigned int online_cpus_cnt);

And the test would do:

	if (tst_get_online_cpus(online_cpus, 2) != 2)
		tst_brK(TCONF, "Require at least 2 online CPUs.");

> +static void run_test(void)
> +{
> +	check_online_cpus(online_cpus, 2);
> +	CPU_ZERO(&cpu0);
> +	CPU_SET(online_cpus[0], &cpu0);
> +	CPU_ZERO(&cpu1);
> +	CPU_SET(online_cpus[1], &cpu1);
> +
> +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
> +	if (err != 0)
> +		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)", online_cpus[0]);
> +
> +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu1);
> +
> +	if (err != 0)
> +		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)", online_cpus[1]);
> +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> +
> +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
> +	if (err != 0)
> +		tst_res(TFAIL, "sched_setaffinity(cpu%d)", online_cpus[0]);
> +	q = (volatile unsigned long *)(p + getpagesize());
> +	*q = 0xdeadbeef;

I suppose that the test crashes here, when the entries are not
propagated, right?

> +	tst_res(TPASS, "Test Passed inconclusive");

We usually print something as tst_res(TPASS, "Nothing bad happend, probably");
in the case that we haven't managed to crash the system.

Also we have to unmap the p here so that the test works properly with -i
parameter.

> +}
> +
> +static void setup(void)
> +{
> +	hpage_size = tst_get_hugepage_size();
> +	fd = tst_creat_unlinked(MNTPOINT, 0);
> +}
> +
> +void cleanup(void)
> +{
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.needs_hugetlbfs = 1,
> +	.needs_tmpdir = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run_test,
> +	.hugepages = {1, TST_NEEDS},
> +};
> -- 
> 2.39.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] Migrating the libhugetlbfs/testcases/slbpacaflush.c test

[Shirisha ganta]

On Mon, 2024-06-24 at 13:57 +0200, Cyril Hrubis wrote:
> Hi!
> > +#define _GNU_SOURCE
> > +#define SYSFS_CPU_ONLINE_FMT
> > "/sys/devices/system/cpu/cpu%d/online"
> > +#define MNTPOINT "hugetlbfs/"
> > +#define SINGLE_CPU (CPU_SETSIZE/8)
> > +#include "hugetlb.h"
> > +
> > +
> > +#include <stdio.h>
> > +#include <sched.h>
> > +
> > +
> > +long hpage_size;
> > +int fd;
> > +void *p;
> > +volatile unsigned long *q;
> > +int online_cpus[2], err;
> > +cpu_set_t cpu0, cpu1;
> 
> All these should be static.
will take care in V3
> 
> > +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> > +{
> > +	char cpu_state, path_buf[64];
> > +	int cpu_idx, fd, ret, i;
> > +
> > +	cpu_idx = 0;
> > +
> > +	if (get_nprocs() < nr_cpus_needed)
> > +		tst_brk(TCONF, "minimum  %d online cpus are required",
> > nr_cpus_needed);
> > +
> > +	for (i = 0; i < get_nprocs_conf() && cpu_idx < nr_cpus_needed;
> > i++) {
> > +		errno = 0;
> > +		sprintf(path_buf, SYSFS_CPU_ONLINE_FMT, i);
> > +		fd = open(path_buf, O_RDONLY);
> > +		if (fd < 0) {
> > +			/* If 'online' is absent, the cpu cannot be
> > offlined */
> > +			if (errno == ENOENT) {
> > +				online_cpus[cpu_idx] = i;
> > +				cpu_idx++;
> > +				continue;
> > +			} else {
> > +				tst_res(TFAIL | TERRNO, "Unable to open
> > %s", path_buf);
> > +			}
> > +		}
> > +
> > +		ret = read(fd, &cpu_state, 1);
> > +		if (ret < 1)
> > +			tst_res(TFAIL | TERRNO, "Unable to read %s",
> > path_buf);
> > +
> > +		if (cpu_state == '1') {
> > +			online_cpus[cpu_idx] = i;
> > +			cpu_idx++;
> > +		}
> > +
> > +		if (fd >= 0)
> > +			SAFE_CLOSE(fd);
> > +	}
> > +
> > +	if (cpu_idx < nr_cpus_needed)
> > +		tst_brk(TCONF, "minimum %d online cpus were not found",
> > nr_cpus_needed);
> > +}
> 
> There seems to be an easier method, recently we needed to find online
> CPU for the startvation tests and all you need to do is to get the
> current thread affinity and then look for non-zero bits in that, see
> commit:
> 
> commit 1800e635783b69cacdce9f654ecd730a8f30915b
> Author: Edward Liaw via ltp <ltp@lists.linux.it>
> Date:   Wed Jun 19 16:28:07 2024 +0000
> 
> And I suppose that it would make sense to put this function in the
> test
> library so taht we do not have to repeat it over in tests, but we
> would
> have to make it return the actual number of CPUs found and do the
> tst_brk(TCONF, ...) in the tests instead, so we would add:
> 
> unsigned int tst_get_online_cpus(int online_cpus[], unsigned int
> online_cpus_cnt);
> 
> And the test would do:
> 
> 	if (tst_get_online_cpus(online_cpus, 2) != 2)
> 		tst_brK(TCONF, "Require at least 2 online CPUs.");
will take care in V3
> 
> > +static void run_test(void)
> > +{
> > +	check_online_cpus(online_cpus, 2);
> > +	CPU_ZERO(&cpu0);
> > +	CPU_SET(online_cpus[0], &cpu0);
> > +	CPU_ZERO(&cpu1);
> > +	CPU_SET(online_cpus[1], &cpu1);
> > +
> > +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
> > +	if (err != 0)
> > +		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)",
> > online_cpus[0]);
> > +
> > +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu1);
> > +
> > +	if (err != 0)
> > +		tst_res(TFAIL | TERRNO, "sched_setaffinity(cpu%d)",
> > online_cpus[1]);
> > +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE,
> > MAP_SHARED, fd, 0);
> > +
> > +	err = sched_setaffinity(getpid(), SINGLE_CPU, &cpu0);
> > +	if (err != 0)
> > +		tst_res(TFAIL, "sched_setaffinity(cpu%d)",
> > online_cpus[0]);
> > +	q = (volatile unsigned long *)(p + getpagesize());
> > +	*q = 0xdeadbeef;
> 
> I suppose that the test crashes here, when the entries are not
> propagated, right?
> 
> > +	tst_res(TPASS, "Test Passed inconclusive");
> 
> We usually print something as tst_res(TPASS, "Nothing bad happend,
> probably");
> in the case that we haven't managed to crash the system.
> 
> Also we have to unmap the p here so that the test works properly with
> -i
> parameter.
sure. will take care in v3
> 
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	hpage_size = tst_get_hugepage_size();
> > +	fd = tst_creat_unlinked(MNTPOINT, 0);
> > +}
> > +
> > +void cleanup(void)
> > +{
> > +	if (fd > 0)
> > +		SAFE_CLOSE(fd);
> > +}
> > +
> > +static struct tst_test test = {
> > +	.needs_root = 1,
> > +	.mntpoint = MNTPOINT,
> > +	.needs_hugetlbfs = 1,
> > +	.needs_tmpdir = 1,
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +	.test_all = run_test,
> > +	.hugepages = {1, TST_NEEDS},
> > +};
> > -- 
> > 2.39.3
> > 
> > 
> > -- 
> > Mailing list info: https://lists.linux.it/listinfo/ltp


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp