| Message ID | 20240409082631.187483-6-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | large folios swap-in: handle refault cases first | expand |
Hi Barry, On Tue, 9 Apr 2024 20:26:31 +1200 Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <v-songbaohua@oppo.com> > > Currently, we are handling the scenario where we've hit a > large folio in the swapcache, and the reclaiming process > for this large folio is still ongoing. > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > --- > include/linux/huge_mm.h | 1 + > mm/huge_memory.c | 2 ++ > mm/memory.c | 1 + > 3 files changed, 4 insertions(+) > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index c8256af83e33..b67294d5814f 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -269,6 +269,7 @@ enum mthp_stat_item { > MTHP_STAT_ANON_ALLOC_FALLBACK, > MTHP_STAT_ANON_SWPOUT, > MTHP_STAT_ANON_SWPOUT_FALLBACK, > + MTHP_STAT_ANON_SWPIN_REFAULT, > __MTHP_STAT_COUNT > }; > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index d8d2ed80b0bf..fb95345b0bde 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > > static struct attribute *stats_attrs[] = { > &anon_alloc_attr.attr, > &anon_alloc_fallback_attr.attr, > &anon_swpout_attr.attr, > &anon_swpout_fallback_attr.attr, > + &anon_swpin_refault_attr.attr, > NULL, > }; > > diff --git a/mm/memory.c b/mm/memory.c > index 9818dc1893c8..acc023795a4d 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > nr_pages = nr; > entry = folio->swap; > page = &folio->page; > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > } From the latest mm-unstable tree, I get below kunit build failure and 'git bisect' points this patch. $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/ [16:07:40] Configuring KUnit Kernel ... [16:07:40] Building KUnit Kernel ... Populating config with: $ make ARCH=um O=../kunit.out/ olddefconfig Building with: $ make ARCH=um O=../kunit.out/ --jobs=36 ERROR:root:.../mm/memory.c: In function ‘do_swap_page’: .../mm/memory.c:4169:17: error: implicit declaration of function ‘count_mthp_stat’ [-Werror=implicit-function-declaration] 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); | ^~~~~~~~~~~~~~~ .../mm/memory.c:4169:53: error: ‘MTHP_STAT_ANON_SWPIN_REFAULT’ undeclared (first use in this function) 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ .../mm/memory.c:4169:53: note: each undeclared identifier is reported only once for each function it appears in cc1: some warnings being treated as errors My kunit build config doesn't have CONFIG_TRANSPARE_HUGEPAGE. Maybe that's the reason and this patch, or the patch that introduced the function and the enum need to take care of the case? Thanks, SJ [...]
>> + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); >> } >> > From the latest mm-unstable tree, I get below kunit build failure and > 'git bisect' points this patch. > > $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/ > [16:07:40] Configuring KUnit Kernel ... > [16:07:40] Building KUnit Kernel ... > Populating config with: > $ make ARCH=um O=../kunit.out/ olddefconfig > Building with: > $ make ARCH=um O=../kunit.out/ --jobs=36 > ERROR:root:.../mm/memory.c: In function ‘do_swap_page’: > .../mm/memory.c:4169:17: error: implicit declaration of function ‘count_mthp_stat’ [-Werror=implicit-function-declaration] > 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > | ^~~~~~~~~~~~~~~ > .../mm/memory.c:4169:53: error: ‘MTHP_STAT_ANON_SWPIN_REFAULT’ undeclared (first use in this function) > 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > .../mm/memory.c:4169:53: note: each undeclared identifier is reported only once for each function it appears in > cc1: some warnings being treated as errors > > My kunit build config doesn't have CONFIG_TRANSPARE_HUGEPAGE. Maybe that's the > reason and this patch, or the patch that introduced the function and the enum > need to take care of the case? Hi SeongJae, Thanks very much, can you check if the below fix the build? If yes, I will include this fix while sending v3. Subject: [PATCH] mm: fix build errors on CONFIG_TRANSPARENT_HUGEPAGE=N Signed-off-by: Barry Song <v-songbaohua@oppo.com> --- mm/memory.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/memory.c b/mm/memory.c index acc023795a4d..1d587d1eb432 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4142,6 +4142,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl); +#ifdef CONFIG_TRANSPARENT_HUGEPAGE /* We hit large folios in swapcache */ if (start_pte && folio_test_large(folio) && folio_test_swapcache(folio)) { int nr = folio_nr_pages(folio); @@ -4171,6 +4172,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } check_pte: +#endif if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf->orig_pte))) goto out_nomap;
On 09/04/2024 09:26, Barry Song wrote: > From: Barry Song <v-songbaohua@oppo.com> > > Currently, we are handling the scenario where we've hit a > large folio in the swapcache, and the reclaiming process > for this large folio is still ongoing. > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > --- > include/linux/huge_mm.h | 1 + > mm/huge_memory.c | 2 ++ > mm/memory.c | 1 + > 3 files changed, 4 insertions(+) > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index c8256af83e33..b67294d5814f 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -269,6 +269,7 @@ enum mthp_stat_item { > MTHP_STAT_ANON_ALLOC_FALLBACK, > MTHP_STAT_ANON_SWPOUT, > MTHP_STAT_ANON_SWPOUT_FALLBACK, > + MTHP_STAT_ANON_SWPIN_REFAULT, I don't see any equivalent counter for small folios. Is there an analogue? > __MTHP_STAT_COUNT > }; > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index d8d2ed80b0bf..fb95345b0bde 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > > static struct attribute *stats_attrs[] = { > &anon_alloc_attr.attr, > &anon_alloc_fallback_attr.attr, > &anon_swpout_attr.attr, > &anon_swpout_fallback_attr.attr, > + &anon_swpin_refault_attr.attr, > NULL, > }; > > diff --git a/mm/memory.c b/mm/memory.c > index 9818dc1893c8..acc023795a4d 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > nr_pages = nr; > entry = folio->swap; > page = &folio->page; > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); I don't think this is the point of no return yet? There's the pte_same() check immediately below (although I've suggested that needs to be moved to earlier), but also the folio_test_uptodate() check. Perhaps this should go after that? > } > > check_pte:
Hi Barry, On Thu, 11 Apr 2024 13:46:36 +1200 Barry Song <21cnbao@gmail.com> wrote: > >> + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > >> } > >> > > From the latest mm-unstable tree, I get below kunit build failure and > > 'git bisect' points this patch. > > > > $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/ > > [16:07:40] Configuring KUnit Kernel ... > > [16:07:40] Building KUnit Kernel ... > > Populating config with: > > $ make ARCH=um O=../kunit.out/ olddefconfig > > Building with: > > $ make ARCH=um O=../kunit.out/ --jobs=36 > > ERROR:root:.../mm/memory.c: In function ‘do_swap_page’: > > .../mm/memory.c:4169:17: error: implicit declaration of function ‘count_mthp_stat’ [-Werror=implicit-function-declaration] > > 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > > | ^~~~~~~~~~~~~~~ > > .../mm/memory.c:4169:53: error: ‘MTHP_STAT_ANON_SWPIN_REFAULT’ undeclared (first use in this function) > > 4169 | count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > .../mm/memory.c:4169:53: note: each undeclared identifier is reported only once for each function it appears in > > cc1: some warnings being treated as errors > > > > My kunit build config doesn't have CONFIG_TRANSPARE_HUGEPAGE. Maybe that's the > > reason and this patch, or the patch that introduced the function and the enum > > need to take care of the case? > > Hi SeongJae, > Thanks very much, can you check if the below fix the build? If yes, I will > include this fix while sending v3. Thank you for quick and kind reply :) I confirmed this fixes the build failure. > > Subject: [PATCH] mm: fix build errors on CONFIG_TRANSPARENT_HUGEPAGE=N > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> Tested-by: SeongJae Park <sj@kernel.org> Thanks, SJ > --- > mm/memory.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/memory.c b/mm/memory.c > index acc023795a4d..1d587d1eb432 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4142,6 +4142,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address, > &vmf->ptl); > > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > /* We hit large folios in swapcache */ > if (start_pte && folio_test_large(folio) && folio_test_swapcache(folio)) { > int nr = folio_nr_pages(folio); > @@ -4171,6 +4172,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > } > > check_pte: > +#endif > if (unlikely(!vmf->pte || !pte_same(ptep_get(vmf->pte), vmf->orig_pte))) > goto out_nomap; > > -- > 2.34.1 > >
On Fri, Apr 12, 2024 at 3:53 AM Ryan Roberts <ryan.roberts@arm.com> wrote: > > On 09/04/2024 09:26, Barry Song wrote: > > From: Barry Song <v-songbaohua@oppo.com> > > > > Currently, we are handling the scenario where we've hit a > > large folio in the swapcache, and the reclaiming process > > for this large folio is still ongoing. > > > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > > --- > > include/linux/huge_mm.h | 1 + > > mm/huge_memory.c | 2 ++ > > mm/memory.c | 1 + > > 3 files changed, 4 insertions(+) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index c8256af83e33..b67294d5814f 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -269,6 +269,7 @@ enum mthp_stat_item { > > MTHP_STAT_ANON_ALLOC_FALLBACK, > > MTHP_STAT_ANON_SWPOUT, > > MTHP_STAT_ANON_SWPOUT_FALLBACK, > > + MTHP_STAT_ANON_SWPIN_REFAULT, > > I don't see any equivalent counter for small folios. Is there an analogue? Indeed, we don't count refaults for small folios, as their refault mechanism is much simpler compared to large folios. Implementing this counter can enhance the system's visibility to users. Personally, having this counter and observing a non-zero value greatly enhances my confidence when debugging this refault series. Otherwise, it feels like being blind to what's happening inside the system :-) > > > __MTHP_STAT_COUNT > > }; > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index d8d2ed80b0bf..fb95345b0bde 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > > > > static struct attribute *stats_attrs[] = { > > &anon_alloc_attr.attr, > > &anon_alloc_fallback_attr.attr, > > &anon_swpout_attr.attr, > > &anon_swpout_fallback_attr.attr, > > + &anon_swpin_refault_attr.attr, > > NULL, > > }; > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 9818dc1893c8..acc023795a4d 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > > nr_pages = nr; > > entry = folio->swap; > > page = &folio->page; > > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > > I don't think this is the point of no return yet? There's the pte_same() check > immediately below (although I've suggested that needs to be moved to earlier), > but also the folio_test_uptodate() check. Perhaps this should go after that? > swap_pte_batch() == nr_pages should have passed the test for pte_same. folio_test_uptodate(folio)) should be also unlikely to be true as we are not reading from swap devices for refault case. but i agree we can move all the refault handling after those two "goto out_nomap". > > } > > > > check_pte: > Thanks Barry
Barry Song <21cnbao@gmail.com> writes: > From: Barry Song <v-songbaohua@oppo.com> > > Currently, we are handling the scenario where we've hit a > large folio in the swapcache, and the reclaiming process > for this large folio is still ongoing. > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > --- > include/linux/huge_mm.h | 1 + > mm/huge_memory.c | 2 ++ > mm/memory.c | 1 + > 3 files changed, 4 insertions(+) > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index c8256af83e33..b67294d5814f 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -269,6 +269,7 @@ enum mthp_stat_item { > MTHP_STAT_ANON_ALLOC_FALLBACK, > MTHP_STAT_ANON_SWPOUT, > MTHP_STAT_ANON_SWPOUT_FALLBACK, > + MTHP_STAT_ANON_SWPIN_REFAULT, This is different from the refault concept used in other place in mm subystem. Please check the following code if (shadow) workingset_refault(folio, shadow); in __read_swap_cache_async(). > __MTHP_STAT_COUNT > }; -- Best Regards, Huang, Ying > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index d8d2ed80b0bf..fb95345b0bde 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > > static struct attribute *stats_attrs[] = { > &anon_alloc_attr.attr, > &anon_alloc_fallback_attr.attr, > &anon_swpout_attr.attr, > &anon_swpout_fallback_attr.attr, > + &anon_swpin_refault_attr.attr, > NULL, > }; > > diff --git a/mm/memory.c b/mm/memory.c > index 9818dc1893c8..acc023795a4d 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > nr_pages = nr; > entry = folio->swap; > page = &folio->page; > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > } > > check_pte:
On Wed, Apr 17, 2024 at 8:47 AM Huang, Ying <ying.huang@intel.com> wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > From: Barry Song <v-songbaohua@oppo.com> > > > > Currently, we are handling the scenario where we've hit a > > large folio in the swapcache, and the reclaiming process > > for this large folio is still ongoing. > > > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > > --- > > include/linux/huge_mm.h | 1 + > > mm/huge_memory.c | 2 ++ > > mm/memory.c | 1 + > > 3 files changed, 4 insertions(+) > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index c8256af83e33..b67294d5814f 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -269,6 +269,7 @@ enum mthp_stat_item { > > MTHP_STAT_ANON_ALLOC_FALLBACK, > > MTHP_STAT_ANON_SWPOUT, > > MTHP_STAT_ANON_SWPOUT_FALLBACK, > > + MTHP_STAT_ANON_SWPIN_REFAULT, > > This is different from the refault concept used in other place in mm > subystem. Please check the following code > > if (shadow) > workingset_refault(folio, shadow); > > in __read_swap_cache_async(). right. it is slightly different as refault can also cover the case folios have been entirely released and a new page fault happens soon after it. Do you have a better name for this? MTHP_STAT_ANON_SWPIN_UNDER_RECLAIM or MTHP_STAT_ANON_SWPIN_RECLAIMING ? > > > __MTHP_STAT_COUNT > > }; > > -- > Best Regards, > Huang, Ying > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index d8d2ed80b0bf..fb95345b0bde 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > > > > static struct attribute *stats_attrs[] = { > > &anon_alloc_attr.attr, > > &anon_alloc_fallback_attr.attr, > > &anon_swpout_attr.attr, > > &anon_swpout_fallback_attr.attr, > > + &anon_swpin_refault_attr.attr, > > NULL, > > }; > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 9818dc1893c8..acc023795a4d 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > > nr_pages = nr; > > entry = folio->swap; > > page = &folio->page; > > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > > } > > > > check_pte:
Barry Song <21cnbao@gmail.com> writes: > On Wed, Apr 17, 2024 at 8:47 AM Huang, Ying <ying.huang@intel.com> wrote: >> >> Barry Song <21cnbao@gmail.com> writes: >> >> > From: Barry Song <v-songbaohua@oppo.com> >> > >> > Currently, we are handling the scenario where we've hit a >> > large folio in the swapcache, and the reclaiming process >> > for this large folio is still ongoing. >> > >> > Signed-off-by: Barry Song <v-songbaohua@oppo.com> >> > --- >> > include/linux/huge_mm.h | 1 + >> > mm/huge_memory.c | 2 ++ >> > mm/memory.c | 1 + >> > 3 files changed, 4 insertions(+) >> > >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >> > index c8256af83e33..b67294d5814f 100644 >> > --- a/include/linux/huge_mm.h >> > +++ b/include/linux/huge_mm.h >> > @@ -269,6 +269,7 @@ enum mthp_stat_item { >> > MTHP_STAT_ANON_ALLOC_FALLBACK, >> > MTHP_STAT_ANON_SWPOUT, >> > MTHP_STAT_ANON_SWPOUT_FALLBACK, >> > + MTHP_STAT_ANON_SWPIN_REFAULT, >> >> This is different from the refault concept used in other place in mm >> subystem. Please check the following code >> >> if (shadow) >> workingset_refault(folio, shadow); >> >> in __read_swap_cache_async(). > > right. it is slightly different as refault can also cover the case folios > have been entirely released and a new page fault happens soon > after it. > Do you have a better name for this? > MTHP_STAT_ANON_SWPIN_UNDER_RECLAIM > or > MTHP_STAT_ANON_SWPIN_RECLAIMING ? TBH, I don't think we need this counter. It's important for you during implementation. But I don't think it's important for end users. -- Best Regards, Huang, Ying >> >> > __MTHP_STAT_COUNT >> > }; >> >> -- >> Best Regards, >> Huang, Ying >> >> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> > index d8d2ed80b0bf..fb95345b0bde 100644 >> > --- a/mm/huge_memory.c >> > +++ b/mm/huge_memory.c >> > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); >> > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); >> > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); >> > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); >> > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); >> > >> > static struct attribute *stats_attrs[] = { >> > &anon_alloc_attr.attr, >> > &anon_alloc_fallback_attr.attr, >> > &anon_swpout_attr.attr, >> > &anon_swpout_fallback_attr.attr, >> > + &anon_swpin_refault_attr.attr, >> > NULL, >> > }; >> > >> > diff --git a/mm/memory.c b/mm/memory.c >> > index 9818dc1893c8..acc023795a4d 100644 >> > --- a/mm/memory.c >> > +++ b/mm/memory.c >> > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > nr_pages = nr; >> > entry = folio->swap; >> > page = &folio->page; >> > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); >> > } >> > >> > check_pte:
On Wed, Apr 17, 2024 at 1:40 PM Huang, Ying <ying.huang@intel.com> wrote: > > Barry Song <21cnbao@gmail.com> writes: > > > On Wed, Apr 17, 2024 at 8:47 AM Huang, Ying <ying.huang@intel.com> wrote: > >> > >> Barry Song <21cnbao@gmail.com> writes: > >> > >> > From: Barry Song <v-songbaohua@oppo.com> > >> > > >> > Currently, we are handling the scenario where we've hit a > >> > large folio in the swapcache, and the reclaiming process > >> > for this large folio is still ongoing. > >> > > >> > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > >> > --- > >> > include/linux/huge_mm.h | 1 + > >> > mm/huge_memory.c | 2 ++ > >> > mm/memory.c | 1 + > >> > 3 files changed, 4 insertions(+) > >> > > >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > >> > index c8256af83e33..b67294d5814f 100644 > >> > --- a/include/linux/huge_mm.h > >> > +++ b/include/linux/huge_mm.h > >> > @@ -269,6 +269,7 @@ enum mthp_stat_item { > >> > MTHP_STAT_ANON_ALLOC_FALLBACK, > >> > MTHP_STAT_ANON_SWPOUT, > >> > MTHP_STAT_ANON_SWPOUT_FALLBACK, > >> > + MTHP_STAT_ANON_SWPIN_REFAULT, > >> > >> This is different from the refault concept used in other place in mm > >> subystem. Please check the following code > >> > >> if (shadow) > >> workingset_refault(folio, shadow); > >> > >> in __read_swap_cache_async(). > > > > right. it is slightly different as refault can also cover the case folios > > have been entirely released and a new page fault happens soon > > after it. > > Do you have a better name for this? > > MTHP_STAT_ANON_SWPIN_UNDER_RECLAIM > > or > > MTHP_STAT_ANON_SWPIN_RECLAIMING ? > > TBH, I don't think we need this counter. It's important for you during > implementation. But I don't think it's important for end users. Okay. If we can't find a shared interest between the implementer and user, I'm perfectly fine with keeping it local only for debugging purposes. > > -- > Best Regards, > Huang, Ying > > >> > >> > __MTHP_STAT_COUNT > >> > }; > >> > >> -- > >> Best Regards, > >> Huang, Ying > >> > >> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >> > index d8d2ed80b0bf..fb95345b0bde 100644 > >> > --- a/mm/huge_memory.c > >> > +++ b/mm/huge_memory.c > >> > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); > >> > DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); > >> > DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); > >> > DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); > >> > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); > >> > > >> > static struct attribute *stats_attrs[] = { > >> > &anon_alloc_attr.attr, > >> > &anon_alloc_fallback_attr.attr, > >> > &anon_swpout_attr.attr, > >> > &anon_swpout_fallback_attr.attr, > >> > + &anon_swpin_refault_attr.attr, > >> > NULL, > >> > }; > >> > > >> > diff --git a/mm/memory.c b/mm/memory.c > >> > index 9818dc1893c8..acc023795a4d 100644 > >> > --- a/mm/memory.c > >> > +++ b/mm/memory.c > >> > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > >> > nr_pages = nr; > >> > entry = folio->swap; > >> > page = &folio->page; > >> > + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); > >> > } > >> > > >> > check_pte:
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index c8256af83e33..b67294d5814f 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -269,6 +269,7 @@ enum mthp_stat_item { MTHP_STAT_ANON_ALLOC_FALLBACK, MTHP_STAT_ANON_SWPOUT, MTHP_STAT_ANON_SWPOUT_FALLBACK, + MTHP_STAT_ANON_SWPIN_REFAULT, __MTHP_STAT_COUNT }; diff --git a/mm/huge_memory.c b/mm/huge_memory.c index d8d2ed80b0bf..fb95345b0bde 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_ANON_ALLOC); DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FALLBACK); DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT); DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_FALLBACK); +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFAULT); static struct attribute *stats_attrs[] = { &anon_alloc_attr.attr, &anon_alloc_fallback_attr.attr, &anon_swpout_attr.attr, &anon_swpout_fallback_attr.attr, + &anon_swpin_refault_attr.attr, NULL, }; diff --git a/mm/memory.c b/mm/memory.c index 9818dc1893c8..acc023795a4d 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) nr_pages = nr; entry = folio->swap; page = &folio->page; + count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWPIN_REFAULT); } check_pte: