diff mbox series

Add hugemmap41(Migrating the libhugetlbfs/testcases/slbpacaflush.c test)

Message ID 20231213084753.61762-1-shirisha@linux.ibm.com
State Changes Requested
Headers show
Series Add hugemmap41(Migrating the libhugetlbfs/testcases/slbpacaflush.c test) | expand

Commit Message

Shirisha G Dec. 13, 2023, 8:47 a.m. UTC
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>
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 144 ++++++++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c

Comments

Petr Vorel Feb. 19, 2024, 10:10 p.m. UTC | #1
Hi Shirisha,

> We are verifying ppc64 kernels prior to 2.6.15-rc5 exhibit a bug in the
Test is from 2005, for 2.6.15-rc5. Is it really relevant now?

> 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.

Please run  make check-hugemmap41 in the test directory and fix formatting.

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

> Signed-off-by: Shirisha G <shirisha@linux.ibm.com>
> ---
>  runtest/hugetlb                               |   1 +
>  testcases/kernel/mem/.gitignore               |   1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 144 ++++++++++++++++++
>  3 files changed, 146 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
nit: Any reason why not add it as hugemmap33? You don't want to clash with other
sent test, right?

>  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..3b3388c68
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
> @@ -0,0 +1,144 @@
> +// 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
> +#include "hugetlb.h"
> +#define SYSFS_CPU_ONLINE_FMT	"/sys/devices/system/cpu/cpu%d/online"
> +#define MNTPOINT "hugetlbfs/"
Could you please have these 2 #define above _GNU_SOURCE? (readablility)
> +
> +
> +#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;
> +
> +
> +
Please remove these blank lines above.

> +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> +{
> +	char cpu_state, path_buf[64];
> +	int total_cpus, cpu_idx, fd, ret, i;
> +
> +	total_cpus = get_nprocs_conf();
> +	cpu_idx = 0;
> +
> +	if (get_nprocs() < nr_cpus_needed)
> +		tst_res(TFAIL, "Atleast online %d cpus are required", nr_cpus_needed);
> +
> +	for (i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
nit: Maybe just use get_nprocs_conf() directly?

> +		errno = 0;
Is it errno reset really needed?

> +		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, "Unable to open %s: %s", path_buf,
> +				     strerror(errno));
We have TERRNO:
				tst_res(TFAIL | TERRNO, "Unable to open %s: %s", path_buf);
> +			}
> +		}
> +
> +		ret = read(fd, &cpu_state, 1);
> +		if (ret < 1)
> +			tst_res(TFAIL, "Unable to read %s: %s", path_buf,
> +			     strerror(errno));
Maybe use SAFE_READ() ?
> +
> +		if (cpu_state == '1') {
> +			online_cpus[cpu_idx] = i;
> +			cpu_idx++;
> +		}
> +
> +		if (fd >= 0)
> +			SAFE_CLOSE(fd);
> +	}
> +
> +	if (cpu_idx < nr_cpus_needed)
> +		tst_res(TFAIL, "Atleast %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(), CPU_SETSIZE/8, &cpu0);
> +	if (err != 0)
> +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[0],
> +				strerror(errno));
Again, please use TTERRNO.
> +
> +	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu1);
Maybe define CPU_SETSIZE/8 at the top?
> +
> +	if (err != 0)
> +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[1],
> +				strerror(errno));
> +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> +
> +	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu0);
> +	if (err != 0)
> +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[0],
> +				strerror(errno));
> +	q = (volatile unsigned long *)(p + getpagesize());
> +	*q = 0xdeadbeef;
Why to set the address before end of testing? (yes, the original does it, but
why?). Wouldn't be better to use guarded buffers instead?

https://github.com/linux-test-project/ltp/wiki/C-Test-API#131-guarded-buffers

Kind regards,
Petr

> +
> +	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},
> +};
Petr Vorel Feb. 19, 2024, 10:18 p.m. UTC | #2
Hi Shirisha,

> +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> +{
> +	char cpu_state, path_buf[64];
> +	int total_cpus, cpu_idx, fd, ret, i;
> +
> +	total_cpus = get_nprocs_conf();
> +	cpu_idx = 0;
> +
> +	if (get_nprocs() < nr_cpus_needed)
> +		tst_res(TFAIL, "Atleast online %d cpus are required", nr_cpus_needed);
s/Atleast/At least/
(or better: "minimum %d online cpus" ..., but I'm not a native speaker.)

Also test should exit when not enough online CPU, right? (it's a test
requirement not a subject of testing). Therefore tst_brk(TCONF, ...) should be
used instead of tst_res(TFAIL).

And shouldn't this be a setup function (i.e. run only once, before test starts -
test itself can be run more times, eg 3x with -i3).

> +
> +	for (i = 0; i < total_cpus && 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, "Unable to open %s: %s", path_buf,
> +				     strerror(errno));
> +			}
> +		}
> +
> +		ret = read(fd, &cpu_state, 1);
> +		if (ret < 1)
> +			tst_res(TFAIL, "Unable to read %s: %s", path_buf,
> +			     strerror(errno));
> +
> +		if (cpu_state == '1') {
> +			online_cpus[cpu_idx] = i;
> +			cpu_idx++;
> +		}
> +
> +		if (fd >= 0)
> +			SAFE_CLOSE(fd);
> +	}
> +
> +	if (cpu_idx < nr_cpus_needed)
> +		tst_res(TFAIL, "Atleast %d online cpus were not found", nr_cpus_needed);
Also here tst_brk(TCONF, ...)
> +}

Kind regards,
Petr
Shirisha G April 16, 2024, 8:11 a.m. UTC | #3
On Mon, 2024-02-19 at 23:18 +0100, Petr Vorel wrote:
> Hi Shirisha,
> 
> > +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> > +{
> > +	char cpu_state, path_buf[64];
> > +	int total_cpus, cpu_idx, fd, ret, i;
> > +
> > +	total_cpus = get_nprocs_conf();
> > +	cpu_idx = 0;
> > +
> > +	if (get_nprocs() < nr_cpus_needed)
> > +		tst_res(TFAIL, "Atleast online %d cpus are required",
> > nr_cpus_needed);
> s/Atleast/At least/
> (or better: "minimum %d online cpus" ..., but I'm not a native
> speaker.)
will take care in V2
> 
> Also test should exit when not enough online CPU, right? (it's a test
> requirement not a subject of testing). Therefore tst_brk(TCONF, ...)
> should be
> used instead of tst_res(TFAIL).
will take care in V2
> 
> And shouldn't this be a setup function (i.e. run only once, before
> test starts -
> test itself can be run more times, eg 3x with -i3).
I think not because of successive iterations the online cpus state
might change
> 
> > +
> > +	for (i = 0; i < total_cpus && 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, "Unable to open %s: %s",
> > path_buf,
> > +				     strerror(errno));
> > +			}
> > +		}
> > +
> > +		ret = read(fd, &cpu_state, 1);
> > +		if (ret < 1)
> > +			tst_res(TFAIL, "Unable to read %s: %s",
> > path_buf,
> > +			     strerror(errno));
> > +
> > +		if (cpu_state == '1') {
> > +			online_cpus[cpu_idx] = i;
> > +			cpu_idx++;
> > +		}
> > +
> > +		if (fd >= 0)
> > +			SAFE_CLOSE(fd);
> > +	}
> > +
> > +	if (cpu_idx < nr_cpus_needed)
> > +		tst_res(TFAIL, "Atleast %d online cpus were not found",
> > nr_cpus_needed);
> Also here tst_brk(TCONF, ...)
will take care in V2
> > +}
> 
> Kind regards,
> Petr
Shirisha G April 16, 2024, 8:30 a.m. UTC | #4
On Mon, 2024-02-19 at 23:10 +0100, Petr Vorel wrote:
> Hi Shirisha,
> 
> > We are verifying ppc64 kernels prior to 2.6.15-rc5 exhibit a bug in
> > the
> Test is from 2005, for 2.6.15-rc5. Is it really relevant now?
yes
> 
> > 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.
> 
> Please run  make check-hugemmap41 in the test directory and fix
> formatting.
will take care in V2
> 
> > Original test originates from 
> > https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/slbpacaflush.c
> > Signed-off-by: Shirisha G <shirisha@linux.ibm.com>
> > ---
> >  runtest/hugetlb                               |   1 +
> >  testcases/kernel/mem/.gitignore               |   1 +
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 144
> > ++++++++++++++++++
> >  3 files changed, 146 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
> nit: Any reason why not add it as hugemmap33? You don't want to clash
> with other
> sent test, right?
yes
> 
> >  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..3b3388c68
> > --- /dev/null
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
> > @@ -0,0 +1,144 @@
> > +// 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
> > +#include "hugetlb.h"
> > +#define SYSFS_CPU_ONLINE_FMT	"/sys/devices/system/cpu/cpu%d/
> > online"
> > +#define MNTPOINT "hugetlbfs/"
> Could you please have these 2 #define above _GNU_SOURCE?
> (readablility)
will take care in V2
> > +
> > +
> > +#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;
> > +
> > +
> > +
> Please remove these blank lines above.
Sure.will take care in V2
> 
> > +void check_online_cpus(int online_cpus[], int nr_cpus_needed)
> > +{
> > +	char cpu_state, path_buf[64];
> > +	int total_cpus, cpu_idx, fd, ret, i;
> > +
> > +	total_cpus = get_nprocs_conf();
> > +	cpu_idx = 0;
> > +
> > +	if (get_nprocs() < nr_cpus_needed)
> > +		tst_res(TFAIL, "Atleast online %d cpus are required",
> > nr_cpus_needed);
> > +
> > +	for (i = 0; i < total_cpus && cpu_idx < nr_cpus_needed; i++) {
> nit: Maybe just use get_nprocs_conf() directly?
Sure.
> 
> > +		errno = 0;
> Is it errno reset really needed?
Since based on the error number we are failing the testcase so need it
> 
> > +		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, "Unable to open %s: %s",
> > path_buf,
> > +				     strerror(errno));
> We have TERRNO:
> 				tst_res(TFAIL | TERRNO, "Unable to open
> %s: %s", path_buf);
will take care in V2
> > +			}
> > +		}
> > +
> > +		ret = read(fd, &cpu_state, 1);
> > +		if (ret < 1)
> > +			tst_res(TFAIL, "Unable to read %s: %s",
> > path_buf,
> > +			     strerror(errno));
> Maybe use SAFE_READ() ?
Am using read here because unable to read may potentially indicate a
test failure
> > +
> > +		if (cpu_state == '1') {
> > +			online_cpus[cpu_idx] = i;
> > +			cpu_idx++;
> > +		}
> > +
> > +		if (fd >= 0)
> > +			SAFE_CLOSE(fd);
> > +	}
> > +
> > +	if (cpu_idx < nr_cpus_needed)
> > +		tst_res(TFAIL, "Atleast %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(), CPU_SETSIZE/8, &cpu0);
> > +	if (err != 0)
> > +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s",
> > online_cpus[0],
> > +				strerror(errno));
> Again, please use TTERRNO.
Sure. will take care in V2
> > +
> > +	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu1);
> Maybe define CPU_SETSIZE/8 at the top?
Sure.will take care in V2
> > +
> > +	if (err != 0)
> > +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s",
> > online_cpus[1],
> > +				strerror(errno));
> > +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE,
> > MAP_SHARED, fd, 0);
> > +
> > +	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu0);
> > +	if (err != 0)
> > +		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s",
> > online_cpus[0],
> > +				strerror(errno));
> > +	q = (volatile unsigned long *)(p + getpagesize());
> > +	*q = 0xdeadbeef;
> Why to set the address before end of testing? (yes, the original does
> it, but
> why?). Wouldn't be better to use guarded buffers instead?
> 
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#131-guarded-buffers
We are trying to access and write into a memory location within the
mapped segment by adding the huge page size to the mapped address and
casting it to a volatile unsigned long pointer to ensure things work
fine.
> 
> Kind regards,
> Petr
> 
> > +
> > +	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},
> > +};
diff mbox series

