Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@linux.ibm.com>
To: linux-btrfs@vger.kernel.org, Qu Wenruo <wqu@suse.com>
Cc: borntraeger@linux.ibm.com, David Sterba <dsterba@suse.com>,
	Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	linux-s390@vger.kernel.org
Subject: [PATCH/RFC] btrfs: fix folio lock leak in writepage_delalloc() for folios dirtied behind btrfs' back
Date: Tue, 21 Jul 2026 21:11:51 +0200	[thread overview]
Message-ID: <20260721191152.101118-2-borntraeger@linux.ibm.com> (raw)
In-Reply-To: <20260721191152.101118-1-borntraeger@linux.ibm.com>

A folio can carry the folio-level dirty flag while its btrfs subpage
dirty bitmap is empty: btrfs data mappings use filemap_dirty_folio(),
so a generic folio_mark_dirty() call sets only the folio flag and the
xarray tag, without setting any subpage dirty bit and without a
delalloc reservation.  The typical source is set_page_dirty_lock() on
a GUP pin, e.g. the s390 KVM irq adapter path
(adapter_indicators_set()) which pins guest indicator pages living in
a file-backed guest RAM file, sets a bit and marks the page dirty.

When writeback then picks up such a folio, writepage_delalloc()
copies the empty subpage dirty bitmap into
bio_ctrl->submit_bitmap, sets up no range locks (nr_locked stays 0),
finds no delalloc range, and finally hits

	if (bitmap_empty(bio_ctrl->submit_bitmap, blocks_per_folio)) {
		wbc->nr_to_write -= delalloc_to_write;
		return 1;
	}

which is meant for "all dirty ranges were submitted asynchronously,
the async paths own the folio unlock".  But nothing was submitted at
all, so extent_writepage() returns without anybody ever unlocking the
folio.  The folio stays locked forever and every subsequent locker
(page faults through btrfs_page_mkwrite(), other flushers, delalloc
space reclaim which then parks holding fs_info->delalloc_root_mutex,
syncfs, ...) blocks in D state.

This was debugged from a crash dump of a hung s390 KVM host: a KVM
guest with its RAM backed by a file on btrfs (zstd compression),
where a 64-page (256K) large data folio of the guest RAM file was
found locked and dirty, with an empty subpage dirty bitmap,
nr_locked == 0, no PG_writeback set and no outstanding block I/O,
with two vCPU threads, the irqfd worker, two flusher workers,
khugepaged and syncfs all queued behind it.

Small folios are not affected because
btrfs_copy_subpage_dirty_bitmap() unconditionally reports bit 0 set
for single-block folios.  Affected are subpage setups (sectorsize <
PAGE_SIZE, e.g. 64K page size kernels with 4K sectorsize) since the
introduction of the submission bitmap in v6.12, and - much easier to
hit - 4K page size systems since btrfs gained large data folio
support, which makes every large folio take the subpage paths.

Fix it by detecting the empty-at-entry case right after the dirty
bitmap has been copied, before any range lock is set up: there is
nothing that can be submitted for such a folio, so clear the stale
folio-level dirty flag (nothing will ever be written back for it,
and all dirty flag setters serialize on the folio lock we hold, so
this cannot race with a new dirtier) and unlock the folio.  Since
folio_clear_dirty_for_io() intentionally leaves PAGECACHE_TAG_DIRTY
in the xarray, also run the same set/clear writeback dance that
extent_writepage_io() uses for the submitted-nothing case, so the
stale tag is dropped and the inode can go clean again.

The data written through the GUP pin is not lost; it sits in the
mapped page cache page.  It is simply not persisted until a proper
btrfs write path dirties the folio again - the same long-standing
semantics as any pin_user_pages() write to a file mapping that the
filesystem was not informed about.

Fixes: bd610c0937aa ("btrfs: only unlock the to-be-submitted ranges inside a folio")
Assisted-by: Claude 
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 7d604524e83c3..6a4a00ad43321 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1492,6 +1492,33 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode,
 	/* Save the dirty bitmap as our submission bitmap will be a subset of it. */
 	btrfs_copy_subpage_dirty_bitmap(fs_info, folio, bio_ctrl->submit_bitmap);
 
+	/*
+	 * The dirty bitmap can be empty even though the folio is dirty: data
+	 * mappings use filemap_dirty_folio(), so a generic folio_mark_dirty()
+	 * call (e.g. set_page_dirty_lock() after GUP) only sets the folio
+	 * flag, without any subpage dirty bit nor a delalloc reservation.
+	 *
+	 * There is nothing to submit for such a folio.  Bail out now,
+	 * otherwise the bitmap_empty() check at the end would mistake it for
+	 * "all ranges submitted asynchronously" and return with the folio
+	 * lock never released, deadlocking every subsequent locker.
+	 *
+	 * Also clear the stale dirty flag: with no subpage dirty bits nothing
+	 * will ever be written back for it, and leaving the flag would make
+	 * writeback rescan the folio forever.  All dirty flag setters hold
+	 * the folio lock, which we own, so this cannot race with a new
+	 * dirtier.  As folio_clear_dirty_for_io() keeps PAGECACHE_TAG_DIRTY,
+	 * use the same set/clear writeback dance as extent_writepage_io() to
+	 * also drop the stale tag, otherwise the inode would never go clean.
+	 */
+	if (unlikely(bitmap_empty(bio_ctrl->submit_bitmap, blocks_per_folio))) {
+		folio_clear_dirty_for_io(folio);
+		btrfs_folio_set_writeback(fs_info, folio, page_start, folio_size(folio));
+		btrfs_folio_clear_writeback(fs_info, folio, page_start, folio_size(folio));
+		folio_unlock(folio);
+		return 1;
+	}
+
 	for_each_set_bitrange(start_bit, end_bit, bio_ctrl->submit_bitmap,
 			      blocks_per_folio) {
 		u64 start = page_start + (start_bit << fs_info->sectorsize_bits);
-- 
2.51.0

  reply	other threads:[~2026-07-21 19:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 19:11 7.2-rc1 regression Folio lock leak in writepage_delalloc() Christian Borntraeger
2026-07-21 19:11 ` Christian Borntraeger [this message]
2026-07-21 19:26   ` [PATCH/RFC] btrfs: fix folio lock leak in writepage_delalloc() for folios dirtied behind btrfs' back sashiko-bot
2026-07-21 21:07   ` Qu Wenruo
2026-07-22  8:35     ` Christian Borntraeger
2026-07-22  8:59       ` Qu Wenruo
2026-07-22  9:29         ` Christian Borntraeger
2026-07-22  9:35           ` Qu Wenruo
2026-07-22 10:40             ` Christian Borntraeger
2026-07-22 12:57           ` Matthew Wilcox
2026-07-23  0:42             ` Qu Wenruo
     [not found]               ` <amICQYTK9Xu4KLtH@casper.infradead.org>
2026-07-23 22:40                 ` Qu Wenruo
2026-07-25  6:26                   ` Boris Burkov
2026-07-27  8:11                     ` Christian Borntraeger
2026-07-27  8:41                       ` Qu Wenruo
2026-07-27 12:59                         ` Christian Borntraeger
2026-07-22  7:21 ` 7.2-rc1 regression Folio lock leak in writepage_delalloc() Qu Wenruo

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=20260721191152.101118-2-borntraeger@linux.ibm.com \
    --to=borntraeger@linux.ibm.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=wqu@suse.com \
    /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