[PATCH/RFC] btrfs: fix folio lock leak in writepage_delalloc() for folios dirtied behind btrfs' back

[Christian Borntraeger]

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@xxxxxxxxxxxxx>
---
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