Re: [PATCH v4] mm: filemap: retain mapped dropbehind folios

[Barry Song]

On Fri, Sep 4, 2026 at 6:36 AM Tal Zussman <tz2294@xxxxxxxxxxxx> wrote:
>
> On 9/4/26 12:28 AM, Barry Song wrote:
> > On Sun, Aug 30, 2026 at 7:59 PM Tal Zussman <tz2294@xxxxxxxxxxxx> wrote:
> >>
> >> On 8/30/26 6:25 AM, Wenjie Qi wrote:
> >> > From: Wenjie Qi <qiwenjie@xxxxxxxxxx>
> >> >
> >> > 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@xxxxxxxxxxxxxxx
> >> > Signed-off-by: Wenjie Qi <qiwenjie@xxxxxxxxxx>
> >> > Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> >> > Reviewed-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> >> > ---
> >> > 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