Re: [PATCH RFC 8/8] mm/huge_memory: walk the folios reversely for the release loop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mm/huge_memory: __folio_split() skips unlocking and putting
  first sub-folio
--

commit 896336ac7b9114a702349806a3b2f69599192c04
Author: Kairui Song <kasong@tencent.com>

mm/huge_memory: walk the folios reversely for the release loop

This commit changes the after-split folio release loop in __folio_split()
to iterate backwards with folio_prev() instead of forwards with
folio_next().

Link: https://lore.kernel.org/r/DKJSFCLP967N.YBR4DNK1NM2N@nvidia.com [1]

> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2ff201a5e5f3b..bb09465a39488 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4366,7 +4366,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	const bool is_anon = folio_test_anon(folio);
>  	const bool is_swapcache = folio_test_swapcache(folio);
>  	int old_order = folio_order(folio);
> -	struct folio *new_folio, *next;
> +	struct folio *new_folio, *lock_folio;
>  	int ret;
>  
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> @@ -4399,9 +4399,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	 * Unlock all after-split folios except the one containing
>  	 * @lock_at page. If @folio is not split, it will be kept locked.
>  	 */
> -	for (new_folio = folio; new_folio != end_folio; new_folio = next) {
> -		next = folio_next(new_folio);
> -		if (new_folio == page_folio(lock_at))
> +	lock_folio = page_folio(lock_at);
> +	for (new_folio = folio_prev(end_folio); new_folio != folio;
> +	     new_folio = folio_prev(new_folio)) {
> +		if (new_folio == lock_folio)
>  			continue;

[Severity: High]
Does stopping the loop at new_folio != folio skip unlocking and putting
@folio when @lock_at belongs to a non-head sub-folio (lock_folio != folio)?

__split_huge_page_to_list_to_order() passes its @page argument as @lock_at:

mm/huge_memory.c:__split_huge_page_to_list_to_order() {
    ...
	return __folio_split(folio, new_order, &folio->page, page, list,
			     SPLIT_TYPE_UNIFORM);
}

When a caller such as try_to_merge_one_page() -> split_huge_page() splits a
large folio on a tail page outside the first sub-folio and then switches to
page_folio(page), the first sub-folio (@folio) is never visited by this
loop and remains permanently locked and referenced, leaking @folio (and its
swap entry if unmapped in swapcache) and hanging any task that later waits
on its folio lock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-swap-thp-split-v1-0-dfa8d4fa448d@tencent.com?part=8