On 16 Feb 2025, at 9:17, Zi Yan wrote:
> On 16 Feb 2025, at 5:32, David Hildenbrand wrote:
>
>> On 11.02.25 16:50, Zi Yan wrote:
>>> folio_split() splits a large folio in the same way as buddy allocator
>>> splits a large free page for allocation. The purpose is to minimize the
>>> number of folios after the split. For example, if user wants to free the
>>> 3rd subpage in a order-9 folio, folio_split() will split the order-9 folio
>>> as:
>>> O-0, O-0, O-0, O-0, O-2, O-3, O-4, O-5, O-6, O-7, O-8 if it is anon,
>>> since anon folio does not support order-1 yet.
>>> -----------------------------------------------------------------
>>> | | | | | | | | |
>>> |O-0|O-0|O-0|O-0| O-2 |...| O-7 | O-8 |
>>> | | | | | | | | |
>>> -----------------------------------------------------------------
>>>
>>> O-1, O-0, O-0, O-2, O-3, O-4, O-5, O-6, O-7, O-9 if it is pagecache
>>> ---------------------------------------------------------------
>>> | | | | | | | |
>>> | O-1 |O-0|O-0| O-2 |...| O-7 | O-8 |
>>> | | | | | | | |
>>> ---------------------------------------------------------------
>>>
>>> It generates fewer folios (i.e., 11 or 10) than existing page split
>>> approach, which splits the order-9 to 512 order-0 folios. It also reduces
>>> the number of new xa_node needed during a pagecache folio split from
>>> 8 to 1, potentially decreasing the folio split failure rate due to memory
>>> constraints.
>>>
>>> folio_split() and existing split_huge_page_to_list_to_order() share
>>> the folio unmapping and remapping code in __folio_split() and the common
>>> backend split code in __split_unmapped_folio() using
>>> uniform_split variable to distinguish their operations.
>>>
>>> uniform_split_supported() and non_uniform_split_supported() are added
>>> to factor out check code and will be used outside __folio_split() in the
>>> following commit.
>>>
>>> Signed-off-by: Zi Yan <z...@nvidia.com>
>>> ---
>>> mm/huge_memory.c | 137 ++++++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 100 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 21ebe2dec5a4..400dfe8a6e60 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -3853,12 +3853,68 @@ static int __split_unmapped_folio(struct folio
>>> *folio, int new_order,
>>> return ret;
>>> }
>>> +static bool non_uniform_split_supported(struct folio *folio, unsigned int
>>> new_order,
>>> + bool warns)
>>> +{
>>> + /* order-1 is not supported for anonymous THP. */
>>> + if (folio_test_anon(folio) && new_order == 1) {
>>> + VM_WARN_ONCE(warns, "Cannot split to order-1 folio");
>>> + return false;
>>> + }
>>> +
>>> + /*
>>> + * No split if the file system does not support large folio.
>>> + * Note that we might still have THPs in such mappings due to
>>> + * CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping
>>> + * does not actually support large folios properly.
>>> + */
>>> + if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>> + !mapping_large_folio_support(folio->mapping)) {
>>
>> In this (and a similar case below), you need
>>
>> if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>> !folio_test_anon(folio) &&
>> !mapping_large_folio_support(folio->mapping)) {
>>
>> Otherwise mapping_large_folio_support() is unhappy:
>>
>
> Thanks. The patch below should fix it.
>
> I am going to send V8, since
> 1. there have been 4 fixes so far for V7, a new series would help people
> review;
>
> 2. based on the discussion with you in THP cabal meeting, to
> convert split_huge_page*() to use __folio_split(), the current
> __folio_split() interface becomes awkward. Two changes are needed:
> a) use in folio offset instead of struct page, since even in
> truncate_inode_partial_folio() I needed to convert in folio offset
> struct page to use my current interface;
> b) split_huge_page*()'s caller might hold the page lock at a non-head
> page, so an additional keep_lock_at_in_folio_offset is needed
> to indicate which after-split folio should be kept locked after
> split is done.
>
Hi Andrew,
I am planing to send V8 to collect all fixup patches I have so far plus
the one below and change folio_split() interface and some of the code.
What is your preferred method?
1. you can pick up the fixup below and I send a new set of patches to
change folio_split();
2. I collect a new V8 with all fixup patches and folio_split() change.
For 1, the commit history might be messy due to my new folio_split()
change. For 2, Minimize xa_node allocation during xarry split [1]
patchset depends on patch 1 of this series, which adds some extra work
for you to collect V8 (alternatively, I can send V8 without patch 1).
Let me know your thoughts. Thanks.
[1] https://lore.kernel.org/linux-mm/20250213034355.516610-1-...@nvidia.com/
>
> From 8b2aa5432c8d726a1fb6ce74c971365650da9370 Mon Sep 17 00:00:00 2001
> From: Zi Yan <z...@nvidia.com>
> Date: Sun, 16 Feb 2025 09:01:29 -0500
> Subject: [PATCH] mm/huge_memory: check folio_test_anon() before
> mapping_large_folio_support()
>
> Otherwise mapping_large_folio_support() complains.
>
> Signed-off-by: Zi Yan <z...@nvidia.com>
> ---
> mm/huge_memory.c | 48 ++++++++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 87cb62c81bf3..deb16fe662c4 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3629,20 +3629,19 @@ static int __split_unmapped_folio(struct folio
> *folio, int new_order,
> bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
> bool warns)
> {
> - /* order-1 is not supported for anonymous THP. */
> - if (folio_test_anon(folio) && new_order == 1) {
> - VM_WARN_ONCE(warns, "Cannot split to order-1 folio");
> - return false;
> - }
> -
> - /*
> - * No split if the file system does not support large folio.
> - * Note that we might still have THPs in such mappings due to
> - * CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping
> - * does not actually support large folios properly.
> - */
> - if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
> + if (folio_test_anon(folio)) {
> + /* order-1 is not supported for anonymous THP. */
> + VM_WARN_ONCE(warns && new_order == 1,
> + "Cannot split to order-1 folio");
> + return new_order != 1;
> + } else if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
> !mapping_large_folio_support(folio->mapping)) {
> + /*
> + * No split if the file system does not support large folio.
> + * Note that we might still have THPs in such mappings due to
> + * CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping
> + * does not actually support large folios properly.
> + */
> VM_WARN_ONCE(warns,
> "Cannot split file folio to non-0 order");
> return false;
> @@ -3662,24 +3661,25 @@ bool non_uniform_split_supported(struct folio *folio,
> unsigned int new_order,
> bool uniform_split_supported(struct folio *folio, unsigned int new_order,
> bool warns)
> {
> - if (folio_test_anon(folio) && new_order == 1) {
> - VM_WARN_ONCE(warns, "Cannot split to order-1 folio");
> - return false;
> - }
> -
> - if (new_order) {
> + if (folio_test_anon(folio)) {
> + VM_WARN_ONCE(warns && new_order == 1,
> + "Cannot split to order-1 folio");
> + return new_order != 1;
> + } else if (new_order) {
> if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
> !mapping_large_folio_support(folio->mapping)) {
> VM_WARN_ONCE(warns,
> "Cannot split file folio to non-0 order");
> return false;
> }
> - if (folio_test_swapcache(folio)) {
> - VM_WARN_ONCE(warns,
> - "Cannot split swapcache folio to non-0 order");
> - return false;
> - }
> }
> +
> + if (new_order && folio_test_swapcache(folio)) {
> + VM_WARN_ONCE(warns,
> + "Cannot split swapcache folio to non-0 order");
> + return false;
> + }
> +
> return true;
> }
>
> --
> 2.47.2
>
>
>
> --
> Best Regards,
> Yan, Zi
--
Best Regards,
Yan, Zi
