* [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