[PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely

Usama Arif posted 5 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely
Posted by Usama Arif 2 months, 1 week ago
The test will set the global system THP setting to madvise and
the 2M setting to inherit before it starts (and reset to original
at teardown)

This tests if the process can:
- successfully set and get the policy to disable THPs completely.
- never get a hugepage when the THPs are completely disabled,
  including with MADV_HUGE and MADV_COLLAPSE.
- successfully reset the policy of the process.
- get hugepages only on MADV_HUGE and MADV_COLLAPSE after reset.
- repeat the above tests in a forked process to make sure
  the policy is carried across forks.

Signed-off-by: Usama Arif <usamaarif642@gmail.com>
---
 tools/testing/selftests/mm/.gitignore         |   1 +
 tools/testing/selftests/mm/Makefile           |   1 +
 .../testing/selftests/mm/prctl_thp_disable.c  | 162 ++++++++++++++++++
 3 files changed, 164 insertions(+)
 create mode 100644 tools/testing/selftests/mm/prctl_thp_disable.c

diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index e7b23a8a05fe..eb023ea857b3 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -58,3 +58,4 @@ pkey_sighandler_tests_32
 pkey_sighandler_tests_64
 guard-regions
 merge
+prctl_thp_disable
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index d13b3cef2a2b..2bb8d3ebc17c 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -86,6 +86,7 @@ TEST_GEN_FILES += on-fault-limit
 TEST_GEN_FILES += pagemap_ioctl
 TEST_GEN_FILES += pfnmap
 TEST_GEN_FILES += process_madv
+TEST_GEN_FILES += prctl_thp_disable
 TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += uffd-stress
diff --git a/tools/testing/selftests/mm/prctl_thp_disable.c b/tools/testing/selftests/mm/prctl_thp_disable.c
new file mode 100644
index 000000000000..52f7e6659b1f
--- /dev/null
+++ b/tools/testing/selftests/mm/prctl_thp_disable.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Basic tests for PR_GET/SET_THP_DISABLE prctl calls
+ *
+ * Author(s): Usama Arif <usamaarif642@gmail.com>
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+
+#include "../kselftest_harness.h"
+#include "thp_settings.h"
+#include "vm_util.h"
+
+#ifndef PR_THP_DISABLE_EXCEPT_ADVISED
+#define PR_THP_DISABLE_EXCEPT_ADVISED (1 << 1)
+#endif
+
+#define NR_HUGEPAGES 6
+
+static int sz2ord(size_t size, size_t pagesize)
+{
+	return __builtin_ctzll(size / pagesize);
+}
+
+enum madvise_buffer {
+	NONE,
+	HUGE,
+	COLLAPSE
+};
+
+/*
+ * Function to mmap a buffer, fault it in, madvise it appropriately (before
+ * page fault for MADV_HUGE, and after for MADV_COLLAPSE), and check if the
+ * mmap region is huge.
+ * returns:
+ * 0 if test doesn't give hugepage
+ * 1 if test gives a hugepage
+ * -1 if mmap fails
+ */
+static int test_mmap_thp(enum madvise_buffer madvise_buf, size_t pmdsize)
+{
+	int ret;
+	int buf_size = NR_HUGEPAGES * pmdsize;
+
+	char *buffer = (char *)mmap(NULL, buf_size, PROT_READ | PROT_WRITE,
+				    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (buffer == MAP_FAILED)
+		return -1;
+
+	if (madvise_buf == HUGE)
+		madvise(buffer, buf_size, MADV_HUGEPAGE);
+
+	/* Ensure memory is allocated */
+	memset(buffer, 1, buf_size);
+
+	if (madvise_buf == COLLAPSE)
+		madvise(buffer, buf_size, MADV_COLLAPSE);
+
+	ret = check_huge_anon(buffer, NR_HUGEPAGES, pmdsize);
+	munmap(buffer, buf_size);
+	return ret;
+}
+FIXTURE(prctl_thp_disable_completely)
+{
+	struct thp_settings settings;
+	size_t pmdsize;
+};
+
+FIXTURE_SETUP(prctl_thp_disable_completely)
+{
+	if (!thp_is_enabled())
+		SKIP(return, "Transparent Hugepages not available\n");
+
+	self->pmdsize = read_pmd_pagesize();
+	if (!self->pmdsize)
+		SKIP(return, "Unable to read PMD size\n");
+
+	thp_read_settings(&self->settings);
+	self->settings.thp_enabled = THP_MADVISE;
+	self->settings.hugepages[sz2ord(self->pmdsize, getpagesize())].enabled = THP_INHERIT;
+	thp_save_settings();
+	thp_push_settings(&self->settings);
+}
+
+FIXTURE_TEARDOWN(prctl_thp_disable_completely)
+{
+	thp_restore_settings();
+}
+
+/* prctl_thp_disable_except_madvise fixture sets system THP setting to madvise */
+static void prctl_thp_disable_completely(struct __test_metadata *const _metadata,
+					 size_t pmdsize)
+{
+	int res = 0;
+
+	res = prctl(PR_GET_THP_DISABLE, NULL, NULL, NULL, NULL);
+	ASSERT_EQ(res, 1);
+
+	/* global = madvise, process = never, we shouldn't get HPs even with madvise */
+	res = test_mmap_thp(NONE, pmdsize);
+	ASSERT_EQ(res, 0);
+
+	res = test_mmap_thp(HUGE, pmdsize);
+	ASSERT_EQ(res, 0);
+
+	res = test_mmap_thp(COLLAPSE, pmdsize);
+	ASSERT_EQ(res, 0);
+
+	/* Reset to system policy */
+	res =  prctl(PR_SET_THP_DISABLE, 0, NULL, NULL, NULL);
+	ASSERT_EQ(res, 0);
+
+	/* global = madvise */
+	res = test_mmap_thp(NONE, pmdsize);
+	ASSERT_EQ(res, 0);
+
+	res = test_mmap_thp(HUGE, pmdsize);
+	ASSERT_EQ(res, 1);
+
+	res = test_mmap_thp(COLLAPSE, pmdsize);
+	ASSERT_EQ(res, 1);
+}
+
+TEST_F(prctl_thp_disable_completely, nofork)
+{
+	int res = 0;
+
+	res = prctl(PR_SET_THP_DISABLE, 1, NULL, NULL, NULL);
+	ASSERT_EQ(res, 0);
+
+	prctl_thp_disable_completely(_metadata, self->pmdsize);
+}
+
+TEST_F(prctl_thp_disable_completely, fork)
+{
+	int res = 0, ret = 0;
+	pid_t pid;
+
+	res = prctl(PR_SET_THP_DISABLE, 1, NULL, NULL, NULL);
+	ASSERT_EQ(res, 0);
+
+	/* Make sure prctl changes are carried across fork */
+	pid = fork();
+	ASSERT_GE(pid, 0);
+
+	if (!pid)
+		prctl_thp_disable_completely(_metadata, self->pmdsize);
+
+	wait(&ret);
+	if (WIFEXITED(ret))
+		ret = WEXITSTATUS(ret);
+	else
+		ret = -EINVAL;
+	ASSERT_EQ(ret, 0);
+}
+
+TEST_HARNESS_MAIN
-- 
2.47.3
Re: [PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely
Posted by David Hildenbrand 2 months, 1 week ago
On 25.07.25 18:22, Usama Arif wrote:
> The test will set the global system THP setting to madvise and
> the 2M setting to inherit before it starts (and reset to original
> at teardown)
> 
> This tests if the process can:
> - successfully set and get the policy to disable THPs completely.
> - never get a hugepage when the THPs are completely disabled,
>    including with MADV_HUGE and MADV_COLLAPSE.
> - successfully reset the policy of the process.
> - get hugepages only on MADV_HUGE and MADV_COLLAPSE after reset.
> - repeat the above tests in a forked process to make sure
>    the policy is carried across forks.
> 
> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
> ---
>   tools/testing/selftests/mm/.gitignore         |   1 +
>   tools/testing/selftests/mm/Makefile           |   1 +
>   .../testing/selftests/mm/prctl_thp_disable.c  | 162 ++++++++++++++++++
>   3 files changed, 164 insertions(+)
>   create mode 100644 tools/testing/selftests/mm/prctl_thp_disable.c
> 
> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
> index e7b23a8a05fe..eb023ea857b3 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -58,3 +58,4 @@ pkey_sighandler_tests_32
>   pkey_sighandler_tests_64
>   guard-regions
>   merge
> +prctl_thp_disable
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index d13b3cef2a2b..2bb8d3ebc17c 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -86,6 +86,7 @@ TEST_GEN_FILES += on-fault-limit
>   TEST_GEN_FILES += pagemap_ioctl
>   TEST_GEN_FILES += pfnmap
>   TEST_GEN_FILES += process_madv
> +TEST_GEN_FILES += prctl_thp_disable
>   TEST_GEN_FILES += thuge-gen
>   TEST_GEN_FILES += transhuge-stress
>   TEST_GEN_FILES += uffd-stress
> diff --git a/tools/testing/selftests/mm/prctl_thp_disable.c b/tools/testing/selftests/mm/prctl_thp_disable.c
> new file mode 100644
> index 000000000000..52f7e6659b1f
> --- /dev/null
> +++ b/tools/testing/selftests/mm/prctl_thp_disable.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Basic tests for PR_GET/SET_THP_DISABLE prctl calls
> + *
> + * Author(s): Usama Arif <usamaarif642@gmail.com>
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <sys/prctl.h>
> +#include <sys/wait.h>
> +
> +#include "../kselftest_harness.h"
> +#include "thp_settings.h"
> +#include "vm_util.h"
> +
> +#ifndef PR_THP_DISABLE_EXCEPT_ADVISED
> +#define PR_THP_DISABLE_EXCEPT_ADVISED (1 << 1)
> +#endif

Into patch #2 I guess.

> +
> +#define NR_HUGEPAGES 6
> +
> +static int sz2ord(size_t size, size_t pagesize)
> +{
> +	return __builtin_ctzll(size / pagesize);
> +}
> +
> +enum madvise_buffer {
> +	NONE,
> +	HUGE,
> +	COLLAPSE
> +};

Is that rather something like:

enum thp_collapse_type {
	THP_COLLAPSE_NONE,
	THP_COLLAPSE_MADV_HUGEPAGE, /* MADV_HUGEPAGE before access */
	THP_COLLAPSE_MADV_COLLAPSE, /* MADV_COLLAPSE after access */
};

> +
> +/*
> + * Function to mmap a buffer, fault it in, madvise it appropriately (before
> + * page fault for MADV_HUGE, and after for MADV_COLLAPSE), and check if the
> + * mmap region is huge.
> + * returns:
> + * 0 if test doesn't give hugepage
> + * 1 if test gives a hugepage
> + * -1 if mmap fails
> + */
> +static int test_mmap_thp(enum madvise_buffer madvise_buf, size_t pmdsize)
> +{
> +	int ret;
> +	int buf_size = NR_HUGEPAGES * pmdsize;
> +
> +	char *buffer = (char *)mmap(NULL, buf_size, PROT_READ | PROT_WRITE,
> +				    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

Can we get rid of NR_HUGEPAGES and just use a single one, aligning in a bigger area? This is similar to what we do in cow.c

/* For alignment purposes, we need twice the thp size. */
mmap_size = 2 * pmdsize;
mmap_area = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

if (mmap_area == MAP_FAILED)
	return -errno; /* todo, document that above */

buffer = (char *)(((uintptr_t)mmap_area + pmdsize) & ~(pmdsize - 1));

...

ret = check_huge_anon(buffer, NR_HUGEPAGES, pmdsize);

...

munmap(mmap_area, mmap_size);

> +	if (buffer == MAP_FAILED)
> +		return -1;
> +
> +	if (madvise_buf == HUGE)
> +		madvise(buffer, buf_size, MADV_HUGEPAGE);
> +
> +	/* Ensure memory is allocated */
> +	memset(buffer, 1, buf_size);
> +
> +	if (madvise_buf == COLLAPSE)
> +		madvise(buffer, buf_size, MADV_COLLAPSE);
> +
> +	ret = check_huge_anon(buffer, NR_HUGEPAGES, pmdsize);
> +	munmap(buffer, buf_size);
> +	return ret;
> +}

Empty line missing :)

> +FIXTURE(prctl_thp_disable_completely)
> +{
> +	struct thp_settings settings;
> +	size_t pmdsize;
> +};
> +
> +FIXTURE_SETUP(prctl_thp_disable_completely)
> +{
> +	if (!thp_is_enabled())
> +		SKIP(return, "Transparent Hugepages not available\n");

Heh, not completely correct. enabled != available.

Do we want a thp_available() that is essentially the first part of thp_is_enabled() ?

> +
> +	self->pmdsize = read_pmd_pagesize();
> +	if (!self->pmdsize)
> +		SKIP(return, "Unable to read PMD size\n");
> +
> +	thp_read_settings(&self->settings);
> +	self->settings.thp_enabled = THP_MADVISE;
> +	self->settings.hugepages[sz2ord(self->pmdsize, getpagesize())].enabled = THP_INHERIT;
> +	thp_save_settings();
> +	thp_push_settings(&self->settings);

push without pop, should that be alarming? :)

Can we just use thp_write_settings()? (not sure why that push/pop is required ... is it?)

> +}
> +
> +FIXTURE_TEARDOWN(prctl_thp_disable_completely)
> +{> +	thp_restore_settings();
> +}
> +
> +/* prctl_thp_disable_except_madvise fixture sets system THP setting to madvise */
> +static void prctl_thp_disable_completely(struct __test_metadata *const _metadata,
> +					 size_t pmdsize)
> +{
> +	int res = 0;
> +
> +	res = prctl(PR_GET_THP_DISABLE, NULL, NULL, NULL, NULL);
> +	ASSERT_EQ(res, 1);
> +
> +	/* global = madvise, process = never, we shouldn't get HPs even with madvise */

s/HPs/THPs/

> +	res = test_mmap_thp(NONE, pmdsize);
> +	ASSERT_EQ(res, 0);
> +
> +	res = test_mmap_thp(HUGE, pmdsize);
> +	ASSERT_EQ(res, 0);
> +
> +	res = test_mmap_thp(COLLAPSE, pmdsize);
> +	ASSERT_EQ(res, 0);
> +
> +	/* Reset to system policy */
> +	res =  prctl(PR_SET_THP_DISABLE, 0, NULL, NULL, NULL);
> +	ASSERT_EQ(res, 0);
> +
> +	/* global = madvise */
> +	res = test_mmap_thp(NONE, pmdsize);
> +	ASSERT_EQ(res, 0);
> +
> +	res = test_mmap_thp(HUGE, pmdsize);
> +	ASSERT_EQ(res, 1);
> +
> +	res = test_mmap_thp(COLLAPSE, pmdsize);
> +	ASSERT_EQ(res, 1);


Makes me wonder: should we test for global=always and global=always?

(or simply for all possible values, including global=never if easily possible?)

At least testing with global=always should exercise more possible paths
than global=always (esp., test_mmap_thp(NONE, pmdsize) which would
never apply in madvise mode).


-- 
Cheers,

David / dhildenb
Re: [PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely
Posted by Usama Arif 2 months, 1 week ago
>> +
>> +    self->pmdsize = read_pmd_pagesize();
>> +    if (!self->pmdsize)
>> +        SKIP(return, "Unable to read PMD size\n");
>> +
>> +    thp_read_settings(&self->settings);
>> +    self->settings.thp_enabled = THP_MADVISE;
>> +    self->settings.hugepages[sz2ord(self->pmdsize, getpagesize())].enabled = THP_INHERIT;
>> +    thp_save_settings();
>> +    thp_push_settings(&self->settings);
> 
> push without pop, should that be alarming? :)
> 
> Can we just use thp_write_settings()? (not sure why that push/pop is required ... is it?)
> 

Thanks for the reviews!
Ack on the previous comments, I have fixed them and will include in next revision.
Yes, I think we can replace thp_push_settings with thp_write_settings.

For this, I actually just copied what cow.c and uffd-wp-mremap.c are doing :)

You can see in these 2 files that we do [1]
- thp_read_settings / thp_save_settings
- thp_push_settings

Than we run the experiment

and at the end we do [2]
- thp_restore_settings

i.e. there is no pop.

I think we can change the thp_push_settings to thp_write_settings in [3] and [4] as well?
I can fix and test it if it makes sense. It should prevent people like me from making a
similar mistake when they just copy from it :)

[1] https://elixir.bootlin.com/linux/v6.16/source/tools/testing/selftests/mm/cow.c#L1884
[2] https://elixir.bootlin.com/linux/v6.16/source/tools/testing/selftests/mm/cow.c#L1911 
[3] https://elixir.bootlin.com/linux/v6.16/source/tools/testing/selftests/mm/cow.c#L1886
[4] https://elixir.bootlin.com/linux/v6.16/source/tools/testing/selftests/mm/uffd-wp-mremap.c#L355


>> +}
>> +
>> +FIXTURE_TEARDOWN(prctl_thp_disable_completely)
>> +{> +    thp_restore_settings();
>> +}
>> +
>> +/* prctl_thp_disable_except_madvise fixture sets system THP setting to madvise */
>> +static void prctl_thp_disable_completely(struct __test_metadata *const _metadata,
>> +                     size_t pmdsize)
>> +{
>> +    int res = 0;
>> +
>> +    res = prctl(PR_GET_THP_DISABLE, NULL, NULL, NULL, NULL);
>> +    ASSERT_EQ(res, 1);
>> +
>> +    /* global = madvise, process = never, we shouldn't get HPs even with madvise */
> 
> s/HPs/THPs/
> 
>> +    res = test_mmap_thp(NONE, pmdsize);
>> +    ASSERT_EQ(res, 0);
>> +
>> +    res = test_mmap_thp(HUGE, pmdsize);
>> +    ASSERT_EQ(res, 0);
>> +
>> +    res = test_mmap_thp(COLLAPSE, pmdsize);
>> +    ASSERT_EQ(res, 0);
>> +
>> +    /* Reset to system policy */
>> +    res =  prctl(PR_SET_THP_DISABLE, 0, NULL, NULL, NULL);
>> +    ASSERT_EQ(res, 0);
>> +
>> +    /* global = madvise */
>> +    res = test_mmap_thp(NONE, pmdsize);
>> +    ASSERT_EQ(res, 0);
>> +
>> +    res = test_mmap_thp(HUGE, pmdsize);
>> +    ASSERT_EQ(res, 1);
>> +
>> +    res = test_mmap_thp(COLLAPSE, pmdsize);
>> +    ASSERT_EQ(res, 1);
> 
> 
> Makes me wonder: should we test for global=always and global=always?

Do you mean global=madvise and global=always?> 
> (or simply for all possible values, including global=never if easily possible?)
> 
> At least testing with global=always should exercise more possible paths
> than global=always (esp., test_mmap_thp(NONE, pmdsize) which would
> never apply in madvise mode).
> 

lol I think over here as well you meant madvise in the 2nd instance.

I was just looking at other selftests and I saw FIXTURE_VARIANT_ADD, I think we can
use that to do it without replicating too much code. Let me see if I
can use that and do it for never, madvise and always. If it doesnt help
there might be some code replication, but that should be ok.

Thanks!




Re: [PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely
Posted by David Hildenbrand 2 months, 1 week ago
On 30.07.25 00:13, Usama Arif wrote:
>>> +
>>> +    self->pmdsize = read_pmd_pagesize();
>>> +    if (!self->pmdsize)
>>> +        SKIP(return, "Unable to read PMD size\n");
>>> +
>>> +    thp_read_settings(&self->settings);
>>> +    self->settings.thp_enabled = THP_MADVISE;
>>> +    self->settings.hugepages[sz2ord(self->pmdsize, getpagesize())].enabled = THP_INHERIT;
>>> +    thp_save_settings();
>>> +    thp_push_settings(&self->settings);
>>
>> push without pop, should that be alarming? :)
>>
>> Can we just use thp_write_settings()? (not sure why that push/pop is required ... is it?)
>>
> 
> Thanks for the reviews!
> Ack on the previous comments, I have fixed them and will include in next revision.
> Yes, I think we can replace thp_push_settings with thp_write_settings.
> 
> For this, I actually just copied what cow.c and uffd-wp-mremap.c are doing :)