Patch

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..3b3388c68
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
@@ -0,0 +1,144 @@ 
+// 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
+#include "hugetlb.h"
+#define SYSFS_CPU_ONLINE_FMT	"/sys/devices/system/cpu/cpu%d/online"
+#define MNTPOINT "hugetlbfs/"
+
+
+#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 total_cpus, cpu_idx, fd, ret, i;
+
+	total_cpus = get_nprocs_conf();
+	cpu_idx = 0;
+
+	if (get_nprocs() < nr_cpus_needed)
+		tst_res(TFAIL, "Atleast online %d cpus are required", nr_cpus_needed);
+
+	for (i = 0; i < total_cpus && 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, "Unable to open %s: %s", path_buf,
+				     strerror(errno));
+			}
+		}
+
+		ret = read(fd, &cpu_state, 1);
+		if (ret < 1)
+			tst_res(TFAIL, "Unable to read %s: %s", path_buf,
+			     strerror(errno));
+
+		if (cpu_state == '1') {
+			online_cpus[cpu_idx] = i;
+			cpu_idx++;
+		}
+
+		if (fd >= 0)
+			SAFE_CLOSE(fd);
+	}
+
+	if (cpu_idx < nr_cpus_needed)
+		tst_res(TFAIL, "Atleast %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(), CPU_SETSIZE/8, &cpu0);
+	if (err != 0)
+		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[0],
+				strerror(errno));
+
+	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu1);
+
+	if (err != 0)
+		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[1],
+				strerror(errno));
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+
+	err = sched_setaffinity(getpid(), CPU_SETSIZE/8, &cpu0);
+	if (err != 0)
+		tst_res(TFAIL, "sched_setaffinity(cpu%d): %s", online_cpus[0],
+				strerror(errno));
+	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},
+};