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

[Zhaoyang Huang]

On Fri, Mar 20, 2026 at 5:20 PM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> On Fri, Mar 20, 2026 at 4:34 PM zhaoyang.huang
> <zhaoyang.huang@xxxxxxxxxx> wrote:
> >
> > From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
>
> 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.
sorry, I missed the above information in previous feedback. No. The
dirty file folios are moved back to lruvec instead of being isolated
under this patch. How about apply this only when isolate_lru_folios is
called from shrink_active_list which has no worries about stuck the
inactive list.
>
> 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@xxxxxxxxxxx/
>
> > 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.
>
> And the wakeup below seems very wrong, you just can't throttle or wait
> or sleep under LRU lock.