quoted

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

quoted

+}
+
+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/

quoted

+    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!