linux-kernel - Re: [PATCH 4/5] selftests: prctl: introduce tests for disabling THPs completely

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4dc95e54-e0ef-4919-973a-748845897ef9@gmail.com>
Date: Tue, 29 Jul 2025 23:13:50 +0100
From: Usama Arif <usamaarif642@...il.com>
To: David Hildenbrand <david@...hat.com>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Cc: linux-fsdevel@...r.kernel.org, corbet@....net, rppt@...nel.org,
 surenb@...gle.com, mhocko@...e.com, hannes@...xchg.org, baohua@...nel.org,
 shakeel.butt@...ux.dev, riel@...riel.com, ziy@...dia.com,
 laoar.shao@...il.com, dev.jain@....com, baolin.wang@...ux.alibaba.com,
 npache@...hat.com, lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
 ryan.roberts@....com, vbabka@...e.cz, jannh@...gle.com,
 Arnd Bergmann <arnd@...db.de>, sj@...nel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH 4/5] selftests: prctl: introduce tests for disabling THPs
 completely

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