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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ple2jysm.fsf@oracle.com>
Date: Mon, 14 Jul 2025 13:35:37 -0700
From: Ankur Arora <ankur.a.arora@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Ankur Arora <ankur.a.arora@...cle.com>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, x86@...nel.org, akpm@...ux-foundation.org,
        bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com,
        mingo@...hat.com, mjguzik@...il.com, luto@...nel.org,
        peterz@...radead.org, acme@...nel.org, namhyung@...nel.org,
        tglx@...utronix.de, willy@...radead.org, raghavendra.kt@....com,
        boris.ostrovsky@...cle.com, konrad.wilk@...cle.com, ziy@...dia.com
Subject: Re: [PATCH v5 12/14] mm: add config option for clearing page-extents


[ Added Zi Yan. ]

David Hildenbrand <david@...hat.com> writes:

> On 11.07.25 19:32, Ankur Arora wrote:
>> David Hildenbrand <david@...hat.com> writes:
>>
>>> On 10.07.25 02:59, Ankur Arora wrote:
>>>> Add CONFIG_CLEAR_PAGE_EXTENT to allow clearing of page-extents
>>>> where architecturally supported.
>>>> This is only available with !CONFIG_HIGHMEM because the intent is to
>>>> use architecture support to clear contiguous extents in a single
>>>> operation (ex. via FEAT_MOPS on arm64, string instructions on x86)
>>>> which excludes any possibility of interspersing kmap()/kunmap().
>>>> Signed-off-by: Ankur Arora <ankur.a.arora@...cle.com>
>>>> ---
>>>
>>> Staring at the next patch, I think this can easily be squashed into the next
>>> patch where you add actual MM core support.
>> I wanted to do this in a separate patch to explicitly document what the
>> responsibility of the interface provided by the architecture is.
>> That said, the commit message didn't actually do a good job of doing
>> that :D.
>> Copying the more detailed commit message from my reply to Andrew,
>> one important part of the clear_pages() is that it be interruptible
>> because clear_pages_resched() implicitly depends on it.
>>
>>> This is only enabled with !CONFIG_HIGHMEM because the intent is
>>> to use architecture support to clear contiguous extents in a
>>> single interruptible operation (ex. via FEAT_MOPS on arm64,
>>> string instructions on x86).
>>
>>> Given that we might be zeroing the whole extent with a single
>>> instruction, this excludes any possibility of constructing
>>> intermediate HIGHMEM maps.
>> Do you think it is best documented in the next patch in a comment
>> instead?
>
> I would just add + document it as part of the next patch.
>
> Looking at the bigger picture now, you introduce
>
> 	ARCH_HAS_CLEAR_PAGES
>
> To say whether an architecture provides clear_pages().
>
> Now we want to conditionally use that to optimize folio_zero_user().
>
> Remind me, why do we want to glue this to THP / HUGETLBFS only? I would assume
> that the code footprint is rather small, and the systems out there that are
> compiled with ARCH_HAS_CLEAR_PAGES but without THP / HUGETLBFS are rather ...
> rare (mostly 32BIT x86 only).

I thought about this some more and there are a few other interfaces that
end up clearing pages:

> clear_highpage()
> clear_highpage_kasan_tagged()
> tag_clear_highpage()

In this set, there are many loops of the form:

   for (i = 0; i < n; i++)
       clear_highpage();

At least some of these (including kernel_init_pages()) could be migrated
to variations on a clear_highpages() which could be:

    static inline void clear_highpages(struct page *page, u32 num_pages)
    {
            if (!IS_ENABLED(CONFIG_HIGHMEM))
                    clear_pages_resched(page, num_pages);
            else
                    for (i = 0; i < num_pages; ++i)
                            clear_highpage(page + i);
    }

(clear_pages_resched() should be safe to be used from here because
everybody using this should be in a schedulable context.)

(The kernel_init_pages() was also suggested by Zi Yan in a review of v3 [1].)

> clear_user_highpage()

Only users folio_zero_user(), __collapse_huge_page_copy() and
userfaultd.

> clear_user_page()
Not many users apart from the highmem interface.

> clear_page()

Not many users apart from the highmem interface.

I'm happy to do this work, just not sure how to stage it. In particular I
would like to avoid a series which tries to address all of the cases.

Maybe it makes sense to handle just add the clear_highpages() variants,
folio_zero_user() handling and some of the obvious users of
clear_highpages() for v6?


Thanks
Ankur

[1] https://lore.kernel.org/lkml/AC2C5344-E655-45BB-B90B-D63C4AC8F2F6@nvidia.com/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