[PATCH] Docs/mm/damon/design: describe the differences for pageout's virtual address and physcial address

[PATCH] Docs/mm/damon/design: describe the differences for pageout's virtual address and physcial address

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

The doc needs update. As only in the case of virtual address, we are
calling madvise() with MADV_PAGEOUT; but in the case of physical
address, we are actually calling reclaim_pages() directly. And
madvise() with MADV_PAGEOUT is much more aggresive on reclamation
compared to reclaim_pages() on paddr.

Cc: SeongJae Park <sj@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 Documentation/mm/damon/design.rst | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 8c89d26f0baa..6c50743552f5 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -315,7 +315,13 @@ that supports each action are as below.
    Supported by ``vaddr`` and ``fvaddr`` operations set.
  - ``cold``: Call ``madvise()`` for the region with ``MADV_COLD``.
    Supported by ``vaddr`` and ``fvaddr`` operations set.
- - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT``.
+ - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT`` if
+   the region is using virtual address; otherwise, call ``reclaim_pages()``
+   for the region if physical address is used.
+   ``madvise()``  with ``MADV_PAGEOUT``  is aggressively reclaiming memory
+   when pages are not shared by multiple processes. Differently, paddr's
+   pageout  will do the best-effort reclamation by scanning PTEs to figure
+   out if pages are young, if not, pages are reclaimed.
    Supported by ``vaddr``, ``fvaddr`` and ``paddr`` operations set.
  - ``hugepage``: Call ``madvise()`` for the region with ``MADV_HUGEPAGE``.
    Supported by ``vaddr`` and ``fvaddr`` operations set.
-- 
2.34.1

Re: [PATCH] Docs/mm/damon/design: describe the differences for pageout's virtual address and physcial address

[SeongJae Park]

Hi Barry,

On Sun, 25 Feb 2024 10:50:23 +1300 Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <v-songbaohua@oppo.com>
> 
> The doc needs update. As only in the case of virtual address, we are
> calling madvise() with MADV_PAGEOUT; but in the case of physical
> address, we are actually calling reclaim_pages() directly. And
> madvise() with MADV_PAGEOUT is much more aggresive on reclamation
> compared to reclaim_pages() on paddr.

Thank you for this nice patch :)

> 
> Cc: SeongJae Park <sj@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---
>  Documentation/mm/damon/design.rst | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> index 8c89d26f0baa..6c50743552f5 100644
> --- a/Documentation/mm/damon/design.rst
> +++ b/Documentation/mm/damon/design.rst
> @@ -315,7 +315,13 @@ that supports each action are as below.
>     Supported by ``vaddr`` and ``fvaddr`` operations set.
>   - ``cold``: Call ``madvise()`` for the region with ``MADV_COLD``.
>     Supported by ``vaddr`` and ``fvaddr`` operations set.
> - - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT``.
> + - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT`` if
> +   the region is using virtual address; otherwise, call ``reclaim_pages()``
> +   for the region if physical address is used.
> +   ``madvise()``  with ``MADV_PAGEOUT``  is aggressively reclaiming memory
> +   when pages are not shared by multiple processes. Differently, paddr's
> +   pageout  will do the best-effort reclamation by scanning PTEs to figure
> +   out if pages are young, if not, pages are reclaimed.

I think this might be too much details.  Especially, explanation of
``MADV_PAGEOUT`` might better to be done on it's document.  We wouldn't want to
update two places for future changes to ``MADV_PAGEOUT``.  What do you think
about making it simple and independent from the implementation detail?  For
example,

    - - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT``.
    + - ``pageout``: Reclaim the region.

Thanks,
SJ

>     Supported by ``vaddr``, ``fvaddr`` and ``paddr`` operations set.
>   - ``hugepage``: Call ``madvise()`` for the region with ``MADV_HUGEPAGE``.
>     Supported by ``vaddr`` and ``fvaddr`` operations set.
> -- 
> 2.34.1

Re: [PATCH] Docs/mm/damon/design: describe the differences for pageout's virtual address and physcial address

[Barry Song]

On Sun, Feb 25, 2024 at 11:08 AM SeongJae Park <sj@kernel.org> wrote:
>
> Hi Barry,
>
> On Sun, 25 Feb 2024 10:50:23 +1300 Barry Song <21cnbao@gmail.com> wrote:
>
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > The doc needs update. As only in the case of virtual address, we are
> > calling madvise() with MADV_PAGEOUT; but in the case of physical
> > address, we are actually calling reclaim_pages() directly. And
> > madvise() with MADV_PAGEOUT is much more aggresive on reclamation
> > compared to reclaim_pages() on paddr.
>
> Thank you for this nice patch :)
>
> >
> > Cc: SeongJae Park <sj@kernel.org>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > ---
> >  Documentation/mm/damon/design.rst | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> > index 8c89d26f0baa..6c50743552f5 100644
> > --- a/Documentation/mm/damon/design.rst
> > +++ b/Documentation/mm/damon/design.rst
> > @@ -315,7 +315,13 @@ that supports each action are as below.
> >     Supported by ``vaddr`` and ``fvaddr`` operations set.
> >   - ``cold``: Call ``madvise()`` for the region with ``MADV_COLD``.
> >     Supported by ``vaddr`` and ``fvaddr`` operations set.
> > - - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT``.
> > + - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT`` if
> > +   the region is using virtual address; otherwise, call ``reclaim_pages()``
> > +   for the region if physical address is used.
> > +   ``madvise()``  with ``MADV_PAGEOUT``  is aggressively reclaiming memory
> > +   when pages are not shared by multiple processes. Differently, paddr's
> > +   pageout  will do the best-effort reclamation by scanning PTEs to figure
> > +   out if pages are young, if not, pages are reclaimed.
>
> I think this might be too much details.  Especially, explanation of
> ``MADV_PAGEOUT`` might better to be done on it's document.  We wouldn't want to
> update two places for future changes to ``MADV_PAGEOUT``.  What do you think
> about making it simple and independent from the implementation detail?  For
> example,
>
>     - - ``pageout``: Call ``madvise()`` for the region with ``MADV_PAGEOUT``.
>     + - ``pageout``: Reclaim the region.

make sense. as someday we might rename reclaim_pages to reclaim_folios or
something else. and we might not use madvise() for vaddr any more, who knows:-)
so it is more sensible to remove the details.
I will send v2 to follow your proposal.

>
> Thanks,
> SJ
>
> >     Supported by ``vaddr``, ``fvaddr`` and ``paddr`` operations set.
> >   - ``hugepage``: Call ``madvise()`` for the region with ``MADV_HUGEPAGE``.
> >     Supported by ``vaddr`` and ``fvaddr`` operations set.
> > --
> > 2.34.1

Thanks
Barry