Hi Barry,
On Sun, 25 Feb 2024 10:50:23 +1300 Barry Song <21cnbao@xxxxxxxxx> wrote:
> From: Barry Song <v-songbaohua@xxxxxxxx>
>
> 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@xxxxxxxxxx>
> Cc: Minchan Kim <minchan@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
> ---
> 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
