Re: [PATCH] mm: madvise: drop MADV_PAGEOUT folios at swap writeback completion

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Tue, Sep 22, 2026 at 07:10:34PM +0800, Barry Song wrote:
> On Tue, Sep 22, 2026 at 6:47 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
> >
> > On Tue, Sep 22, 2026 at 06:37:38PM +0800, Barry Song wrote:
> > > On Tue, Sep 22, 2026 at 6:20 PM David Hildenbrand (Arm)
> > > <david@kernel.org> wrote:
> > > >
> > > > On 9/21/26 23:56, Barry Song wrote:
> > > > > On Mon, Sep 21, 2026 at 11:37 PM David Hildenbrand (Arm)
> > > > > <david@kernel.org> wrote:
> > > > >>
> > > > >> On 9/21/26 17:24, Alexandre Ghiti wrote:
> > > > >>> On an asynchronous swap device MADV_PAGEOUT only marks the folio
> > > > >>> PG_reclaim and rotates it to the tail of the inactive list once its
> > > > >>> writeback completes, so the memory is not actually freed until a later
> > > > >>> reclaim scan removes the by then clean swap cache folio.
> > > > >> But we have the same behavior when just reclaiming memory ordinarily? It's added
> > > > >> to the swapcache and only the next scan actually frees up the memory.
> > > > >>
> > > > >> Wouldn't we memory we reclaim ... just gone, like in the sync case?
> > > > >
> > > > > For synchronous I/O, such as zswap and zram, the memory is released
> > > > > immediately after sync I/O is done.
> > > > >
> > > > > For asynchronous I/O, such as NVMe, the swapcache is currently
> > > > > expected to be rotated back to the tail of the LRU and wait for
> > > > > another scan. Alexandre once mentioned that when he tried handling
> > > > > async I/O the same way as sync I/O—releasing the memory once the I/O
> > > > > completed—he saw some regression. So, delaying the release until a
> > > > > later scan may allow swapcache hits before the folios are eventually
> > > > > reclaimed.
> > > >
> > > > "may", do we have any evidence that this actually is relevant in practice?
> > > >
> > > > We asked to reclaim memory. We wrote the memory out to disk. We unmapped it from
> > > > the page tables. We made the workload the could, access the page immediately
> > > > again suffer already.
> > > >
> > > > We should just evict them as soon as possible to free up memory.
> > >
> > > I suggested this to Alexandre, and he found that it could regress some
> > > workloads [1]. That is why Alexandre is only making the folios
> > > immediately reclaimable for `MADV_PAGEOUT`.
> > >
> > > See Alexandre's description:
> > >
> > > "Future work
> > > -----------
> > > Barry suggested extending this to MADV_PAGEOUT and general reclaim. I
> > > prototyped dropbehind for all reclaimed swap folios and it regressed
> > > sysbench OLTP throughput by ~15% on NVMe swap: dropping the swap cache
> > > immediately turns cheap in-cache refaults into disk reads and collapses
> > > swap readahead clustering. Neither blk-wbt, mq-deadline nor a PG_workingset
> > > gate recovered it. MADV_PAGEOUT alone may still be worth it, since there
> > > userspace has explicitly declared the range cold, but I have not measured
> > > that case in isolation yet."
> > >
> > > [1] https://lore.kernel.org/linux-mm/20260921151306.625134-1-alex@ghiti.fr/
> > >
> > > Best Regards
> > > Barry
> >
> > This patch as-is is just way way way WAY too complicated and fragile IMO.
> >
> > Whatever cases you have found, they need to be fixed somewhere
> > fundamental. All of this feels like a hack.
> >
> > If you're having to write a comment like:
> >
> > /*
> >  * If X is Y, but not if B, and if Z is J but not if the moon's bright at
> >  * night, then maybe we will foo the bar, but only if the baz is blarghed,
> >  * ...
> >  */
> >
> > That usually means you're doing something horribly wrong.
> >
> > And this patch has multiple comments like that.
>
> I'm not quite sure if you've read the context carefully :-) or why
> you're angry with me.
>
> I did suggest that we should reclaim memory immediately after
> writeback completes, both for general memory reclaim and for
> `MADV_PAGEOUT`, while Alexandre was working on zswap dropbehind.
>
> Alexandre then found that this might not be a good idea for generic
> reclaim, but thought it might still be worth trying for `MADV_PAGEOUT`.
> That's why this patch came about.
>
> The discussion between David and me is whether we should do this for
> `MADV_PAGEOUT` only, or also apply the same approach to generic
> reclaim. So far, we haven't even gotten into the code details.
>
> And this is also the first day I've seen the code. I agree that I can
> work with Alexandre to improve the code through review, but please
> keep in mind that this is literally my first day looking at it,
> my respected Lorenzo. :-)

I'm not angry with anybody, I'm saying the patch feels like a (fragile)
hack and the issue needs addressing more fundamentally, echoing David's
suggestion.

I also pointed out that when comments need to be extremely specific about
exactly why you're doing something, that usually means something is wrong.

To expand on that - the reason why is that clearly the logic you are
adjusting is assuming something.

You are violating that assumption, which is why the comment is there.

Generally that means the assumptions are wrong and something more
fundamental needs to be changed.

>
> Best Regards
> Barry

--
Cheers, Lorenzo