Re: [PATCH v5 12/14] mm: add config option for clearing page-extents

[Ankur Arora]

[ Added Zi Yan. ]

David Hildenbrand <david@xxxxxxxxxx> writes:

> On 11.07.25 19:32, Ankur Arora wrote:
>> David Hildenbrand <david@xxxxxxxxxx> 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@xxxxxxxxxx>
>>>> ---
>>>
>>> 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@xxxxxxxxxx/