linux-kernel - Re: [PATCH 2/8] mm/vmscan: remove unneeded can_split_huge

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: <877d8abjoj.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date:   Thu, 31 Mar 2022 16:38:52 +0800
From:   "Huang, Ying" <ying.huang@...el.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/8] mm/vmscan: remove unneeded can_split_huge_page check

Miaohe Lin <linmiaohe@...wei.com> writes:

> can_split_huge_page is introduced via commit b8f593cd0896 ("mm, THP, swap:
> check whether THP can be split firstly") to avoid deleting the THP from
> the swap cache and freeing the swap cluster when the THP cannot be split.
> But since commit bd4c82c22c36 ("mm, THP, swap: delay splitting THP after
> swapped out"), splitting THP is delayed until THP is swapped out. There's
> no need to delete the THP from the swap cache and free the swap cluster
> anymore. Thus we can remove this unneeded can_split_huge_page check now to
> simplify the code logic.
>
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
>  mm/vmscan.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 7c1a9713bfc9..09b452c4d256 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1691,9 +1691,6 @@ static unsigned int shrink_page_list(struct list_head *page_list,
>  				if (folio_maybe_dma_pinned(folio))
>  					goto keep_locked;
>  				if (PageTransHuge(page)) {
> -					/* cannot split THP, skip it */
> -					if (!can_split_folio(folio, NULL))
> -						goto activate_locked;
>  					/*
>  					 * Split pages without a PMD map right
>  					 * away. Chances are some or all of the

I'm OK with the change itself.  But THP still needs to be split after
being swapped out.  The reason we don't need to check can_split_folio()
is that folio_maybe_dma_pinned() is checked before.  Which will avoid
the long term pinned pages to be swapped out.  And we can live with
short term pinned pages.  Without can_split_folio() checking we can
simplify the code as follows,

	/*
	 * Split pages without a PMD map right
	 * away. Chances are some or all of the
	 * tail pages can be freed without IO.
	 */
	if (PageTransHuge(page) && !compound_mapcount(page) &&
            split_huge_page_to_list(page, page_list))
		goto keep_locked;
                                                                
activate_locked can be changed to keep_locked too, because it's just
short term pinning.

Best Regards,
Huang, Ying