Right, I see push vs. pop in run_anon_test_case(), but push vs. restore 
from main(). At least cow.c applies a configuration on top of another 
one, so it needs the push+pop semantics.

In your case, we really only perform a single configuration. So 
write+restore should be good enough I guess.

> 
> You can see in these 2 files that we do [1]
> - thp_read_settings / thp_save_settings
> - thp_push_settings
> 
> Than we run the experiment
> 
> and at the end we do [2]
> - thp_restore_settings
> 
> i.e. there is no pop.
> 
> I think we can change the thp_push_settings to thp_write_settings in [3] and [4] as well?

I think we have to push there, so the following push+pop will do the 
right thing (I think that was the whole idea of push+pop).

An alternative would have been to just have write+restore, whereby write 
always returns the old state you save in a local variable.

I wonder if a final pop could be used instead of the restore somehow.

Anyhow, probably best to leave the other test cases alone for now, 
unless you want to clean it up properly :)

[...]

>>> +}
>>> +
>>> +FIXTURE_TEARDOWN(prctl_thp_disable_completely)
>>> +{> +    thp_restore_settings();
>>> +}
>>> +
>>> +/* prctl_thp_disable_except_madvise fixture sets system THP setting to madvise */
>>> +static void prctl_thp_disable_completely(struct __test_metadata *const _metadata,
>>> +                     size_t pmdsize)
>>> +{
>>> +    int res = 0;
>>> +
>>> +    res = prctl(PR_GET_THP_DISABLE, NULL, NULL, NULL, NULL);
>>> +    ASSERT_EQ(res, 1);
>>> +
>>> +    /* global = madvise, process = never, we shouldn't get HPs even with madvise */
>>
>> s/HPs/THPs/
>>
>>> +    res = test_mmap_thp(NONE, pmdsize);
>>> +    ASSERT_EQ(res, 0);
>>> +
>>> +    res = test_mmap_thp(HUGE, pmdsize);
>>> +    ASSERT_EQ(res, 0);
>>> +
>>> +    res = test_mmap_thp(COLLAPSE, pmdsize);
>>> +    ASSERT_EQ(res, 0);
>>> +
>>> +    /* Reset to system policy */
>>> +    res =  prctl(PR_SET_THP_DISABLE, 0, NULL, NULL, NULL);
>>> +    ASSERT_EQ(res, 0);
>>> +
>>> +    /* global = madvise */
>>> +    res = test_mmap_thp(NONE, pmdsize);
>>> +    ASSERT_EQ(res, 0);
>>> +
>>> +    res = test_mmap_thp(HUGE, pmdsize);
>>> +    ASSERT_EQ(res, 1);
>>> +
>>> +    res = test_mmap_thp(COLLAPSE, pmdsize);
>>> +    ASSERT_EQ(res, 1);
>>
>>
>> Makes me wonder: should we test for global=always and global=always?
> 
> Do you mean global=madvise and global=always?>

Yeah, rewrote it 3 times and then messed it up.

>> (or simply for all possible values, including global=never if easily possible?)
>>
>> At least testing with global=always should exercise more possible paths
>> than global=always (esp., test_mmap_thp(NONE, pmdsize) which would
>> never apply in madvise mode).
>>
> 
> lol I think over here as well you meant madvise in the 2nd instance.

Yeah :)

> 
> I was just looking at other selftests and I saw FIXTURE_VARIANT_ADD, I think we can
> use that to do it without replicating too much code. Let me see if I
> can use that and do it for never, madvise and always. If it doesnt help
> there might be some code replication, but that should be ok.

Yeah, some easy way without replicating would be very nice.

-- 
Cheers,

David / dhildenb