| Message ID | 20221213092735.187924-5-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: converted page idle and damon to use folios | expand |
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?
On Tue, 13 Dec 2022 14:40:41 +0000 Matthew Wilcox <willy@infradead.org> wrote: > 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? I'd prefer to simply keep the original behavior, returning NULL, for now unless someone complaints. Thanks, SJ
diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c index 75409601f934..40b79b022129 100644 --- a/mm/damon/ops-common.c +++ b/mm/damon/ops-common.c @@ -13,24 +13,24 @@ #include "ops-common.h" /* - * Get an online page for a pfn if it's in the LRU list. Otherwise, returns + * Get an online folio for a pfn if it's in the LRU list. Otherwise, returns * NULL. * - * The body of this function is stolen from the 'page_idle_get_page()'. We + * The body of this function is stolen from the 'folio_idle_get_folio()'. We * steal rather than reuse it because the code is quite simple. */ -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; - if (unlikely(!PageLRU(page))) { - put_page(page); - page = NULL; + if (unlikely(!folio_test_lru(folio))) { + folio_put(folio); + folio = NULL; } - return page; + return folio; } void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr) diff --git a/mm/damon/ops-common.h b/mm/damon/ops-common.h index 8d82d3722204..4ee607813981 100644 --- a/mm/damon/ops-common.h +++ b/mm/damon/ops-common.h @@ -7,7 +7,13 @@ #include <linux/damon.h> -struct page *damon_get_page(unsigned long pfn); +struct folio *damon_get_folio(unsigned long pfn); +static inline struct page *damon_get_page(unsigned long pfn) +{ + struct folio *folio = damon_get_folio(pfn); + + return &folio->page; +} void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr); void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr);
Introduce a temporary wrapper function damon_get_folio(), which help us to convert damon related function to use folios, and it will be droped once the conversion is completed. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/damon/ops-common.c | 18 +++++++++--------- mm/damon/ops-common.h | 8 +++++++- 2 files changed, 16 insertions(+), 10 deletions(-)