| 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
| ||
|
Message-ID: <7b13d8b5-a534-47f8-b6c5-09a65bffc691@redhat.com>
Date: Thu, 31 Jul 2025 21:42:30 +0200
From: David Hildenbrand <david@...hat.com>
To: Usama Arif <usamaarif642@...il.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 v2 4/5] selftests: prctl: introduce tests for disabling
THPs completely
On 31.07.25 14:27, Usama Arif wrote:
> The test will set the global system THP setting to never, madvise
> or always depending on the fixture variant 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
> with the prctl, including with MADV_HUGE and MADV_COLLAPSE.
> - successfully reset the policy of the process.
> - after reset, only get hugepages with:
> - MADV_COLLAPSE when policy is set to never.
> - MADV_HUGE and MADV_COLLAPSE when policy is set to madvise.
> - always when policy is set to "always".
> - repeat the above tests in a forked process to make sure
> the policy is carried across forks.
>
> Signed-off-by: Usama Arif <usamaarif642@...il.com>
> ---
[...]
Looks much better already. Some quirks.
> +#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"
> +
> +static int sz2ord(size_t size, size_t pagesize)
> +{
> + return __builtin_ctzll(size / pagesize);
> +}
> +
> +enum thp_collapse_type {
> + THP_COLLAPSE_NONE,
> + THP_COLLAPSE_MADV_HUGEPAGE, /* MADV_HUGEPAGE before access */
> + THP_COLLAPSE_MADV_COLLAPSE, /* MADV_COLLAPSE after access */
> +};
> +
> +enum thp_policy {
> + THP_POLICY_NEVER,
> + THP_POLICY_MADVISE,
> + THP_POLICY_ALWAYS,
> +};
Couldn't you have reused "enum thp_enabled" end simply never specified
the "THP_INHERIT"? Then, you need to do less translation.
> +
> +struct test_results {
> + int prctl_get_thp_disable;
The result is always one, does that here make sense?
> + int prctl_applied_collapse_none;
"prctl_applied" is a bit confusing. And most of these always have the
same value.
Can't we special case the remaining two cases on the current policy and
avoid this struct compeltely?
> + int prctl_applied_collapse_madv_huge;
> + int prctl_applied_collapse_madv_collapse;
> + int prctl_removed_collapse_none;
> + int prctl_removed_collapse_madv_huge;
> + int prctl_removed_collapse_madv_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
> + * -errno if mmap fails
> + */
> +static int test_mmap_thp(enum thp_collapse_type madvise_buf, size_t pmdsize)
> +{
> + char *mem, *mmap_mem;
> + size_t mmap_size;
> + int ret;
> +
> + /* For alignment purposes, we need twice the THP size. */
> + mmap_size = 2 * pmdsize;
> + mmap_mem = (char *)mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (mmap_mem == MAP_FAILED)
> + return -errno;
> +
> + /* We need a THP-aligned memory area. */
> + mem = (char *)(((uintptr_t)mmap_mem + pmdsize) & ~(pmdsize - 1));
> +
> + if (madvise_buf == THP_COLLAPSE_MADV_HUGEPAGE)
> + madvise(mem, pmdsize, MADV_HUGEPAGE);
> +
> + /* Ensure memory is allocated */
> + memset(mem, 1, pmdsize);
> +
> + if (madvise_buf == THP_COLLAPSE_MADV_COLLAPSE)
> + madvise(mem, pmdsize, MADV_COLLAPSE);
> +
To avoid even mmap_mem to get merged with some other VMA, maybe just do
before reading the smap here:
/* HACK: make sure we have a separate VMA that we can check reliably. */
mprotect(mem, pmdsize, PROT_READ);
or
madvise(mem, pmdsize, MADV_DONTFORK);
before reading smaps.
That is probably the easiest approach. The you can drop the lengthy
comment and perform a single thp check.
[...]
> +
> +static void prctl_thp_disable_test(struct __test_metadata *const _metadata,
> + size_t pmdsize, struct test_results *results)
> +{
> +
> + ASSERT_EQ(prctl(PR_GET_THP_DISABLE, NULL, NULL, NULL, NULL),
> + results->prctl_get_thp_disable);
> +
> + /* tests after prctl overrides global policy */
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_NONE, pmdsize),
> + results->prctl_applied_collapse_none);
> +
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_MADV_HUGEPAGE, pmdsize),
> + results->prctl_applied_collapse_madv_huge);
> +
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_MADV_COLLAPSE, pmdsize),
> + results->prctl_applied_collapse_madv_collapse);
> +
> + /* Reset to global policy */
> + ASSERT_EQ(prctl(PR_SET_THP_DISABLE, 0, NULL, NULL, NULL), 0);
> +
> + /* tests after prctl is cleared, and only global policy is effective */
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_NONE, pmdsize),
> + results->prctl_removed_collapse_none);
> +
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_MADV_HUGEPAGE, pmdsize),
> + results->prctl_removed_collapse_madv_huge);
> +
> + ASSERT_EQ(test_mmap_thp(THP_COLLAPSE_MADV_COLLAPSE, pmdsize),
> + results->prctl_removed_collapse_madv_collapse);
> +}
> +
> +FIXTURE(prctl_thp_disable_completely)
> +{
> + struct thp_settings settings;
> + struct test_results results;
Is this "expected_results" ?
But again, hopefully we can remove that and instead just base it on the
polocy that we configured.
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists