This change replaces 2 calls to compound_head() with one. This is in
preparation for the conversion of deactivate_page() to
deactivate_folio().
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
mm/damon/paddr.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index e1a4315c4be6..73548bc82297 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -238,15 +238,18 @@ static inline unsigned long damon_pa_mark_accessed_or_deactivate(
for (addr = r->ar.start; addr < r->ar.end; addr += PAGE_SIZE) {
struct page *page = damon_get_page(PHYS_PFN(addr));
+ struct folio *folio;
if (!page)
continue;
+ folio = page_folio(page);
+
if (mark_accessed)
- mark_page_accessed(page);
+ folio_mark_accessed(folio);
else
- deactivate_page(page);
- put_page(page);
- applied++;
+ deactivate_page(&folio->page);
+ folio_put(folio);
+ applied += folio_nr_pages(folio);
}
return applied * PAGE_SIZE;
}
--
2.38.1
On Wed, 7 Dec 2022 00:56:24 +0000 SeongJae Park <[email protected]> wrote:
> Hi Vishal,
>
>
> Thank you for this patch.
>
> Nit pick. Could we please replace 'damon:' on the subject with 'mm/damon:' to
> look more consistent with other DAMON patches?
>
> On Tue, 6 Dec 2022 16:21:57 -0800 "Vishal Moola (Oracle)" <[email protected]> wrote:
>
> > This change replaces 2 calls to compound_head() with one.
>
> I guess you mean _compound_head() calls in page_folio() that called from
> mark_page_accessed() and folio_mark_accessed().
s/folio_mark_accessed()/deactivate_page()/
Sorry for the typo.
Thanks,
SJ
[...]
Hi Vishal,
Thank you for this patch.
Nit pick. Could we please replace 'damon:' on the subject with 'mm/damon:' to
look more consistent with other DAMON patches?
On Tue, 6 Dec 2022 16:21:57 -0800 "Vishal Moola (Oracle)" <[email protected]> wrote:
> This change replaces 2 calls to compound_head() with one.
I guess you mean _compound_head() calls in page_folio() that called from
mark_page_accessed() and folio_mark_accessed(). However, deactivate_page()
calls page_folio() anyway, so this patch doesn't reduce the number of calls to
one but keep the number, correct? Am I missing something? If I'm not, I'd
like to clean up the wording.
> This is in preparation for the conversion of deactivate_page() to
> deactivate_folio().
I think folio_deactivate() might be a more consistent naming. What do you
think?
Also, you may keep the above sentence if you implement folio_deactivate()
first, update deactivate_page() calls to use folio_deactivate(), and finally
remove deactivate_page() definition.
I don't really care about the name and the sequence, though.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
> ---
> mm/damon/paddr.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index e1a4315c4be6..73548bc82297 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -238,15 +238,18 @@ static inline unsigned long damon_pa_mark_accessed_or_deactivate(
>
> for (addr = r->ar.start; addr < r->ar.end; addr += PAGE_SIZE) {
> struct page *page = damon_get_page(PHYS_PFN(addr));
> + struct folio *folio;
>
> if (!page)
> continue;
> + folio = page_folio(page);
One _compound_head() call here,
> +
> if (mark_accessed)
> - mark_page_accessed(page);
> + folio_mark_accessed(folio);
> else
> - deactivate_page(page);
> - put_page(page);
> - applied++;
> + deactivate_page(&folio->page);
And second _compound_head() call here.
> + folio_put(folio);
> + applied += folio_nr_pages(folio);
> }
> return applied * PAGE_SIZE;
> }
> --
> 2.38.1
>
Thanks,
SJ
On Tue, Dec 6, 2022 at 4:56 PM SeongJae Park <[email protected]> wrote:
>
> Hi Vishal,
>
>
> Thank you for this patch.
>
> Nit pick. Could we please replace 'damon:' on the subject with 'mm/damon:' to
> look more consistent with other DAMON patches?
Thanks for looking over this patch! I'll make the subject mm/damon in a v2.
> On Tue, 6 Dec 2022 16:21:57 -0800 "Vishal Moola (Oracle)" <[email protected]> wrote:
>
> > This change replaces 2 calls to compound_head() with one.
>
> I guess you mean _compound_head() calls in page_folio() that called from
> mark_page_accessed() and folio_mark_accessed(). However, deactivate_page()
> calls page_folio() anyway, so this patch doesn't reduce the number of calls to
> one but keep the number, correct? Am I missing something? If I'm not, I'd
> like to clean up the wording.
The 2 calls I was referring to were from mark_page_accessed() and put_page().
As you've noticed, deactivate_page() still calls page_folio() here :).
> > This is in preparation for the conversion of deactivate_page() to
> > deactivate_folio().
>
> I think folio_deactivate() might be a more consistent naming. What do you
> think?
I do like the name folio_deactivate() better than deactivate_folio(), I'll
change that in v2 as well.