Hi Kefeng,
Could we use 'mm/damon:' prefix instead of 'mm: damon:' for the patch subjects?
On Tue, 27 Dec 2022 20:27:09 +0800 Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:
> Introduce damon_get_folio(), and the temporary wrapper function
> damon_get_page(), 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@xxxxxxxxxx>
> ---
> mm/damon/ops-common.c | 14 ++++++++------
> mm/damon/ops-common.h | 8 +++++++-
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 75409601f934..ff38a19aa92e 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -16,21 +16,23 @@
> * Get an online page 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 'page_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;
>
> if (!page || !PageLRU(page) || !get_page_unless_zero(page))
> return NULL;
>
> - if (unlikely(!PageLRU(page))) {
> - put_page(page);
> - page = NULL;
> + folio = page_folio(page);
> + if (unlikely(!folio_test_lru(folio))) {
> + folio_put(folio);
> + folio = NULL;
> }
I think Matthew's comment for 'page_idle_get_folio()'[1] could applied here?
[1] https://lore.kernel.org/damon/Y6s2HPjrON2Sx%2Fgr@xxxxxxxxxxxxxxxxxxxx/
> - 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;
I think we should check if folio is NULL before dereferencing it?
> +}
>
> 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);
> --
> 2.35.3
>
