linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Boris Burkov <boris@bur.io>
To: linux-mm@kvack.org
Cc: willy@infradead.org, shakeel.butt@linux.dev
Subject: [PATCH RFC] mm: fix refcount check in mapping_evict_folio
Date: Tue, 13 Aug 2024 11:25:56 -0700	[thread overview]
Message-ID: <f1f6909c39ffac4c24ba7feed4a561a61cecd742.1723573450.git.boris@bur.io> (raw)

The commit e41c81d0d30e ("mm/truncate: Replace page_mapped() call in
invalidate_inode_page()") replaced the page_mapped(page) check with a
refcount check. However, this refcount check does not work as expected
with drop_caches, at least for btrfs's metadata pages.

Btrfs has a per-sb metadata inode with cached pages, and when not in
active use by btrfs, they have a refcount of 3. One from the initial
call to alloc_pages, one (nr_pages == 1) from filemap_add_folio, and one
from folio_attach_private. We would expect such pages to get dropped by
drop_caches. However, drop_caches calls into mapping_evict_folio via
mapping_try_invalidate which gets a reference on the folio with
find_lock_entries(). As a result, these pages have a refcount of 4, and
fail this check.

For what it's worth, such pages do get reclaimed under memory pressure,
and if I change this refcount check to `if folio_mapped(folio)`

The following script produces such pages and uses drgn to further
analyze the state of the folios:
https://github.com/boryas/scripts/blob/main/sh/strand-meta/run.sh
It should at least outline the basic idea for producing some btrfs
metadata via creating inlined-extent files.

My proposed fix for the issue is to add one more hardcoded refcount to
this check to account for the caller having a refcount on the page.
However, I am less familiar with the other caller into
mapping_evict_folio in the page fault path, so I am concerned this fix
will not work properly there, and would appreciate extra scrutiny there.

Fixes: e41c81d0d30e ("mm/truncate: Replace page_mapped() call in invalidate_inode_page()")
Signed-off-by: Boris Burkov <boris@bur.io>
---
 mm/truncate.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/mm/truncate.c b/mm/truncate.c
index 4d61fbdd4b2f..c710c84710b4 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -267,9 +267,17 @@ long mapping_evict_folio(struct address_space *mapping, struct folio *folio)
 		return 0;
 	if (folio_test_dirty(folio) || folio_test_writeback(folio))
 		return 0;
-	/* The refcount will be elevated if any page in the folio is mapped */
+	/*
+	 * The refcount will be elevated if any page in the folio is mapped.
+	 *
+	 * The refcounts break down as follows:
+	 * 1 per mapped page
+	 * 1 from folio_attach_private, if private is set
+	 * 1 from allocating the page in the first place
+	 * 1 from the caller
+	 */
 	if (folio_ref_count(folio) >
-			folio_nr_pages(folio) + folio_has_private(folio) + 1)
+			folio_nr_pages(folio) + folio_has_private(folio) + 1 + 1)
 		return 0;
 	if (!filemap_release_folio(folio, 0))
 		return 0;
-- 
2.46.0



             reply	other threads:[~2024-08-13 18:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-13 18:25 Boris Burkov [this message]
2024-08-13 19:58 ` Shakeel Butt
2024-08-14  3:15   ` Matthew Wilcox
2024-08-14  3:27     ` Boris Burkov
2024-08-14  3:46       ` Matthew Wilcox
2024-08-14  4:23         ` Boris Burkov
2024-08-20  8:00 ` David Hildenbrand
2024-08-20 14:00   ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f1f6909c39ffac4c24ba7feed4a561a61cecd742.1723573450.git.boris@bur.io \
    --to=boris@bur.io \
    --cc=linux-mm@kvack.org \
    --cc=shakeel.butt@linux.dev \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox