Message-ID: <CAGsJ_4xjvid+JTbDwJJ9P4PkC2t6SWxyOkLq30-gbicNM6BSLw@mail.gmail.com>
Date: Thu, 22 Aug 2024 22:12:35 +1200
From: Barry Song <21cnbao@...il.com>
To: David Hildenbrand <david@...hat.com>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org, 
	baolin.wang@...ux.alibaba.com, chrisl@...nel.org, hanchuanhua@...o.com, 
	ioworker0@...il.com, kaleshsingh@...gle.com, kasong@...cent.com, 
	linux-kernel@...r.kernel.org, ryan.roberts@....com, v-songbaohua@...o.com, 
	ziy@...dia.com, yuanshuai@...o.com
Subject: Re: [PATCH v2 1/2] mm: collect the number of anon large folios

On Thu, Aug 22, 2024 at 10:01 PM David Hildenbrand <david@...hat.com> wrote:
>
> On 22.08.24 11:21, Barry Song wrote:
> > On Thu, Aug 22, 2024 at 8:59 PM David Hildenbrand <david@...hat.com> wrote:
> >>
> >> On 22.08.24 10:44, Barry Song wrote:
> >>> On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@...il.com> wrote:
> >>>>
> >>>> On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@...hat.com> wrote:
> >>>>>
> >>>>> On 12.08.24 00:49, Barry Song wrote:
> >>>>>> From: Barry Song <v-songbaohua@...o.com>
> >>>>>>
> >>>>>> Anon large folios come from three places:
> >>>>>> 1. new allocated large folios in PF, they will call folio_add_new_anon_rmap()
> >>>>>> for rmap;
> >>>>>> 2. a large folio is split into multiple lower-order large folios;
> >>>>>> 3. a large folio is migrated to a new large folio.
> >>>>>>
> >>>>>> In all above three counts, we increase nr_anon by 1;
> >>>>>>
> >>>>>> Anon large folios might go either because of be split or be put
> >>>>>> to free, in these cases, we reduce the count by 1.
> >>>>>>
> >>>>>> Folios that have been added to the swap cache but have not yet received
> >>>>>> an anon mapping won't be counted. This is consistent with the AnonPages
> >>>>>> statistics in /proc/meminfo.
> >>>>>
> >>>>> Thinking out loud, I wonder if we want to have something like that for
> >>>>> any anon folios (including small ones).
> >>>>>
> >>>>> Assume we longterm-pinned an anon folio and unmapped/zapped it. It would
> >>>>> be quite interesting to see that these are actually anon pages still
> >>>>> consuming memory. Same with memory leaks, when an anon folio doesn't get
> >>>>> freed for some reason.
> >>>>>
> >>>>> The whole "AnonPages" counter thingy is just confusing, it only counts
> >>>>> what's currently mapped ... so we'd want something different.
> >>>>>
> >>>>> But it's okay to start with large folios only, there we have a new
> >>>>> interface without that legacy stuff :)
> >>>>
> >>>> We have two options to do this:
> >>>> 1. add a new separate nr_anon_unmapped interface which
> >>>> counts unmapped anon memory only
> >>>> 2. let the nr_anon count both mapped and unmapped anon
> >>>> folios.
> >>>>
> >>>> I would assume 1 is clearer as right now AnonPages have been
> >>>> there for years. and counting all mapped and unmapped together,
> >>>> we are still lacking an approach to find out anon memory leak
> >>>> problem you mentioned.
> >>>>
> >>>> We are right now comparing nr_anon(including mapped folios only)
> >>>> with AnonPages to get the distribution of different folio sizes in
> >>>> performance profiling.
> >>>>
> >>>> unmapped_nr_anon should be normally always quite small. otherwise,
> >>>> something must be wrong.
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Barry Song <v-songbaohua@...o.com>
> >>>>>> ---
> >>>>>>     Documentation/admin-guide/mm/transhuge.rst |  5 +++++
> >>>>>>     include/linux/huge_mm.h                    | 15 +++++++++++++--
> >>>>>>     mm/huge_memory.c                           | 13 ++++++++++---
> >>>>>>     mm/migrate.c                               |  4 ++++
> >>>>>>     mm/page_alloc.c                            |  5 ++++-
> >>>>>>     mm/rmap.c                                  |  1 +
> >>>>>>     6 files changed, 37 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> >>>>>> index 058485daf186..9fdfb46e4560 100644
> >>>>>> --- a/Documentation/admin-guide/mm/transhuge.rst
> >>>>>> +++ b/Documentation/admin-guide/mm/transhuge.rst
> >>>>>> @@ -527,6 +527,11 @@ split_deferred
> >>>>>>             it would free up some memory. Pages on split queue are going to
> >>>>>>             be split under memory pressure, if splitting is possible.
> >>>>>>
> >>>>>> +nr_anon
> >>>>>> +       the number of anon huge pages we have in the whole system.
> >>>>>
> >>>>> "transparent ..." otherwise people might confuse it with anon hugetlb
> >>>>> "huge pages" ... :)
> >>>>>
> >>>>> I briefly tried coming up with a better name than "nr_anon" but failed.
> >>>>>
> >>>>>
> >>>>
> >>>> if we might have unmapped_anon counter later, maybe rename it to
> >>>> nr_anon_mapped? and the new interface we will have in the future
> >>>> might be nr_anon_unmapped?
> >>
> >> We really shouldn't be using the mapped/unmapped terminology here ... we
> >> allocated pages and turned them into anonymous folios. At some point we
> >> free them. That's the lifecycle.
> >>
> >>>
> >>> On second thought, this might be incorrect as well. Concepts like 'anon',
> >>> 'shmem', and 'file' refer to states after mapping. If an 'anon' has been
> >>> unmapped but is still pinned and not yet freed, it isn't technically an
> >>> 'anon' anymore?
> >>
> >> It's just not mapped, and cannot get mapped, anymore. In the memdesc
> >> world, we'd be freeing the "struct anon" or "struct folio" once the last
> >> refcount goes to 0, not once (e.g., temporarily during a failed
> >> migration?) unmapped.
> >>
> >> The important part to me would be: this is memory that was allocated for
> >> anonymous memory, and it's still around for some reason and not getting
> >> freed. Usually, we would expect anon memory to get freed fairly quickly
> >> once unmapped. Except when there are long-term pinnings or other types
> >> of memory leaks.
> >>
> >> You could happily continue using these anon pages via vmsplice() or
> >> similar, even thought he original page table mapping was torn down.
> >>
> >>>
> >>> On the other hand, implementing nr_anon_unmapped could be extremely
> >>> tricky. I have no idea how to implement it as we are losing those mapping
> >>> flags.
> >>
> >> folio_mapcount() can tell you efficiently whether a folio is mapped or
> >> not -- and that information will stay for all eternity as long as we
> >> have any mapcounts :) . It cannot tell "how many" pages of a large folio
> >> are mapped, but at least "is any page of this large folio mapped".
> >
> > Exactly. AnonPages decreases by -1 when removed from the rmap,
> > whereas nr_anon decreases by -1 when an anon folio is freed. So,
> > I would assume nr_anon includes those pinned and unmapped anon
> > folios but AnonPages doesn't.
>
> Right, note how internally it is called "NR_ANON_MAPPED", but we ended
> up calling it "AnonPages". But that's rather a legacy interface we
> cannot change (fix) that easily. At least not without a config option.
>
> At some point it might indeed be interesting to have "nr_anon_mapped",
> here, but that would correspond to "is any part of this large folio
> mapped". For debugging purposes in the future, that might be indeed
> interesting.
>
> "nr_anon": anon allocations (until freed -> +1)
> "nr_anon_mapped": anon allocations that are mapped (any part mapped -> +1)
> "nr_anon_partially_mapped": anon allocations that was detected to be
> partially mapped at some point -> +1
>
> If a folio is in the swapcache, I would still want to see that it is an
> anon allocation lurking around in the system. Like we do with pagecache
> pages. *There* we do have the difference between "allocated" and
> "mapped" already.
>
> So likely, calling it "nr_anon" here, and tracking it on an allocation
> level, is good enough for now and future proof.

Right. I plan to send v3 tomorrow to at least unblock Usama's series,
in case he wants to rebase on top of it.

>
> >
> > If there's a significant amount of 'leaked' anon, we should consider
> > having a separate counter for them. For instance, if nr_anon is
> > 100,000 and pinned/unmapped pages account for 50%, then nr_anon
> > alone doesn’t effectively reflect the system's state.
>
> Right, but if you stare at the system you could tell that a significant
> amount of memory is still getting consumed through existing/previous
> anon mappings. Depends on how valuable that distinction really is.
>
> --
> Cheers,
>
> David / dhildenb
>

Thanks
Barry

Powered by blists - more mailing lists