Re: [PATCH -next 4/8] mm: damon: add temporary damon_get_folio()

From: Matthew Wilcox <willy@infradead.org>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Oscar Salvador <osalvador@suse.de>, SeongJae Park <sj@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	damon@lists.linux.dev, vishal.moola@gmail.com
Subject: Re: [PATCH -next 4/8] mm: damon: add temporary damon_get_folio()
Date: Tue, 13 Dec 2022 14:40:41 +0000	[thread overview]
Message-ID: <Y5iO6VSsyDiQQ2mk@casper.infradead.org> (raw)
In-Reply-To: <20221213092735.187924-5-wangkefeng.wang@huawei.com>

On Tue, Dec 13, 2022 at 05:27:31PM +0800, Kefeng Wang wrote:
> -struct page *damon_get_page(unsigned long pfn)
> +struct folio *damon_get_folio(unsigned long pfn)
>  {
> -	struct page *page = pfn_to_online_page(pfn);
> +	struct folio *folio = pfn_to_online_folio(pfn);
>  
> -	if (!page || !PageLRU(page) || !get_page_unless_zero(page))
> +	if (!folio || !folio_test_lru(folio) || !folio_try_get(folio))
>  		return NULL;

Well, this is awkward.  I asked Vishal to think about exactly this problem
and we were going to talk about it after we're both back from vacation
in January.  But I guess we're going to do this in public instead ...

Specifically, what should the semantics be for a putative
damon_get_folio() when it encounters a tail page?  Should it return
the containing folio, or should it return NULL?  And if the semantics
change here to return the containing folio, what adjustments need to
be made to the callers?