Message-ID: <c5aabc87-0f1d-5967-f60e-924322456e60@nvidia.com>
Date:   Mon, 10 Jan 2022 23:49:02 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
Subject: Re: [PATCH v2 27/28] mm: Add isolate_lru_folio()
Content-Language: en-US
To:     "Matthew Wilcox (Oracle)" <willy@infradead.org>, linux-mm@kvack.org
Cc:     Christoph Hellwig <hch@infradead.org>,
        William Kucharski <william.kucharski@oracle.com>,
        linux-kernel@vger.kernel.org, Jason Gunthorpe <jgg@ziepe.ca>
References: <20220110042406.499429-1-willy@infradead.org>
 <20220110042406.499429-28-willy@infradead.org>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220110042406.499429-28-willy@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VUovRmxCR1pHTmtMeXZyZm95Sjh5Tk1TdHdnMllmbE9WYlJjMllZQ0JXdjVl?=
 =?utf-8?B?WEVidngreTQwL1FWU3JGcnM4ajhGRHpZWThEWjhGZ0pEUE1tRFFJMVZlclFN?=
 =?utf-8?B?cDFvbFFLbkJ6LzgrMHhwZk5KSzM5amNibG5pcHVXRkdhaVMxM1ROREdFYnVB?=
 =?utf-8?B?clBPRW9wb2lXUDRDZWdkVUpsYjM1ZDRVZTk2OWFsNnJ0bFAyUDlDOHE1eUtR?=
 =?utf-8?B?YnBMSHRaWDJFSHhXc1BlQjNlRzd6SGZWTWZZYVpKTjFuR3JORWVndjdCWHI5?=
 =?utf-8?B?S3VsQXVUc3hvVjZGTHBYZ0dUc0U5NWNhYktGYmRmWk5VL1lmT0lqcjlFUHFK?=
 =?utf-8?B?WkFraWNCaW9RWGh0c1lEOWxSajZXcjMvSlpMa0Z6S2tnVVJLSnE1bVNyLy85?=
 =?utf-8?B?TU9XTjFjd1NJVVZmMGNUTDYzT0t5K3BMSU41YmcxS3lzNk1XQWpMdUYyajhU?=
 =?utf-8?B?dExXNzNjYlpzNlRLRDZpTG9YTDNyVm56NmlGdVVOMk9qTldjcXBkTnQ4SlE5?=
 =?utf-8?B?azliZFNZc2tjdENaWEhVcVI5Mm5yRFNtU3QwUHI3RVVyVUtha2wycU1wamxI?=
 =?utf-8?B?azQ2bk1wWFdxazJLTDdXZ2RqVjlJTXhVS2JreWxIM1M3VHJlbVVGWVlneWsr?=
 =?utf-8?B?ZXdHWXJaVzZYdFJXY3JZZmR2aXVnU3cwZGlvRWIyK3ovYkhQS08wbGVULzdH?=
 =?utf-8?B?azVsS1p3L2dNMVdnMGhZSkdiT1o5OUFMNFZtWkFCMTRFdlJYaXkxR0Erc1Ry?=
 =?utf-8?B?NzF3b3V3cUhicWcrT2JxMTZCUnJKYlcwNlEwMk0vU0NJTTE2TmtGTTFtWUVY?=
 =?utf-8?B?RTgxQUc2bWFvTVBqcVFwVXhqTXFVTjFBTmVQM3VOVit5c29oWldRVnpqZmZk?=
 =?utf-8?B?NTNJTDg5SFhLRTVrdUNYWDFmN2VzOFdPR3JmMVRwc05JeHBqZ1J5K2xuZmNt?=
 =?utf-8?B?MWFoSVNycXl6b0JGRUpBWnRWdUlaeUZJUWpIcy9JRm1UZGZyTzZrZkNMYkRq?=
 =?utf-8?B?M3dseTFTQmxkVVFPbDd5ekJUYlFPeThmWlJiN0tSVkJ3aDh1TytvWGJBcEto?=
 =?utf-8?B?TUM4ajlQOEQrMFNzZDc0ckdnWHZ2WmQyaFdVY1ZtRDJ5ejNTYnBLRGxRQ0FF?=
 =?utf-8?B?UWhQcThweFhMOFVoTkRlLzZuMENRbkt2aG1uSGYvNXEvY2Y0cmZzNUZkVy9K?=
 =?utf-8?B?dzk0WmRFdGE4WDdGMklWRy96RmpHcDFwbC9saU0vN09Cam5JSG44TFFsUzA2?=
 =?utf-8?B?Rzd5QVQxMy92RmZadW1VclJDYWFlQ0pKcGtNVGNBa2d4amVDMzJIbkowVk5r?=
 =?utf-8?B?Z2dmMGdmbTVQbitTUHFEUi9QUWlWV2ROelFLSC81MHBOOVU1V2dTSk43bGFZ?=
 =?utf-8?B?aXhycmpOYUp4VE9mbHh1aTd0VldIaUVpcTEzK3VObzE1OFF2dW9KMnQ5WGl5?=
 =?utf-8?B?c0Y1eDNqTDNnOWQwR2FDVngyTDJzRDN2OEFycnN1RzRmWnA4NGgxYlp6Ri9S?=
 =?utf-8?B?aUZ1bnVSM2o4S2lQR3FxdHp4SkFRbm9WeXZ2RjBUVmVZV2MrMDQ2Vk5nRyth?=
 =?utf-8?B?S3hTd1pHVnl5RGMvQkJFQi9uMmFUSDc0TFNvdXBkZEdJUE1Eek1MM0RUTkFv?=
 =?utf-8?B?dlJoNjg4cE5YcWEwSU41NUdVa2VFdnpMSGhMV25QbnZkaXdjMG5nejQxZ0N4?=
 =?utf-8?B?YTM1K0JnUWY1dG12MEJSUUV5VVg5SDJuVWl1QktJcm51dVBKMytHUUIyWHBa?=
 =?utf-8?B?ejFxKy9uZTR0dmR4azV5TU5qS2xrWFNrczNSUDdZMTlGNEpJL1JwSDRMMVNu?=
 =?utf-8?B?K1pwdFE2T3RsT21xeXcydFpaK1pTZC94QTZ2d3Z4MGF5ZVNsRkJwZDREV3l2?=
 =?utf-8?B?bGg2TXR3cDRFbythMExYckx4dmNGZURiV1dSWnVCVnBoampLbGUzQzExcFBo?=
 =?utf-8?B?Z0Y3Rjd6TnJJUlkzWFdDRm1laU8raGdNdDhoaUR1dXVQbUJZUFQrR1lpRUFL?=
 =?utf-8?B?bkRpZXhiRElYSGo4eHpRTVJRWERUOGZ3WEd5aDYvYXdNbm5veVdKb0VRTXZ3?=
 =?utf-8?B?Vyt6MW01OE9JbkFUN2k0dWJMbG1waFVsSjVQUWZEN2pUQWNSanhWc1dFbk5B?=
 =?utf-8?B?QVVtcnlKSTNGWVVDb2NucmtVSGdjVm5LTHFMdllNS2cwSjZqUTZLVDRmQ3A0?=
 =?utf-8?Q?yC1WLHVEyp+PiqD31onXxwo=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: afcf472d-1d49-49d7-f4b3-08d9d4d6d6a4
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2022 07:49:04.0110
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JVeO8ukB4B+ZOGRh07dbEzKQzkL6LSp7SPjbVfXI4BaGVKDH7eniETpcSfDzzqB5w1B/dhMX9buENek3h1MsKA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB3062
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/9/22 20:24, Matthew Wilcox (Oracle) wrote:
> Turn isolate_lru_page() into a wrapper around isolate_lru_folio().
> TestClearPageLRU() would have always failed on a tail page, so
> returning -EBUSY is the same behaviour.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   arch/powerpc/include/asm/mmu_context.h |  1 -
>   mm/folio-compat.c                      |  8 +++++
>   mm/internal.h                          |  3 +-
>   mm/vmscan.c                            | 43 ++++++++++++--------------
>   4 files changed, 29 insertions(+), 26 deletions(-)

