Re: [PATCH] mm: skip dirty file folios during isolation of legacy LRU

[Zhaoyang Huang <huangzhaoyang@gmail.com>]

On Fri, Mar 20, 2026 at 5:20 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Fri, Mar 20, 2026 at 4:34 PM zhaoyang.huang
> <zhaoyang.huang@unisoc.com> wrote:
> >
> > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>
> Hi Zhaoyang,
>
> > Since dirty file folios are no longer writeout in reclaiming after
> > 'commit 84798514db50 ("mm: Remove swap_writepage() and
> > shmem_writepage()")', there is no need to isolate them which could help
> > to improve the scan efficiency and decrease the unnecessary TLB flush.
>
> But you are still isolating them with this patch, you just adjusted
> where the statistical update happens.
>
> And this is kind of opposite thing to what I'm trying to do here:
> https://lore.kernel.org/linux-mm/20260318-mglru-reclaim-v1-0-2c46f9eb0508@tencent.com/
>
> > This commit would like to bring the dirty file folios detection forward
> > to isolation phase as well as the statistics which could affect wakeup
> > the flusher thread under legacy LRU. In terms of MGLRU, the dirty file
> > folios have been brought to younger gen when sort_folios.
>
> If you really just skip isolating them, it could cause a regression:
> skipping the isolate and put it back will cause some ping pong effect
> on writeback / dirty folios as they will be stuck at inactive list. It
> will instead decrease scan efficiency.
>
> Currently shrink_folio_list will reactivate them and set the
> PG_reclaim flag. They will be deactivated by writeback callback.
> Simply changing that and the flusher wakeup logic could be a bad idea.
> You can check the link above and see the benchmark result.
>
> And for under writeback folios, there is no IPI flush or unmap as it
> was returned early. For dirty file folios they are unmapped indeed,
> but following flush should reclaim them anyway.
>
> It might be a good idea to skip the unmmap part for dirty file folio?
> Maybe, some benchmark is needed.
>
> >         while (scan < nr_to_scan && !list_empty(src)) {
> >                 struct list_head *move_to = src;
> > +               bool dirty, writeback;
> >                 struct folio *folio;
> >
> >                 folio = lru_to_folio(src);
> > @@ -1749,6 +1731,30 @@ static unsigned long isolate_lru_folios(unsigned long nr_to_scan,
> >                  */
> >                 scan += nr_pages;
> >
> > +               if (!folio_trylock(folio))
> > +                       goto move;
> > +               /*
> > +                * The number of dirty pages determines if a node is marked
> > +                * reclaim_congested. kswapd will stall and start writing
> > +                * folios if the tail of the LRU is all dirty unqueued folios.
> > +                */
> > +               folio_check_dirty_writeback(folio, &dirty, &writeback);
> > +               folio_unlock(folio);
>
> For LRU contention, to force active you always have to take it off the
> LRU first, folio_activate will take them off and touch LRU lock
> anyway. And now here, there is more work under lruvec lock and it is
> also trying to lock the folio under the lruvec lock. The LRU
> contention might get worse.
Thanks for the information and agree with you, it seems that the
simple and right thing is to have dirty folios skip try_to_unmap to
save TLB flush
>
> And the wakeup below seems very wrong, you just can't throttle or wait
> or sleep under LRU lock.
oh, sorry for the stupid change, I don't confirm the context for lock issue