| Message ID | 20240811224940.39876-2-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: collect the number of anon mTHP | expand |
On 12.08.24 00:49, Barry Song wrote: > From: Barry Song <v-songbaohua@oppo.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 :) > > Signed-off-by: Barry Song <v-songbaohua@oppo.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. [...] > @@ -447,6 +449,8 @@ static int __folio_migrate_mapping(struct address_space *mapping, > */ > newfolio->index = folio->index; > newfolio->mapping = folio->mapping; > + if (folio_test_anon(folio) && folio_test_large(folio)) > + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); > folio_ref_add(newfolio, nr); /* add cache reference */ > if (folio_test_swapbacked(folio)) { > __folio_set_swapbacked(newfolio); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 84a7154fde93..382c364d3efa 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1084,8 +1084,11 @@ __always_inline bool free_pages_prepare(struct page *page, > (page + i)->flags &= ~PAGE_FLAGS_CHECK_AT_PREP; > } > } > - if (PageMappingFlags(page)) > + if (PageMappingFlags(page)) { > + if (PageAnon(page) && compound) > + mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1); I wonder if you could even drop the "compound" check. mod_mthp_stat would handle order == 0 just fine. Not that I think it makes much difference. Nothing else jumped at me. Acked-by: David Hildenbrand <david@redhat.com>
On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: > > On 12.08.24 00:49, Barry Song wrote: > > From: Barry Song <v-songbaohua@oppo.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@oppo.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? > [...] > > > @@ -447,6 +449,8 @@ static int __folio_migrate_mapping(struct address_space *mapping, > > */ > > newfolio->index = folio->index; > > newfolio->mapping = folio->mapping; > > + if (folio_test_anon(folio) && folio_test_large(folio)) > > + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); > > folio_ref_add(newfolio, nr); /* add cache reference */ > > if (folio_test_swapbacked(folio)) { > > __folio_set_swapbacked(newfolio); > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 84a7154fde93..382c364d3efa 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1084,8 +1084,11 @@ __always_inline bool free_pages_prepare(struct page *page, > > (page + i)->flags &= ~PAGE_FLAGS_CHECK_AT_PREP; > > } > > } > > - if (PageMappingFlags(page)) > > + if (PageMappingFlags(page)) { > > + if (PageAnon(page) && compound) > > + mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1); > > I wonder if you could even drop the "compound" check. mod_mthp_stat > would handle order == 0 just fine. Not that I think it makes much > difference. i think either is fine as mod_mthp_stat will filter out order==0 right now. > > > Nothing else jumped at me. > > Acked-by: David Hildenbrand <david@redhat.com> > Thanks! > -- > Cheers, > > David / dhildenb > Barry
On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@gmail.com> wrote: > > On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: > > > > On 12.08.24 00:49, Barry Song wrote: > > > From: Barry Song <v-songbaohua@oppo.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@oppo.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? 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? 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. However, a page that is read-ahead but not yet mapped can still become an anon, which seems slightly less tricky to count though seems still difficult - except anon pages, shmem can be also swapped-backed? > > > [...] > > > > > @@ -447,6 +449,8 @@ static int __folio_migrate_mapping(struct address_space *mapping, > > > */ > > > newfolio->index = folio->index; > > > newfolio->mapping = folio->mapping; > > > + if (folio_test_anon(folio) && folio_test_large(folio)) > > > + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); > > > folio_ref_add(newfolio, nr); /* add cache reference */ > > > if (folio_test_swapbacked(folio)) { > > > __folio_set_swapbacked(newfolio); > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > > index 84a7154fde93..382c364d3efa 100644 > > > --- a/mm/page_alloc.c > > > +++ b/mm/page_alloc.c > > > @@ -1084,8 +1084,11 @@ __always_inline bool free_pages_prepare(struct page *page, > > > (page + i)->flags &= ~PAGE_FLAGS_CHECK_AT_PREP; > > > } > > > } > > > - if (PageMappingFlags(page)) > > > + if (PageMappingFlags(page)) { > > > + if (PageAnon(page) && compound) > > > + mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1); > > > > I wonder if you could even drop the "compound" check. mod_mthp_stat > > would handle order == 0 just fine. Not that I think it makes much > > difference. > > i think either is fine as mod_mthp_stat will filter out order==0 > right now. > > > > > > > Nothing else jumped at me. > > > > Acked-by: David Hildenbrand <david@redhat.com> > > > > Thanks! > > > -- > > Cheers, > > > > David / dhildenb > > > > Barry Thanks Barry
On 22.08.24 10:44, Barry Song wrote: > On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@gmail.com> wrote: >> >> On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: >>> >>> On 12.08.24 00:49, Barry Song wrote: >>>> From: Barry Song <v-songbaohua@oppo.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@oppo.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". > > However, a page that is read-ahead but not yet mapped can still become > an anon, which seems slightly less tricky to count though seems still > difficult - except anon pages, shmem can be also swapped-backed? Yes. I'm sure there would be ways to achieve it, but I am not sure if it's worth the churn. These pages can be reclaimed easily (I would assume? They are not even mapped and were never accessible via GUP), and can certainly not have any longterm pinnings or similar. There are more like "cached things that could become an anon folio".
On Thu, Aug 22, 2024 at 8:59 PM David Hildenbrand <david@redhat.com> wrote: > > On 22.08.24 10:44, Barry Song wrote: > > On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@gmail.com> wrote: > >> > >> On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: > >>> > >>> On 12.08.24 00:49, Barry Song wrote: > >>>> From: Barry Song <v-songbaohua@oppo.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@oppo.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. 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. to implement that, it seems we do need to detect the moment mapcount==0 and the moment of freeing anon? when mapcount==0 in rmap unmapped_pinned_anon++; when free unmapped_pinned_anon--; Anyway, it seems this is a separate job. > > > > > However, a page that is read-ahead but not yet mapped can still become > > an anon, which seems slightly less tricky to count though seems still > > difficult - except anon pages, shmem can be also swapped-backed? > > Yes. I'm sure there would be ways to achieve it, but I am not sure if > it's worth the churn. These pages can be reclaimed easily (I would > assume? They are not even mapped and were never accessible via GUP), and > can certainly not have any longterm pinnings or similar. There are more > like "cached things that could become an anon folio". Exactly. If no one maps the pages for an extended period, I assume the LRU will reclaim them as well. > > -- > Cheers, > > David / dhildenb Thanks Barry
On 22.08.24 11:21, Barry Song wrote: > On Thu, Aug 22, 2024 at 8:59 PM David Hildenbrand <david@redhat.com> wrote: >> >> On 22.08.24 10:44, Barry Song wrote: >>> On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@gmail.com> wrote: >>>> >>>> On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: >>>>> >>>>> On 12.08.24 00:49, Barry Song wrote: >>>>>> From: Barry Song <v-songbaohua@oppo.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@oppo.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. > > 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.
On Thu, Aug 22, 2024 at 10:01 PM David Hildenbrand <david@redhat.com> wrote: > > On 22.08.24 11:21, Barry Song wrote: > > On Thu, Aug 22, 2024 at 8:59 PM David Hildenbrand <david@redhat.com> wrote: > >> > >> On 22.08.24 10:44, Barry Song wrote: > >>> On Thu, Aug 22, 2024 at 12:52 PM Barry Song <21cnbao@gmail.com> wrote: > >>>> > >>>> On Thu, Aug 22, 2024 at 5:34 AM David Hildenbrand <david@redhat.com> wrote: > >>>>> > >>>>> On 12.08.24 00:49, Barry Song wrote: > >>>>>> From: Barry Song <v-songbaohua@oppo.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@oppo.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
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. + These huge pages could be entirely mapped or have partially + unmapped/unused subpages. + As the system ages, allocating huge pages may be expensive as the system uses memory compaction to copy data around memory to free a huge page for use. There are some counters in ``/proc/vmstat`` to help diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index 4c32058cacfe..2ee2971e4e10 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -126,6 +126,7 @@ enum mthp_stat_item { MTHP_STAT_SPLIT, MTHP_STAT_SPLIT_FAILED, MTHP_STAT_SPLIT_DEFERRED, + MTHP_STAT_NR_ANON, __MTHP_STAT_COUNT }; @@ -136,14 +137,24 @@ struct mthp_stat { DECLARE_PER_CPU(struct mthp_stat, mthp_stats); -static inline void count_mthp_stat(int order, enum mthp_stat_item item) +static inline void mod_mthp_stat(int order, enum mthp_stat_item item, int delta) { if (order <= 0 || order > PMD_ORDER) return; - this_cpu_inc(mthp_stats.stats[order][item]); + this_cpu_add(mthp_stats.stats[order][item], delta); +} + +static inline void count_mthp_stat(int order, enum mthp_stat_item item) +{ + mod_mthp_stat(order, item, 1); } + #else +static inline void mod_mthp_stat(int order, enum mthp_stat_item item, int delta) +{ +} + static inline void count_mthp_stat(int order, enum mthp_stat_item item) { } diff --git a/mm/huge_memory.c b/mm/huge_memory.c index d90d6e94a800..afb911789df8 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -596,6 +596,7 @@ DEFINE_MTHP_STAT_ATTR(shmem_fallback_charge, MTHP_STAT_SHMEM_FALLBACK_CHARGE); DEFINE_MTHP_STAT_ATTR(split, MTHP_STAT_SPLIT); DEFINE_MTHP_STAT_ATTR(split_failed, MTHP_STAT_SPLIT_FAILED); DEFINE_MTHP_STAT_ATTR(split_deferred, MTHP_STAT_SPLIT_DEFERRED); +DEFINE_MTHP_STAT_ATTR(nr_anon, MTHP_STAT_NR_ANON); static struct attribute *anon_stats_attrs[] = { &anon_fault_alloc_attr.attr, @@ -608,6 +609,7 @@ static struct attribute *anon_stats_attrs[] = { &split_attr.attr, &split_failed_attr.attr, &split_deferred_attr.attr, + &nr_anon_attr.attr, NULL, }; @@ -3216,8 +3218,9 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, struct deferred_split *ds_queue = get_deferred_split_queue(folio); /* reset xarray order to new order after split */ XA_STATE_ORDER(xas, &folio->mapping->i_pages, folio->index, new_order); - struct anon_vma *anon_vma = NULL; + bool is_anon = folio_test_anon(folio); struct address_space *mapping = NULL; + struct anon_vma *anon_vma = NULL; int order = folio_order(folio); int extra_pins, ret; pgoff_t end; @@ -3229,7 +3232,7 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, if (new_order >= folio_order(folio)) return -EINVAL; - if (folio_test_anon(folio)) { + if (is_anon) { /* order-1 is not supported for anonymous THP. */ if (new_order == 1) { VM_WARN_ONCE(1, "Cannot split to order-1 folio"); @@ -3269,7 +3272,7 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, if (folio_test_writeback(folio)) return -EBUSY; - if (folio_test_anon(folio)) { + if (is_anon) { /* * The caller does not necessarily hold an mmap_lock that would * prevent the anon_vma disappearing so we first we take a @@ -3382,6 +3385,10 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, } } + if (is_anon) { + mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1); + mod_mthp_stat(new_order, MTHP_STAT_NR_ANON, 1 << (order - new_order)); + } __split_huge_page(page, list, end, new_order); ret = 0; } else { diff --git a/mm/migrate.c b/mm/migrate.c index 7e1267042a56..bde573ec2af8 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -423,6 +423,8 @@ static int __folio_migrate_mapping(struct address_space *mapping, /* No turning back from here */ newfolio->index = folio->index; newfolio->mapping = folio->mapping; + if (folio_test_anon(folio) && folio_test_large(folio)) + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); if (folio_test_swapbacked(folio)) __folio_set_swapbacked(newfolio); @@ -447,6 +449,8 @@ static int __folio_migrate_mapping(struct address_space *mapping, */ newfolio->index = folio->index; newfolio->mapping = folio->mapping; + if (folio_test_anon(folio) && folio_test_large(folio)) + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); folio_ref_add(newfolio, nr); /* add cache reference */ if (folio_test_swapbacked(folio)) { __folio_set_swapbacked(newfolio); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 84a7154fde93..382c364d3efa 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1084,8 +1084,11 @@ __always_inline bool free_pages_prepare(struct page *page, (page + i)->flags &= ~PAGE_FLAGS_CHECK_AT_PREP; } } - if (PageMappingFlags(page)) + if (PageMappingFlags(page)) { + if (PageAnon(page) && compound) + mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1); page->mapping = NULL; + } if (is_check_pages_enabled()) { if (free_page_is_bad(page)) bad++; diff --git a/mm/rmap.c b/mm/rmap.c index a6b9cd0b2b18..b7609920704c 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1467,6 +1467,7 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, } __folio_mod_stat(folio, nr, nr_pmdmapped); + mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, 1); } static __always_inline void __folio_add_file_rmap(struct folio *folio,