Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 12 Apr 2022 10:59:51 +0200
From:   Oscar Salvador <osalvador@suse.de>
To:     Miaohe Lin <linmiaohe@huawei.com>
Cc:     akpm@linux-foundation.org, ying.huang@intel.com,
        songmuchun@bytedance.com, hch@infradead.org, willy@infradead.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/9] mm/vmscan: remove unneeded can_split_huge_page
 check
Message-ID: <YlU/h0fdE1L846Bd@localhost.localdomain>
References: <20220409093500.10329-1-linmiaohe@huawei.com>
 <20220409093500.10329-3-linmiaohe@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220409093500.10329-3-linmiaohe@huawei.com>
Precedence: bulk

On Sat, Apr 09, 2022 at 05:34:53PM +0800, Miaohe Lin wrote:
> We don't need to check can_split_folio() because folio_maybe_dma_pinned()
> is checked before. It 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. Also activate_locked can be changed to
> keep_locked as it's just short term pinning.

What do you mean by "we can live with short term pinned pages"?
Does it mean that it was not pinned when we check
folio_maybe_dma_pinned() but now it is?

To me it looks like the pinning is fluctuating and we rely on
split_folio_to_list() to see whether we succeed or not, and if not
we give it another spin in the next round?

> Suggested-by: Huang, Ying <ying.huang@intel.com>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/vmscan.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4a76be47bed1..01f5db75a507 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1711,20 +1711,14 @@ static unsigned int shrink_page_list(struct list_head *page_list,
>  					goto keep_locked;
>  				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
> -					 * tail pages can be freed without IO.
> -					 */
> -					if (!folio_entire_mapcount(folio) &&
> -					    split_folio_to_list(folio,
> -								page_list))
> -						goto activate_locked;
> -				}
> +				/*
> +				 * 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) && !folio_entire_mapcount(folio) &&
> +				    split_folio_to_list(folio, page_list))
> +					goto keep_locked;
>  				if (!add_to_swap(page)) {
>  					if (!PageTransHuge(page))
>  						goto activate_locked_split;
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs