Fix deferred_split_isolate() and clean up __folio_freeze_and_split_unmapped()

[PATCH 0/5] Fix deferred_split_isolate() and clean up __folio_freeze_and_split_unmapped()

Posted by Kiryl Shutsemau 1 month ago

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

deferred_split_isolate() probes each queued folio with folio_try_get().
folio_try_get() failure is treated as a lost race with folio_put().

It leads to wrong results when !folio_try_get() was not caused by
folio_put(): for a frozen folio, PG_partially_mapped gets wrongfully
cleared and the folio dropped from the queue.

It came up in the review of my collapse RFC series:

  https://lore.kernel.org/all/20260824131224.73344-1-lance.yang@linux.dev/

The bug is inert in upstream code:

  - __folio_split() works around it;
  - __folio_migrate_mapping() freezes a folio it is about to replace;
  - reclaim freezes only what try_to_unmap() already unmapped.

No stable@ needed. But my collapse rework steps on it, so it is worth
fixing.

The first patch fixes deferred_split_isolate().

The second patch removes the workaround for this deferred_split_isolate()
behaviour from __folio_freeze_and_split_unmapped().

The other three patches are trivial cleanups in
__folio_freeze_and_split_unmapped() that I stumbled on while looking at
it.

Tested in a VM: split_huge_page_test, folio_split_race_test and cow pass.

Also ran a test that leaves 16 partially mapped THPs on the deferred split
queue and drives thp-deferred_split through debugfs, checking
nr_anon_partially_mapped.

Kiryl Shutsemau (Meta) (5):
  mm/huge_memory: do not touch frozen folios in deferred_split_isolate()
  mm/huge_memory: dequeue the deferred split after the split freeze
  mm/huge_memory: reduce indent level in
    __folio_freeze_and_split_unmapped()
  mm/huge_memory: fold nested ifs in __folio_freeze_and_split_unmapped()
  mm/huge_memory: turn the swapcache-with-mapping error case into an
    assert

 mm/huge_memory.c | 260 ++++++++++++++++++++---------------------------
 1 file changed, 109 insertions(+), 151 deletions(-)


base-commit: 33f61b12d297562321533c048e034b1fb21c1cf3
-- 
2.54.0

Re: [PATCH 0/5] Fix deferred_split_isolate() and clean up __folio_freeze_and_split_unmapped()

Posted by Balbir Singh 1 month ago

On 8/27/26 2:20 AM, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> 
> deferred_split_isolate() probes each queued folio with folio_try_get().
> folio_try_get() failure is treated as a lost race with folio_put().
> 
> It leads to wrong results when !folio_try_get() was not caused by
> folio_put(): for a frozen folio, PG_partially_mapped gets wrongfully
> cleared and the folio dropped from the queue.
> 
> It came up in the review of my collapse RFC series:
> 
>   https://lore.kernel.org/all/20260824131224.73344-1-lance.yang@linux.dev/
> 
> The bug is inert in upstream code:
> 
>   - __folio_split() works around it;
>   - __folio_migrate_mapping() freezes a folio it is about to replace;
>   - reclaim freezes only what try_to_unmap() already unmapped.
> 
> No stable@ needed. But my collapse rework steps on it, so it is worth
> fixing.
> 
> The first patch fixes deferred_split_isolate().
> 
> The second patch removes the workaround for this deferred_split_isolate()
> behaviour from __folio_freeze_and_split_unmapped().
> 
> The other three patches are trivial cleanups in
> __folio_freeze_and_split_unmapped() that I stumbled on while looking at
> it.
> 
> Tested in a VM: split_huge_page_test, folio_split_race_test and cow pass.
> 
> Also ran a test that leaves 16 partially mapped THPs on the deferred split
> queue and drives thp-deferred_split through debugfs, checking
> nr_anon_partially_mapped.
> 
> Kiryl Shutsemau (Meta) (5):
>   mm/huge_memory: do not touch frozen folios in deferred_split_isolate()
>   mm/huge_memory: dequeue the deferred split after the split freeze
>   mm/huge_memory: reduce indent level in
>     __folio_freeze_and_split_unmapped()
>   mm/huge_memory: fold nested ifs in __folio_freeze_and_split_unmapped()
>   mm/huge_memory: turn the swapcache-with-mapping error case into an
>     assert
> 
>  mm/huge_memory.c | 260 ++++++++++++++++++++---------------------------
>  1 file changed, 109 insertions(+), 151 deletions(-)
> 
> 
> base-commit: 33f61b12d297562321533c048e034b1fb21c1cf3

Thanks, any chance you ran hmm-tests on these? If not, I can run them and
provide some testing coverage from a zone device folio perspective

Balbir