mm/filemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: Wenjie Qi <qiwenjie@xiaomi.com>
Fault-around can map ready dropbehind folios without going through the
normal page-cache lookup that clears dropbehind. A mapping represents a
competing cached user, but writeback completion can currently unmap that
folio. A later mmap access must then fault it back in.
Retain mapped folios instead. For a mapped folio,
folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
folio_end_dropbehind() runs in non-preemptible task context.
Unmapped dropbehind folios continue through the existing invalidation path.
Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
Cc: stable@vger.kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Tal Zussman <tz2294@columbia.edu>
---
The mapped-plus-dropbehind state was reproduced in QEMU. The patched
kernel retained the mapped folio and continued to evict the unmapped
dontcache folio. I did not reproduce the sleeping-in-atomic warning and
am not aware of an existing report.
Changes since v3:
- Describe the forced-unmap/refault runtime effect.
- Record the reproduction boundary and add Cc: stable@vger.kernel.org.
mm/filemap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 6afec636881f..00fd89cf6f55 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1616,7 +1616,7 @@ static void filemap_end_dropbehind(struct folio *folio)
return;
if (!folio_test_clear_dropbehind(folio))
return;
- if (mapping)
+ if (mapping && !folio_mapped(folio))
folio_unmap_invalidate(mapping, folio, 0);
}
--
2.43.0On 8/30/26 6:25 AM, Wenjie Qi wrote:
> From: Wenjie Qi <qiwenjie@ xiaomi. com> Fault-around can map ready dropbehind
> folios without going through the normal page-cache lookup that clears
> dropbehind. A mapping represents a competing cached user, but writeback
> completion can currently
> ZjQcmQRYFpfptBannerStart
> This Message Is From an External Sender
> This message came from outside your organization.
> ZjQcmQRYFpfptBannerEnd
>
> From: Wenjie Qi <qiwenjie@xiaomi.com>
>
> Fault-around can map ready dropbehind folios without going through the
> normal page-cache lookup that clears dropbehind. A mapping represents a
> competing cached user, but writeback completion can currently unmap that
> folio. A later mmap access must then fault it back in.
>
> Retain mapped folios instead. For a mapped folio,
> folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> folio_end_dropbehind() runs in non-preemptible task context.
>
> Unmapped dropbehind folios continue through the existing invalidation path.
>
> Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> ---
> The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> kernel retained the mapped folio and continued to evict the unmapped
> dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> am not aware of an existing report.
>
I was able to trigger the sleeping-while-atomic BUG on current mainline with
some help from Claude. The reproducer itself is straightforward enough, but the
configuration was a little finicky. The atomic context comes from using
threadirqs so that virtblk_done() completes requests from the irq thread under
local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
inline instead of on a workqueue). With v4 applied the bug is gone, so:
Tested-by: Tal Zussman <tz2294@columbia.edu>
Full splat:
[ 46.745106] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1573
[ 46.745183] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 61, name: irq/40-virtio1-
[ 46.745211] preempt_count: 201, expected: 0
[ 46.745233] RCU nest depth: 0, expected: 0
[ 46.745292] locks held by irq/40-virtio1-/61: 1, last CPU#0:
[ 46.745324] #0: ff1fce9901fc2f20 (&vblk->vqs[i].lock){....}-{3:3}, at: virtblk_done+0x56/0x100
[ 46.746801] irq event stamp: 1585
[ 46.748015] hardirqs last enabled at (1583): [<ffffffff88dc7c54>] finish_task_switch.isra.0+0xd4/0x360
[ 46.748224] hardirqs last disabled at (1585): [<ffffffff88e2b16c>] irq_forced_thread_fn+0x5c/0x60
[ 46.748405] softirqs last enabled at (1578): [<ffffffff88e2b159>] irq_forced_thread_fn+0x49/0x60
[ 46.748580] softirqs last disabled at (1584): [<ffffffff88e2b12d>] irq_forced_thread_fn+0x1d/0x60
[ 46.749233] CPU: 0 UID: 0 PID: 61 Comm: irq/40-virtio1- Not tainted 7.2.0+ #5 PREEMPT(full)
[ 46.749323] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 46.749472] Call Trace:
[ 46.749552] <TASK>
[ 46.749653] dump_stack_lvl+0x66/0xa0
[ 46.749747] __might_resched+0x154/0x260
[ 46.749787] down_read+0x1e/0x170
[ 46.749806] unmap_mapping_folio+0x74/0xd0
[ 46.749854] folio_unmap_invalidate+0x92/0x200
[ 46.749886] folio_end_writeback+0x51/0xb0
[ 46.749906] ext4_finish_bio+0x36d/0x3a0
[ 46.749979] ext4_end_bio+0x51/0x140
[ 46.750010] blk_update_request+0x101/0x4d0
[ 46.750045] ? detach_buf_split_in_order+0x18c/0x1b0
[ 46.750098] blk_mq_end_request+0x20/0x130
[ 46.750135] virtblk_done+0x75/0x100
[ 46.750173] ? irq_thread+0xb3/0x2a0
[ 46.750190] vring_interrupt+0x8c/0xd0
[ 46.750205] irq_thread_fn+0x23/0x60
[ 46.750238] irq_forced_thread_fn+0x35/0x60
[ 46.750261] irq_thread+0x17c/0x2a0
[ 46.750274] ? __pfx_irq_forced_thread_fn+0x10/0x10
[ 46.750299] ? __pfx_irq_thread_dtor+0x10/0x10
[ 46.750333] ? __pfx_irq_thread+0x10/0x10
[ 46.750353] kthread+0xf6/0x130
[ 46.750369] ? __pfx_kthread+0x10/0x10
[ 46.750404] ret_from_fork+0x248/0x350
[ 46.750421] ? __pfx_kthread+0x10/0x10
[ 46.750442] ret_from_fork_asm+0x1a/0x30
[ 46.750585] </TASK>
[ 46.752964] =============================
[ 46.753047] [ BUG: Invalid wait context ]
[ 46.753230] 7.2.0+ #5 Tainted: G W
[ 46.753353] -----------------------------
[ 46.753438] irq/40-virtio1-/61 is trying to lock:
[ 46.753533] ff1fce99004e4990 (&mapping->i_mmap_rwsem){++++}-{4:4}, at: unmap_mapping_folio+0x74/0xd0
[ 46.753882] other info that might help us debug this:
[ 46.754000] context-{5:5}
[ 46.754062] locks held by irq/40-virtio1-/61: 1, last CPU#0:
[ 46.754171] #0: ff1fce9901fc2f20 (&vblk->vqs[i].lock){....}-{3:3}, at: virtblk_done+0x56/0x100
[ 46.754371] stack backtrace:
[ 46.754441] CPU: 0 UID: 0 PID: 61 Comm: irq/40-virtio1- Tainted: G W 7.2.0+ #5 PREEMPT(full)
[ 46.754656] Tainted: [W]=WARN
[ 46.754725] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 46.754887] Call Trace:
[ 46.754942] <TASK>
[ 46.754996] dump_stack_lvl+0x66/0xa0
[ 46.755082] __lock_acquire+0x92e/0x1a80
[ 46.755197] lock_acquire+0xcd/0x2d0
[ 46.755282] ? unmap_mapping_folio+0x74/0xd0
[ 46.755403] down_read+0x42/0x170
[ 46.755476] ? unmap_mapping_folio+0x74/0xd0
[ 46.755565] unmap_mapping_folio+0x74/0xd0
[ 46.755664] folio_unmap_invalidate+0x92/0x200
[ 46.755763] folio_end_writeback+0x51/0xb0
[ 46.755851] ext4_finish_bio+0x36d/0x3a0
[ 46.755958] ext4_end_bio+0x51/0x140
[ 46.756043] blk_update_request+0x101/0x4d0
[ 46.756135] ? detach_buf_split_in_order+0x18c/0x1b0
[ 46.756254] blk_mq_end_request+0x20/0x130
[ 46.756349] virtblk_done+0x75/0x100
[ 46.756438] ? irq_thread+0xb3/0x2a0
[ 46.756515] vring_interrupt+0x8c/0xd0
[ 46.756595] irq_thread_fn+0x23/0x60
[ 46.756678] irq_forced_thread_fn+0x35/0x60
[ 46.756768] irq_thread+0x17c/0x2a0
[ 46.756842] ? __pfx_irq_forced_thread_fn+0x10/0x10
[ 46.756945] ? __pfx_irq_thread_dtor+0x10/0x10
[ 46.757045] ? __pfx_irq_thread+0x10/0x10
[ 46.757136] kthread+0xf6/0x130
[ 46.757207] ? __pfx_kthread+0x10/0x10
[ 46.757302] ret_from_fork+0x248/0x350
[ 46.757383] ? __pfx_kthread+0x10/0x10
[ 46.757466] ret_from_fork_asm+0x1a/0x30
[ 46.757594] </TASK>
> Changes since v3:
> - Describe the forced-unmap/refault runtime effect.
> - Record the reproduction boundary and add Cc: stable@vger.kernel.org.
>
> mm/filemap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 6afec636881f..00fd89cf6f55 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1616,7 +1616,7 @@ static void filemap_end_dropbehind(struct folio *folio)
> return;
> if (!folio_test_clear_dropbehind(folio))
> return;
> - if (mapping)
> + if (mapping && !folio_mapped(folio))
> folio_unmap_invalidate(mapping, folio, 0);
> }
>
> --
> 2.43.0
>
On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
>
> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> > From: Wenjie Qi <qiwenjie@ xiaomi. com> Fault-around can map ready dropbehind
> > folios without going through the normal page-cache lookup that clears
> > dropbehind. A mapping represents a competing cached user, but writeback
> > completion can currently
> > ZjQcmQRYFpfptBannerStart
> > This Message Is From an External Sender
> > This message came from outside your organization.
> > ZjQcmQRYFpfptBannerEnd
> >
> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> >
> > Fault-around can map ready dropbehind folios without going through the
> > normal page-cache lookup that clears dropbehind. A mapping represents a
> > competing cached user, but writeback completion can currently unmap that
> > folio. A later mmap access must then fault it back in.
> >
> > Retain mapped folios instead. For a mapped folio,
> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> > folio_end_dropbehind() runs in non-preemptible task context.
> >
> > Unmapped dropbehind folios continue through the existing invalidation path.
> >
> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> > ---
> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> > kernel retained the mapped folio and continued to evict the unmapped
> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> > am not aware of an existing report.
> >
>
> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> some help from Claude. The reproducer itself is straightforward enough, but the
> configuration was a little finicky. The atomic context comes from using
> threadirqs so that virtblk_done() completes requests from the irq thread under
> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> inline instead of on a workqueue). With v4 applied the bug is gone, so:
>
Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
patch, we are no longer going to sleep for dropbehind in `filemap.c`,
do we still need all the complex logic in the block device and file
system layers to move dropbehind to a workqueue task context?
> Tested-by: Tal Zussman <tz2294@columbia.edu>
>
Thanks
Barry
On 9/4/26 12:28 AM, Barry Song wrote:
> On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
>>
>> On 8/30/26 6:25 AM, Wenjie Qi wrote:
>> > From: Wenjie Qi <qiwenjie@xiaomi.com>
>> >
>> > Fault-around can map ready dropbehind folios without going through the
>> > normal page-cache lookup that clears dropbehind. A mapping represents a
>> > competing cached user, but writeback completion can currently unmap that
>> > folio. A later mmap access must then fault it back in.
>> >
>> > Retain mapped folios instead. For a mapped folio,
>> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
>> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
>> > folio_end_dropbehind() runs in non-preemptible task context.
>> >
>> > Unmapped dropbehind folios continue through the existing invalidation path.
>> >
>> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
>> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
>> > ---
>> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
>> > kernel retained the mapped folio and continued to evict the unmapped
>> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
>> > am not aware of an existing report.
>> >
>>
>> I was able to trigger the sleeping-while-atomic BUG on current mainline with
>> some help from Claude. The reproducer itself is straightforward enough, but the
>> configuration was a little finicky. The atomic context comes from using
>> threadirqs so that virtblk_done() completes requests from the irq thread under
>> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
>> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
>> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
>> inline instead of on a workqueue). With v4 applied the bug is gone, so:
>>
>
> Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> do we still need all the complex logic in the block device and file
> system layers to move dropbehind to a workqueue task context?
>
Hi Barry,
Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
The existing in_task() check covered most other cases, in that it just
doesn't do dropbehind invalidation if we're not executing in task context
(for example, block device writeback completing in interrupt context).
However, dropping the invalidation defeats the point of dropbehind, as it
leaves the pages in the page cache. If we get rid of the task-context
deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
Wenjie's change is largely unrelated to this.
Thanks,
Tal
On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
>
> On 9/4/26 12:28 AM, Barry Song wrote:
> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
> >>
> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> >> >
> >> > Fault-around can map ready dropbehind folios without going through the
> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
> >> > competing cached user, but writeback completion can currently unmap that
> >> > folio. A later mmap access must then fault it back in.
> >> >
> >> > Retain mapped folios instead. For a mapped folio,
> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> >> > folio_end_dropbehind() runs in non-preemptible task context.
> >> >
> >> > Unmapped dropbehind folios continue through the existing invalidation path.
> >> >
> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> >> > Cc: stable@vger.kernel.org
> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> >> > ---
> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> >> > kernel retained the mapped folio and continued to evict the unmapped
> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> >> > am not aware of an existing report.
> >> >
> >>
> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> >> some help from Claude. The reproducer itself is straightforward enough, but the
> >> configuration was a little finicky. The atomic context comes from using
> >> threadirqs so that virtblk_done() completes requests from the irq thread under
> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
> >>
> >
> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> > do we still need all the complex logic in the block device and file
> > system layers to move dropbehind to a workqueue task context?
> >
>
> Hi Barry,
>
> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
> The existing in_task() check covered most other cases, in that it just
> doesn't do dropbehind invalidation if we're not executing in task context
> (for example, block device writeback completing in interrupt context).
>
> However, dropping the invalidation defeats the point of dropbehind, as it
> leaves the pages in the page cache. If we get rid of the task-context
> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
> Wenjie's change is largely unrelated to this.
Hi Tal,
Thanks for the clarification. However, I’m still not convinced.
If `unmap_mapping_folio()` is the only place where we might
sleep — where we might take `mapping->i_mmap_rwsem` through
`i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
prevented that from happening with `if (!folio_mapped(folio))`.
So no mapped folio should call `folio_unmap_invalidate()`:
static void filemap_end_dropbehind(struct folio *folio)
{
struct address_space *mapping = folio->mapping;
VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
if (folio_test_writeback(folio) || folio_test_dirty(folio))
return;
if (!folio_test_clear_dropbehind(folio))
return;
if (mapping && !folio_mapped(folio))
folio_unmap_invalidate(mapping, folio, 0);
}
Why do we still need the `in_task()` check in
`folio_end_dropbehind()`?
void folio_end_dropbehind(struct folio *folio)
{
if (!folio_test_dropbehind(folio))
return;
/*
* Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
* but can happen if normal writeback just happens to find dirty folios
* that were created as part of uncached writeback, and that writeback
* would otherwise not need non-IRQ handling. Just skip the
* invalidation in that case.
*/
if (in_task() && folio_trylock(folio)) {
filemap_end_dropbehind(folio);
folio_unlock(folio);
}
}
Could any other operation in `folio_unmap_invalidate()` sleep for
unmapped folios? Otherwise, it seems we don't need `in_task()` at
all?
[1] https://sashiko.dev/#/message/CAGFpFsRarmDScAQxHpCtSZxK5H4WCg4Y%3Dy0hXQLjGT804ZoWkw%40mail.gmail.com
Best Regards
Barry
On 9/13/26 6:51 PM, Barry Song wrote:
> On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
>>
>> On 9/4/26 12:28 AM, Barry Song wrote:
>> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
>> >>
>> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
>> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
>> >> >
>> >> > Fault-around can map ready dropbehind folios without going through the
>> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
>> >> > competing cached user, but writeback completion can currently unmap that
>> >> > folio. A later mmap access must then fault it back in.
>> >> >
>> >> > Retain mapped folios instead. For a mapped folio,
>> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
>> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
>> >> > folio_end_dropbehind() runs in non-preemptible task context.
>> >> >
>> >> > Unmapped dropbehind folios continue through the existing invalidation path.
>> >> >
>> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
>> >> > Cc: stable@vger.kernel.org
>> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
>> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
>> >> > ---
>> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
>> >> > kernel retained the mapped folio and continued to evict the unmapped
>> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
>> >> > am not aware of an existing report.
>> >> >
>> >>
>> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
>> >> some help from Claude. The reproducer itself is straightforward enough, but the
>> >> configuration was a little finicky. The atomic context comes from using
>> >> threadirqs so that virtblk_done() completes requests from the irq thread under
>> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
>> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
>> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
>> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
>> >>
>> >
>> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
>> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
>> > do we still need all the complex logic in the block device and file
>> > system layers to move dropbehind to a workqueue task context?
>> >
>>
>> Hi Barry,
>>
>> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
>> The existing in_task() check covered most other cases, in that it just
>> doesn't do dropbehind invalidation if we're not executing in task context
>> (for example, block device writeback completing in interrupt context).
>>
>> However, dropping the invalidation defeats the point of dropbehind, as it
>> leaves the pages in the page cache. If we get rid of the task-context
>> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
>> Wenjie's change is largely unrelated to this.
>
> Hi Tal,
>
> Thanks for the clarification. However, I’m still not convinced.
>
> If `unmap_mapping_folio()` is the only place where we might
> sleep — where we might take `mapping->i_mmap_rwsem` through
> `i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
> prevented that from happening with `if (!folio_mapped(folio))`.
>
> So no mapped folio should call `folio_unmap_invalidate()`:
>
> static void filemap_end_dropbehind(struct folio *folio)
> {
> struct address_space *mapping = folio->mapping;
>
> VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>
> if (folio_test_writeback(folio) || folio_test_dirty(folio))
> return;
> if (!folio_test_clear_dropbehind(folio))
> return;
> if (mapping && !folio_mapped(folio))
> folio_unmap_invalidate(mapping, folio, 0);
> }
>
> Why do we still need the `in_task()` check in
> `folio_end_dropbehind()`?
>
> void folio_end_dropbehind(struct folio *folio)
> {
> if (!folio_test_dropbehind(folio))
> return;
>
> /*
> * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> * but can happen if normal writeback just happens to find dirty folios
> * that were created as part of uncached writeback, and that writeback
> * would otherwise not need non-IRQ handling. Just skip the
> * invalidation in that case.
> */
> if (in_task() && folio_trylock(folio)) {
> filemap_end_dropbehind(folio);
> folio_unlock(folio);
> }
> }
>
> Could any other operation in `folio_unmap_invalidate()` sleep for
> unmapped folios? Otherwise, it seems we don't need `in_task()` at
> all?
>
So I think you're right about the sleeping aspect, but the problem is
that this still gets called from interrupt context. folio_unmap_invalidate()
grabs a number of locks in a non-IRQ safe way. Just at a glance, the
spin_lock(&mapping->host->i_lock) call could lead to a deadlock, since other
paths don't call it with interrupts disabled. And xa_unlock_irq() enables
interrupts unconditionally, which would be incorrect from hardirq I believe.
- Tal
On Mon, Sep 14, 2026 at 7:34 AM Tal Zussman <tz2294@columbia.edu> wrote:
>
> On 9/13/26 6:51 PM, Barry Song wrote:
> > On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
> >>
> >> On 9/4/26 12:28 AM, Barry Song wrote:
> >> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
> >> >>
> >> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> >> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> >> >> >
> >> >> > Fault-around can map ready dropbehind folios without going through the
> >> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
> >> >> > competing cached user, but writeback completion can currently unmap that
> >> >> > folio. A later mmap access must then fault it back in.
> >> >> >
> >> >> > Retain mapped folios instead. For a mapped folio,
> >> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> >> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> >> >> > folio_end_dropbehind() runs in non-preemptible task context.
> >> >> >
> >> >> > Unmapped dropbehind folios continue through the existing invalidation path.
> >> >> >
> >> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> >> >> > Cc: stable@vger.kernel.org
> >> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> >> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> >> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> >> >> > ---
> >> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> >> >> > kernel retained the mapped folio and continued to evict the unmapped
> >> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> >> >> > am not aware of an existing report.
> >> >> >
> >> >>
> >> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> >> >> some help from Claude. The reproducer itself is straightforward enough, but the
> >> >> configuration was a little finicky. The atomic context comes from using
> >> >> threadirqs so that virtblk_done() completes requests from the irq thread under
> >> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> >> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> >> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> >> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
> >> >>
> >> >
> >> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> >> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> >> > do we still need all the complex logic in the block device and file
> >> > system layers to move dropbehind to a workqueue task context?
> >> >
> >>
> >> Hi Barry,
> >>
> >> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
> >> The existing in_task() check covered most other cases, in that it just
> >> doesn't do dropbehind invalidation if we're not executing in task context
> >> (for example, block device writeback completing in interrupt context).
> >>
> >> However, dropping the invalidation defeats the point of dropbehind, as it
> >> leaves the pages in the page cache. If we get rid of the task-context
> >> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
> >> Wenjie's change is largely unrelated to this.
> >
> > Hi Tal,
> >
> > Thanks for the clarification. However, I’m still not convinced.
> >
> > If `unmap_mapping_folio()` is the only place where we might
> > sleep — where we might take `mapping->i_mmap_rwsem` through
> > `i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
> > prevented that from happening with `if (!folio_mapped(folio))`.
> >
> > So no mapped folio should call `folio_unmap_invalidate()`:
> >
> > static void filemap_end_dropbehind(struct folio *folio)
> > {
> > struct address_space *mapping = folio->mapping;
> >
> > VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> >
> > if (folio_test_writeback(folio) || folio_test_dirty(folio))
> > return;
> > if (!folio_test_clear_dropbehind(folio))
> > return;
> > if (mapping && !folio_mapped(folio))
> > folio_unmap_invalidate(mapping, folio, 0);
> > }
> >
> > Why do we still need the `in_task()` check in
> > `folio_end_dropbehind()`?
> >
> > void folio_end_dropbehind(struct folio *folio)
> > {
> > if (!folio_test_dropbehind(folio))
> > return;
> >
> > /*
> > * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> > * but can happen if normal writeback just happens to find dirty folios
> > * that were created as part of uncached writeback, and that writeback
> > * would otherwise not need non-IRQ handling. Just skip the
> > * invalidation in that case.
> > */
> > if (in_task() && folio_trylock(folio)) {
> > filemap_end_dropbehind(folio);
> > folio_unlock(folio);
> > }
> > }
> >
> > Could any other operation in `folio_unmap_invalidate()` sleep for
> > unmapped folios? Otherwise, it seems we don't need `in_task()` at
> > all?
> >
>
> So I think you're right about the sleeping aspect, but the problem is
> that this still gets called from interrupt context. folio_unmap_invalidate()
> grabs a number of locks in a non-IRQ safe way. Just at a glance, the
> spin_lock(&mapping->host->i_lock) call could lead to a deadlock, since other
> paths don't call it with interrupts disabled. And xa_unlock_irq() enables
> interrupts unconditionally, which would be incorrect from hardirq I believe.
That makes sense to me. If that's the case, it seems the
comment above
`if (in_task() && folio_trylock(folio))`
is no longer relevant at all.
BTW, if we use `spin_lock_irqsave(&mapping->host->i_lock)` in
other places and also restore IRQs when unlocking the xarray, then
we shouldn't need the `in_task()` check anymore, should we?
Best Regards
Barry
On Mon 14-09-26 09:09:32, Barry Song wrote:
> On Mon, Sep 14, 2026 at 7:34 AM Tal Zussman <tz2294@columbia.edu> wrote:
> >
> > On 9/13/26 6:51 PM, Barry Song wrote:
> > > On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
> > >>
> > >> On 9/4/26 12:28 AM, Barry Song wrote:
> > >> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
> > >> >>
> > >> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> > >> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> > >> >> >
> > >> >> > Fault-around can map ready dropbehind folios without going through the
> > >> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
> > >> >> > competing cached user, but writeback completion can currently unmap that
> > >> >> > folio. A later mmap access must then fault it back in.
> > >> >> >
> > >> >> > Retain mapped folios instead. For a mapped folio,
> > >> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> > >> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> > >> >> > folio_end_dropbehind() runs in non-preemptible task context.
> > >> >> >
> > >> >> > Unmapped dropbehind folios continue through the existing invalidation path.
> > >> >> >
> > >> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> > >> >> > Cc: stable@vger.kernel.org
> > >> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > >> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > >> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> > >> >> > ---
> > >> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> > >> >> > kernel retained the mapped folio and continued to evict the unmapped
> > >> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> > >> >> > am not aware of an existing report.
> > >> >> >
> > >> >>
> > >> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> > >> >> some help from Claude. The reproducer itself is straightforward enough, but the
> > >> >> configuration was a little finicky. The atomic context comes from using
> > >> >> threadirqs so that virtblk_done() completes requests from the irq thread under
> > >> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> > >> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> > >> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> > >> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
> > >> >>
> > >> >
> > >> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> > >> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> > >> > do we still need all the complex logic in the block device and file
> > >> > system layers to move dropbehind to a workqueue task context?
> > >> >
> > >>
> > >> Hi Barry,
> > >>
> > >> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
> > >> The existing in_task() check covered most other cases, in that it just
> > >> doesn't do dropbehind invalidation if we're not executing in task context
> > >> (for example, block device writeback completing in interrupt context).
> > >>
> > >> However, dropping the invalidation defeats the point of dropbehind, as it
> > >> leaves the pages in the page cache. If we get rid of the task-context
> > >> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
> > >> Wenjie's change is largely unrelated to this.
> > >
> > > Hi Tal,
> > >
> > > Thanks for the clarification. However, I’m still not convinced.
> > >
> > > If `unmap_mapping_folio()` is the only place where we might
> > > sleep — where we might take `mapping->i_mmap_rwsem` through
> > > `i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
> > > prevented that from happening with `if (!folio_mapped(folio))`.
> > >
> > > So no mapped folio should call `folio_unmap_invalidate()`:
> > >
> > > static void filemap_end_dropbehind(struct folio *folio)
> > > {
> > > struct address_space *mapping = folio->mapping;
> > >
> > > VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> > >
> > > if (folio_test_writeback(folio) || folio_test_dirty(folio))
> > > return;
> > > if (!folio_test_clear_dropbehind(folio))
> > > return;
> > > if (mapping && !folio_mapped(folio))
> > > folio_unmap_invalidate(mapping, folio, 0);
> > > }
> > >
> > > Why do we still need the `in_task()` check in
> > > `folio_end_dropbehind()`?
> > >
> > > void folio_end_dropbehind(struct folio *folio)
> > > {
> > > if (!folio_test_dropbehind(folio))
> > > return;
> > >
> > > /*
> > > * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> > > * but can happen if normal writeback just happens to find dirty folios
> > > * that were created as part of uncached writeback, and that writeback
> > > * would otherwise not need non-IRQ handling. Just skip the
> > > * invalidation in that case.
> > > */
> > > if (in_task() && folio_trylock(folio)) {
> > > filemap_end_dropbehind(folio);
> > > folio_unlock(folio);
> > > }
> > > }
> > >
> > > Could any other operation in `folio_unmap_invalidate()` sleep for
> > > unmapped folios? Otherwise, it seems we don't need `in_task()` at
> > > all?
We do need it. folio_unmap_invalidate() calls filemap_release_folio() which
calls into ->release_folio() handler and generally filesystems are allowed
to sleep in this handler. Now usually they don't actually sleep there but
they definitely grab a lot of irq unsafe locks there as Tal writes below.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On Mon, Sep 14, 2026 at 5:33 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 14-09-26 09:09:32, Barry Song wrote:
> > On Mon, Sep 14, 2026 at 7:34 AM Tal Zussman <tz2294@columbia.edu> wrote:
> > >
> > > On 9/13/26 6:51 PM, Barry Song wrote:
> > > > On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
> > > >>
> > > >> On 9/4/26 12:28 AM, Barry Song wrote:
> > > >> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
> > > >> >>
> > > >> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> > > >> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> > > >> >> >
> > > >> >> > Fault-around can map ready dropbehind folios without going through the
> > > >> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
> > > >> >> > competing cached user, but writeback completion can currently unmap that
> > > >> >> > folio. A later mmap access must then fault it back in.
> > > >> >> >
> > > >> >> > Retain mapped folios instead. For a mapped folio,
> > > >> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> > > >> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> > > >> >> > folio_end_dropbehind() runs in non-preemptible task context.
> > > >> >> >
> > > >> >> > Unmapped dropbehind folios continue through the existing invalidation path.
> > > >> >> >
> > > >> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> > > >> >> > Cc: stable@vger.kernel.org
> > > >> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > > >> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > > >> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> > > >> >> > ---
> > > >> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> > > >> >> > kernel retained the mapped folio and continued to evict the unmapped
> > > >> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> > > >> >> > am not aware of an existing report.
> > > >> >> >
> > > >> >>
> > > >> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> > > >> >> some help from Claude. The reproducer itself is straightforward enough, but the
> > > >> >> configuration was a little finicky. The atomic context comes from using
> > > >> >> threadirqs so that virtblk_done() completes requests from the irq thread under
> > > >> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> > > >> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> > > >> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> > > >> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
> > > >> >>
> > > >> >
> > > >> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> > > >> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> > > >> > do we still need all the complex logic in the block device and file
> > > >> > system layers to move dropbehind to a workqueue task context?
> > > >> >
> > > >>
> > > >> Hi Barry,
> > > >>
> > > >> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
> > > >> The existing in_task() check covered most other cases, in that it just
> > > >> doesn't do dropbehind invalidation if we're not executing in task context
> > > >> (for example, block device writeback completing in interrupt context).
> > > >>
> > > >> However, dropping the invalidation defeats the point of dropbehind, as it
> > > >> leaves the pages in the page cache. If we get rid of the task-context
> > > >> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
> > > >> Wenjie's change is largely unrelated to this.
> > > >
> > > > Hi Tal,
> > > >
> > > > Thanks for the clarification. However, I’m still not convinced.
> > > >
> > > > If `unmap_mapping_folio()` is the only place where we might
> > > > sleep — where we might take `mapping->i_mmap_rwsem` through
> > > > `i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
> > > > prevented that from happening with `if (!folio_mapped(folio))`.
> > > >
> > > > So no mapped folio should call `folio_unmap_invalidate()`:
> > > >
> > > > static void filemap_end_dropbehind(struct folio *folio)
> > > > {
> > > > struct address_space *mapping = folio->mapping;
> > > >
> > > > VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> > > >
> > > > if (folio_test_writeback(folio) || folio_test_dirty(folio))
> > > > return;
> > > > if (!folio_test_clear_dropbehind(folio))
> > > > return;
> > > > if (mapping && !folio_mapped(folio))
> > > > folio_unmap_invalidate(mapping, folio, 0);
> > > > }
> > > >
> > > > Why do we still need the `in_task()` check in
> > > > `folio_end_dropbehind()`?
> > > >
> > > > void folio_end_dropbehind(struct folio *folio)
> > > > {
> > > > if (!folio_test_dropbehind(folio))
> > > > return;
> > > >
> > > > /*
> > > > * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> > > > * but can happen if normal writeback just happens to find dirty folios
> > > > * that were created as part of uncached writeback, and that writeback
> > > > * would otherwise not need non-IRQ handling. Just skip the
> > > > * invalidation in that case.
> > > > */
> > > > if (in_task() && folio_trylock(folio)) {
> > > > filemap_end_dropbehind(folio);
> > > > folio_unlock(folio);
> > > > }
> > > > }
> > > >
> > > > Could any other operation in `folio_unmap_invalidate()` sleep for
> > > > unmapped folios? Otherwise, it seems we don't need `in_task()` at
> > > > all?
>
> We do need it. folio_unmap_invalidate() calls filemap_release_folio() which
> calls into ->release_folio() handler and generally filesystems are allowed
> to sleep in this handler. Now usually they don't actually sleep there but
> they definitely grab a lot of irq unsafe locks there as Tal writes below.
Hi Jan,
Thanks very much for your explanation. That makes a lot of
sense to me.
Then, based on the above, I am getting into another confusion and
have another question. If `release_folio()` is actually allowed
to sleep, and some filesystem might sleep there in the future,
wouldn't Wenjie's patch [1] make sense in that case, as it
guarantees that `->release_folio()` is always called in a
sleepable context, while `in_task()` does not provide such a
guarantee?
https://lore.kernel.org/linux-mm/20260820142956.1414337-1-qiwenjie@xiaomi.com/
Best Regards
Barry
On Mon 14-09-26 17:42:59, Barry Song wrote:
> On Mon, Sep 14, 2026 at 5:33 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 14-09-26 09:09:32, Barry Song wrote:
> > > On Mon, Sep 14, 2026 at 7:34 AM Tal Zussman <tz2294@columbia.edu> wrote:
> > > >
> > > > On 9/13/26 6:51 PM, Barry Song wrote:
> > > > > On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@columbia.edu> wrote:
> > > > >>
> > > > >> On 9/4/26 12:28 AM, Barry Song wrote:
> > > > >> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@columbia.edu> wrote:
> > > > >> >>
> > > > >> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> > > > >> >> > From: Wenjie Qi <qiwenjie@xiaomi.com>
> > > > >> >> >
> > > > >> >> > Fault-around can map ready dropbehind folios without going through the
> > > > >> >> > normal page-cache lookup that clears dropbehind. A mapping represents a
> > > > >> >> > competing cached user, but writeback completion can currently unmap that
> > > > >> >> > folio. A later mmap access must then fault it back in.
> > > > >> >> >
> > > > >> >> > Retain mapped folios instead. For a mapped folio,
> > > > >> >> > folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> > > > >> >> > i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> > > > >> >> > folio_end_dropbehind() runs in non-preemptible task context.
> > > > >> >> >
> > > > >> >> > Unmapped dropbehind folios continue through the existing invalidation path.
> > > > >> >> >
> > > > >> >> > Fixes: fb7d3bc41493 ("mm/filemap: drop streaming/uncached pages when writeback completes")
> > > > >> >> > Cc: stable@vger.kernel.org
> > > > >> >> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > > > >> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > > > >> >> > Reviewed-by: Tal Zussman <tz2294@columbia.edu>
> > > > >> >> > ---
> > > > >> >> > The mapped-plus-dropbehind state was reproduced in QEMU. The patched
> > > > >> >> > kernel retained the mapped folio and continued to evict the unmapped
> > > > >> >> > dontcache folio. I did not reproduce the sleeping-in-atomic warning and
> > > > >> >> > am not aware of an existing report.
> > > > >> >> >
> > > > >> >>
> > > > >> >> I was able to trigger the sleeping-while-atomic BUG on current mainline with
> > > > >> >> some help from Claude. The reproducer itself is straightforward enough, but the
> > > > >> >> configuration was a little finicky. The atomic context comes from using
> > > > >> >> threadirqs so that virtblk_done() completes requests from the irq thread under
> > > > >> >> local_bh_disable() and the vq spinlock with irqs off, so in_task() is true but
> > > > >> >> the i_mmap_rwsem down_read() sleeps. It also needed 1 vCPU (so completion stays
> > > > >> >> in the irq thread rather than softirq) and ext4 -o dioread_lock (so end_io runs
> > > > >> >> inline instead of on a workqueue). With v4 applied the bug is gone, so:
> > > > >> >>
> > > > >> >
> > > > >> > Thanks, Tal. Then I'm getting quite confused. Since with Wenjie's
> > > > >> > patch, we are no longer going to sleep for dropbehind in `filemap.c`,
> > > > >> > do we still need all the complex logic in the block device and file
> > > > >> > system layers to move dropbehind to a workqueue task context?
> > > > >> >
> > > > >>
> > > > >> Hi Barry,
> > > > >>
> > > > >> Let me clarify. Wenjie's patch prevents sleeping in a very specific case.
> > > > >> The existing in_task() check covered most other cases, in that it just
> > > > >> doesn't do dropbehind invalidation if we're not executing in task context
> > > > >> (for example, block device writeback completing in interrupt context).
> > > > >>
> > > > >> However, dropping the invalidation defeats the point of dropbehind, as it
> > > > >> leaves the pages in the page cache. If we get rid of the task-context
> > > > >> deferral mechanism, we wouldn't sleep, but we also wouldn't invalidate.
> > > > >> Wenjie's change is largely unrelated to this.
> > > > >
> > > > > Hi Tal,
> > > > >
> > > > > Thanks for the clarification. However, I’m still not convinced.
> > > > >
> > > > > If `unmap_mapping_folio()` is the only place where we might
> > > > > sleep — where we might take `mapping->i_mmap_rwsem` through
> > > > > `i_mmap_lock_read()`, as Wenjian pointed out in [1] — we have now
> > > > > prevented that from happening with `if (!folio_mapped(folio))`.
> > > > >
> > > > > So no mapped folio should call `folio_unmap_invalidate()`:
> > > > >
> > > > > static void filemap_end_dropbehind(struct folio *folio)
> > > > > {
> > > > > struct address_space *mapping = folio->mapping;
> > > > >
> > > > > VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> > > > >
> > > > > if (folio_test_writeback(folio) || folio_test_dirty(folio))
> > > > > return;
> > > > > if (!folio_test_clear_dropbehind(folio))
> > > > > return;
> > > > > if (mapping && !folio_mapped(folio))
> > > > > folio_unmap_invalidate(mapping, folio, 0);
> > > > > }
> > > > >
> > > > > Why do we still need the `in_task()` check in
> > > > > `folio_end_dropbehind()`?
> > > > >
> > > > > void folio_end_dropbehind(struct folio *folio)
> > > > > {
> > > > > if (!folio_test_dropbehind(folio))
> > > > > return;
> > > > >
> > > > > /*
> > > > > * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> > > > > * but can happen if normal writeback just happens to find dirty folios
> > > > > * that were created as part of uncached writeback, and that writeback
> > > > > * would otherwise not need non-IRQ handling. Just skip the
> > > > > * invalidation in that case.
> > > > > */
> > > > > if (in_task() && folio_trylock(folio)) {
> > > > > filemap_end_dropbehind(folio);
> > > > > folio_unlock(folio);
> > > > > }
> > > > > }
> > > > >
> > > > > Could any other operation in `folio_unmap_invalidate()` sleep for
> > > > > unmapped folios? Otherwise, it seems we don't need `in_task()` at
> > > > > all?
> >
> > We do need it. folio_unmap_invalidate() calls filemap_release_folio() which
> > calls into ->release_folio() handler and generally filesystems are allowed
> > to sleep in this handler. Now usually they don't actually sleep there but
> > they definitely grab a lot of irq unsafe locks there as Tal writes below.
>
> Hi Jan,
>
> Thanks very much for your explanation. That makes a lot of
> sense to me.
>
> Then, based on the above, I am getting into another confusion and
> have another question. If `release_folio()` is actually allowed
> to sleep, and some filesystem might sleep there in the future,
> wouldn't Wenjie's patch [1] make sense in that case, as it
> guarantees that `->release_folio()` is always called in a
> sleepable context, while `in_task()` does not provide such a
> guarantee?
>
> https://lore.kernel.org/linux-mm/20260820142956.1414337-1-qiwenjie@xiaomi.com/
Well, adding the folio_mapped() check makes sense as well as Matthew
explained - we shouldn't be dropping mapped folios(). But yes, now that
you've made me look into this I don't think dropping the in_task() check
here is correct and we still need to make sure that *at least* we are in
non-interrupt context and preferably to keep fs maintainer's sanity in a
sleepable context. Matthew?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On Sun, Aug 30, 2026 at 11:25 AM Wenjie Qi <qwjhust@gmail.com> wrote:
>
> From: Wenjie Qi <qiwenjie@xiaomi.com>
>
> Fault-around can map ready dropbehind folios without going through the
> normal page-cache lookup that clears dropbehind. A mapping represents a
> competing cached user, but writeback completion can currently unmap that
> folio. A later mmap access must then fault it back in.
Andrew is asking for a visible user-facing impact. I think the current
mainline code might result in an extra fault-in and extra I/O. Maybe we
should highlight this?
On the other hand, my gut feeling is that this patch might only be
50% right, rather than 100% right.
Fault-around might just end up mapping data that will never be accessed
at all (fault-around mappings aren't really fault-mapped), while the user
has explicitly requested "dropbehind", which seems like a strong hint
that they don't need it :-)
>
> Retain mapped folios instead. For a mapped folio,
> folio_unmap_invalidate() can call unmap_mapping_folio(), which takes
> i_mmap_rwsem and may sleep. Retaining the folio also avoids this path when
> folio_end_dropbehind() runs in non-preemptible task context.
>
I'm not quite sure this can be entirely avoided. Somehow,
`folio_launder(mapping, folio)` could also be nasty. For example, NFS
can still be problematic today:
static int nfs_launder_folio(struct folio *folio)
{
struct inode *inode = folio->mapping->host;
int ret;
dfprintk(PAGECACHE, "NFS: launder_folio(%llu, %llu)\n",
inode->i_ino, folio_pos(folio));
folio_wait_private_2(folio); /* [DEPRECATED] */
ret = nfs_wb_folio(inode, folio);
trace_nfs_launder_folio_done(inode, folio_pos(folio),
folio_size(folio), ret);
return ret;
}
Best Regards
Barry
Yes. The current path removes the folio from the page cache, so a later mmap access can incur both another fault and I/O. I should have stated that explicitly. For fault-around neighbors, folio_mapped() cannot distinguish the faulting page from a speculative neighbor. This patch takes the conservative policy that an installed PTE represents a competing cache user and wins over the writer's RWF_DONTCACHE hint. It can therefore retain a speculative neighbor which is never accessed. Distinguishing those cases seems to require fault-around to preserve the faulting folio while not mapping, or later dropping, dropbehind neighbors. I think that should be considered as a separate follow-up. For folio_launder(), filemap_end_dropbehind() holds the folio lock and returns if the folio is dirty or under writeback before calling folio_unmap_invalidate(). folio_launder() also immediately returns for a clean folio. I do not see how nfs_launder_folio() is reached from this completion path unless the locked, unmapped folio can become dirty between those checks. Is there a path I am missing?
On Sun, Aug 30, 2026 at 5:13 PM Wenjie Qi <qwjhust@gmail.com> wrote: > > Yes. The current path removes the folio from the page cache, so a later > mmap access can incur both another fault and I/O. I should have stated > that explicitly. > > For fault-around neighbors, folio_mapped() cannot distinguish the faulting > page from a speculative neighbor. This patch takes the conservative policy > that an installed PTE represents a competing cache user and wins over the > writer's RWF_DONTCACHE hint. It can therefore retain a speculative neighbor > which is never accessed. Distinguishing those cases seems to require > fault-around to preserve the faulting folio while not mapping, or later > dropping, dropbehind neighbors. I think that should be considered as a > separate follow-up. Yes. Maybe we can skip mapping neighbors with the dropbehind flag in fault-around. > > For folio_launder(), filemap_end_dropbehind() holds the folio lock and returns > if the folio is dirty or under writeback before calling > folio_unmap_invalidate(). folio_launder() also immediately returns for a > clean folio. I do not see how nfs_launder_folio() is reached from this > completion path unless the locked, unmapped folio can become dirty between > those checks. Is there a path I am missing? You are probably right. I was referring to the bit wait in nfs_launder_folio(), though I'm not quite sure whether this can actually happen: folio_wait_private_2(folio); /* [DEPRECATED] */ I assume we won't wait for `private_2` while the folio is clean. If so, it should be fine.
© 2016 - 2026 Red Hat, Inc.