Reviewed-by: John Hubbard <jhubbard@nvidia.com>


thanks,
-- 
John Hubbard
NVIDIA

> 
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 9ba6b585337f..b9cab0a11421 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -21,7 +21,6 @@ extern void destroy_context(struct mm_struct *mm);
>   #ifdef CONFIG_SPAPR_TCE_IOMMU
>   struct mm_iommu_table_group_mem_t;
>   
> -extern int isolate_lru_page(struct page *page);	/* from internal.h */
>   extern bool mm_iommu_preregistered(struct mm_struct *mm);
>   extern long mm_iommu_new(struct mm_struct *mm,
>   		unsigned long ua, unsigned long entries,
> diff --git a/mm/folio-compat.c b/mm/folio-compat.c
> index 749555a232a8..782e766cd1ee 100644
> --- a/mm/folio-compat.c
> +++ b/mm/folio-compat.c
> @@ -7,6 +7,7 @@
>   #include <linux/migrate.h>
>   #include <linux/pagemap.h>
>   #include <linux/swap.h>
> +#include "internal.h"
>   
>   struct address_space *page_mapping(struct page *page)
>   {
> @@ -151,3 +152,10 @@ int try_to_release_page(struct page *page, gfp_t gfp)
>   	return filemap_release_folio(page_folio(page), gfp);
>   }
>   EXPORT_SYMBOL(try_to_release_page);
> +
> +int isolate_lru_page(struct page *page)
> +{
> +	if (WARN_RATELIMIT(PageTail(page), "trying to isolate tail page"))
> +		return -EBUSY;
> +	return isolate_lru_folio((struct folio *)page);
> +}
> diff --git a/mm/internal.h b/mm/internal.h
> index 9a72d1ecdab4..8b90db90e7f2 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -157,7 +157,8 @@ extern unsigned long highest_memmap_pfn;
>   /*
>    * in mm/vmscan.c:
>    */
> -extern int isolate_lru_page(struct page *page);
> +int isolate_lru_page(struct page *page);
> +int isolate_lru_folio(struct folio *folio);
>   extern void putback_lru_page(struct page *page);
>   extern void reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason);
>   
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index fb9584641ac7..ac2f5b76cdb2 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2168,45 +2168,40 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>   }
>   
>   /**
> - * isolate_lru_page - tries to isolate a page from its LRU list
> - * @page: page to isolate from its LRU list
> + * isolate_lru_folio - Try to isolate a folio from its LRU list.
> + * @folio: Folio to isolate from its LRU list.
>    *
> - * Isolates a @page from an LRU list, clears PageLRU and adjusts the
> - * vmstat statistic corresponding to whatever LRU list the page was on.
> + * Isolate a @folio from an LRU list and adjust the vmstat statistic
> + * corresponding to whatever LRU list the folio was on.
>    *
> - * Returns 0 if the page was removed from an LRU list.
> - * Returns -EBUSY if the page was not on an LRU list.
> - *
> - * The returned page will have PageLRU() cleared.  If it was found on
> - * the active list, it will have PageActive set.  If it was found on
> - * the unevictable list, it will have the PageUnevictable bit set. That flag
> + * The folio will have its LRU flag cleared.  If it was found on the
> + * active list, it will have the Active flag set.  If it was found on the
> + * unevictable list, it will have the Unevictable flag set.  These flags
>    * may need to be cleared by the caller before letting the page go.
>    *
> - * The vmstat statistic corresponding to the list on which the page was
> - * found will be decremented.
> - *
> - * Restrictions:
> + * Context:
>    *
>    * (1) Must be called with an elevated refcount on the page. This is a
> - *     fundamental difference from isolate_lru_pages (which is called
> + *     fundamental difference from isolate_lru_pages() (which is called
>    *     without a stable reference).
> - * (2) the lru_lock must not be held.
> - * (3) interrupts must be enabled.
> + * (2) The lru_lock must not be held.
> + * (3) Interrupts must be enabled.
> + *
> + * Return: 0 if the folio was removed from an LRU list.
> + * -EBUSY if the folio was not on an LRU list.
>    */
> -int isolate_lru_page(struct page *page)
> +int isolate_lru_folio(struct folio *folio)
>   {
> -	struct folio *folio = page_folio(page);
>   	int ret = -EBUSY;
>   
> -	VM_BUG_ON_PAGE(!page_count(page), page);
> -	WARN_RATELIMIT(PageTail(page), "trying to isolate tail page");
> +	VM_BUG_ON_FOLIO(!folio_ref_count(folio), folio);
>   
> -	if (TestClearPageLRU(page)) {
> +	if (folio_test_clear_lru(folio)) {
>   		struct lruvec *lruvec;
>   
> -		get_page(page);
> +		folio_get(folio);
>   		lruvec = folio_lruvec_lock_irq(folio);
> -		del_page_from_lru_list(page, lruvec);
> +		lruvec_del_folio(lruvec, folio);
>   		unlock_page_lruvec_irq(lruvec);
>   		ret = 0;
>   	}