| Message ID | 20240224215023.5271-1-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Docs/mm/damon/design: describe the differences for pageout's virtual address and physcial address | expand |
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
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
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.