* [PATCH v2] mm: Fix for negative counter: nr_file_hugepages @ 2023-11-07 18:18 Stefan Roesch 2023-11-07 19:35 ` Matthew Wilcox 0 siblings, 1 reply; 6+ messages in thread From: Stefan Roesch @ 2023-11-07 18:18 UTC (permalink / raw) To: kernel-team; +Cc: shr, akpm, hannes, riel, linux-kernel, linux-mm, stable While qualifiying the 6.4 release, the following warning was detected in messages: vmstat_refresh: nr_file_hugepages -15664 The warning is caused by the incorrect updating of the NR_FILE_THPS counter in the function split_huge_page_to_list. The if case is checking for folio_test_swapbacked, but the else case is missing the check for folio_test_pmd_mappable. The other functions that manipulate the counter like __filemap_add_folio and filemap_unaccount_folio have the corresponding check. I have a test case, which reproduces the problem. It can be found here: https://github.com/sroeschus/testcase/blob/main/vmstat_refresh/madv.c The test case reproduces on an XFS filesystem. Running the same test case on a BTRFS filesystem does not reproduce the problem. AFAIK version 6.1 until 6.6 are affected by this problem. Signed-off-by: Stefan Roesch <shr@devkernel.io> Co-debugged-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Johannes Weiner <hannes@cmpxchg.org> --- mm/huge_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 064fbd90822b4..9dbd5ef5a3902 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) if (folio_test_swapbacked(folio)) { __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, -nr); - } else { + } else if (folio_test_pmd_mappable(folio)) { __lruvec_stat_mod_folio(folio, NR_FILE_THPS, -nr); filemap_nr_thps_dec(mapping); base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa -- 2.39.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: Fix for negative counter: nr_file_hugepages 2023-11-07 18:18 [PATCH v2] mm: Fix for negative counter: nr_file_hugepages Stefan Roesch @ 2023-11-07 19:35 ` Matthew Wilcox 2023-11-07 20:06 ` Johannes Weiner 2023-11-08 17:09 ` Stefan Roesch 0 siblings, 2 replies; 6+ messages in thread From: Matthew Wilcox @ 2023-11-07 19:35 UTC (permalink / raw) To: Stefan Roesch Cc: kernel-team, akpm, hannes, riel, linux-kernel, linux-mm, stable On Tue, Nov 07, 2023 at 10:18:05AM -0800, Stefan Roesch wrote: > +++ b/mm/huge_memory.c > @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > if (folio_test_swapbacked(folio)) { > __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, > -nr); > - } else { > + } else if (folio_test_pmd_mappable(folio)) { > __lruvec_stat_mod_folio(folio, NR_FILE_THPS, > -nr); > filemap_nr_thps_dec(mapping); As I said, we also need the folio_test_pmd_mappable() for swapbacked. Not because there's currently a problem, but because we don't leave landmines for other people to trip over in future! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: Fix for negative counter: nr_file_hugepages 2023-11-07 19:35 ` Matthew Wilcox @ 2023-11-07 20:06 ` Johannes Weiner 2023-11-07 20:07 ` Matthew Wilcox 2023-11-08 17:09 ` Stefan Roesch 1 sibling, 1 reply; 6+ messages in thread From: Johannes Weiner @ 2023-11-07 20:06 UTC (permalink / raw) To: Matthew Wilcox Cc: Stefan Roesch, kernel-team, akpm, riel, linux-kernel, linux-mm, stable On Tue, Nov 07, 2023 at 07:35:37PM +0000, Matthew Wilcox wrote: > On Tue, Nov 07, 2023 at 10:18:05AM -0800, Stefan Roesch wrote: > > +++ b/mm/huge_memory.c > > @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > if (folio_test_swapbacked(folio)) { > > __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, > > -nr); > > - } else { > > + } else if (folio_test_pmd_mappable(folio)) { > > __lruvec_stat_mod_folio(folio, NR_FILE_THPS, > > -nr); > > filemap_nr_thps_dec(mapping); > > As I said, we also need the folio_test_pmd_mappable() for swapbacked. > Not because there's currently a problem, but because we don't leave > landmines for other people to trip over in future! Do we need to fix filemap_unaccount_folio() as well? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: Fix for negative counter: nr_file_hugepages 2023-11-07 20:06 ` Johannes Weiner @ 2023-11-07 20:07 ` Matthew Wilcox 2023-11-07 20:20 ` Johannes Weiner 0 siblings, 1 reply; 6+ messages in thread From: Matthew Wilcox @ 2023-11-07 20:07 UTC (permalink / raw) To: Johannes Weiner Cc: Stefan Roesch, kernel-team, akpm, riel, linux-kernel, linux-mm, stable On Tue, Nov 07, 2023 at 03:06:16PM -0500, Johannes Weiner wrote: > On Tue, Nov 07, 2023 at 07:35:37PM +0000, Matthew Wilcox wrote: > > On Tue, Nov 07, 2023 at 10:18:05AM -0800, Stefan Roesch wrote: > > > +++ b/mm/huge_memory.c > > > @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > > if (folio_test_swapbacked(folio)) { > > > __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, > > > -nr); > > > - } else { > > > + } else if (folio_test_pmd_mappable(folio)) { > > > __lruvec_stat_mod_folio(folio, NR_FILE_THPS, > > > -nr); > > > filemap_nr_thps_dec(mapping); > > > > As I said, we also need the folio_test_pmd_mappable() for swapbacked. > > Not because there's currently a problem, but because we don't leave > > landmines for other people to trip over in future! > > Do we need to fix filemap_unaccount_folio() as well? Looks to me like it is already correct? __lruvec_stat_mod_folio(folio, NR_FILE_PAGES, -nr); if (folio_test_swapbacked(folio)) { __lruvec_stat_mod_folio(folio, NR_SHMEM, -nr); if (folio_test_pmd_mappable(folio)) __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, -nr); } else if (folio_test_pmd_mappable(folio)) { __lruvec_stat_mod_folio(folio, NR_FILE_THPS, -nr); filemap_nr_thps_dec(mapping); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: Fix for negative counter: nr_file_hugepages 2023-11-07 20:07 ` Matthew Wilcox @ 2023-11-07 20:20 ` Johannes Weiner 0 siblings, 0 replies; 6+ messages in thread From: Johannes Weiner @ 2023-11-07 20:20 UTC (permalink / raw) To: Matthew Wilcox Cc: Stefan Roesch, kernel-team, akpm, riel, linux-kernel, linux-mm, stable On Tue, Nov 07, 2023 at 08:07:59PM +0000, Matthew Wilcox wrote: > On Tue, Nov 07, 2023 at 03:06:16PM -0500, Johannes Weiner wrote: > > On Tue, Nov 07, 2023 at 07:35:37PM +0000, Matthew Wilcox wrote: > > > On Tue, Nov 07, 2023 at 10:18:05AM -0800, Stefan Roesch wrote: > > > > +++ b/mm/huge_memory.c > > > > @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > > > if (folio_test_swapbacked(folio)) { > > > > __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, > > > > -nr); > > > > - } else { > > > > + } else if (folio_test_pmd_mappable(folio)) { > > > > __lruvec_stat_mod_folio(folio, NR_FILE_THPS, > > > > -nr); > > > > filemap_nr_thps_dec(mapping); > > > > > > As I said, we also need the folio_test_pmd_mappable() for swapbacked. > > > Not because there's currently a problem, but because we don't leave > > > landmines for other people to trip over in future! > > > > Do we need to fix filemap_unaccount_folio() as well? > > Looks to me like it is already correct? > > __lruvec_stat_mod_folio(folio, NR_FILE_PAGES, -nr); > if (folio_test_swapbacked(folio)) { > __lruvec_stat_mod_folio(folio, NR_SHMEM, -nr); > if (folio_test_pmd_mappable(folio)) > __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, -nr); > } else if (folio_test_pmd_mappable(folio)) { > __lruvec_stat_mod_folio(folio, NR_FILE_THPS, -nr); > filemap_nr_thps_dec(mapping); > } Argh, I overlooked it because it's nested further in due to that NR_SHMEM update. Sorry about the noise. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: Fix for negative counter: nr_file_hugepages 2023-11-07 19:35 ` Matthew Wilcox 2023-11-07 20:06 ` Johannes Weiner @ 2023-11-08 17:09 ` Stefan Roesch 1 sibling, 0 replies; 6+ messages in thread From: Stefan Roesch @ 2023-11-08 17:09 UTC (permalink / raw) To: Matthew Wilcox Cc: kernel-team, akpm, hannes, riel, linux-kernel, linux-mm, stable Matthew Wilcox <willy@infradead.org> writes: > On Tue, Nov 07, 2023 at 10:18:05AM -0800, Stefan Roesch wrote: >> +++ b/mm/huge_memory.c >> @@ -2740,7 +2740,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) >> if (folio_test_swapbacked(folio)) { >> __lruvec_stat_mod_folio(folio, NR_SHMEM_THPS, >> -nr); >> - } else { >> + } else if (folio_test_pmd_mappable(folio)) { >> __lruvec_stat_mod_folio(folio, NR_FILE_THPS, >> -nr); >> filemap_nr_thps_dec(mapping); > > As I said, we also need the folio_test_pmd_mappable() for swapbacked. > Not because there's currently a problem, but because we don't leave > landmines for other people to trip over in future! I'll add it in the next version. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-08 17:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-11-07 18:18 [PATCH v2] mm: Fix for negative counter: nr_file_hugepages Stefan Roesch 2023-11-07 19:35 ` Matthew Wilcox 2023-11-07 20:06 ` Johannes Weiner 2023-11-07 20:07 ` Matthew Wilcox 2023-11-07 20:20 ` Johannes Weiner 2023-11-08 17:09 ` Stefan Roesch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox