Re: [PATCH v2 08/12] mm/mglru: simplify and improve dirty writeback handling

Kairui Song
2026-03-29T08:47:25Z

On Sun, Mar 29, 2026 at 4:21 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> Forwarding sashiko review:
>
> https://sashiko.dev/#/message/20260329-mglru-reclaim-v2-8-b53a3678513c%40tencent.com
>
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 8de5c8d5849e..17b5318fad39 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > [ ... ]
> > > @@ -4633,21 +4632,6 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> > > return true;
> > > }
> > >
> > > - dirty = folio_test_dirty(folio);
> > > - writeback = folio_test_writeback(folio);
> > > - if (type == LRU_GEN_FILE && dirty) {
> > > - sc->nr.file_taken += delta;
> > > - if (!writeback)
> > > - sc->nr.unqueued_dirty += delta;
> > > - }
> > > -
> > > - /* waiting for writeback */
> > > - if (writeback || (type == LRU_GEN_FILE && dirty)) {
> > > - gen = folio_inc_gen(lruvec, folio, true);
> > > - list_move(&folio->lru, &lrugen->folios[gen][type][zone]);
> > > - return true;
> > > - }
> > > -
> > > return false;
> > > }
> >
> > Could removing this proactive promotion cause a livelock and premature OOM
> > during GFP_NOIO or GFP_NOFS direct reclaim?
> > Without this check, dirty file folios fall through to isolate_folio(). If
> > the reclaim does not have __GFP_IO, isolate_folio() will skip them.
> > These skipped folios are spliced back to the front of the oldest generation
> > list in scan_folios(). Because isolate_folio() isolates 0 folios,
> > evict_folios() returns early.
> > This completely bypasses the new wakeup_flusher_threads() call, leaving the
> > dirty folios unqueued for writeback, and the oldest generation can never
> > advance.
>
> This is a nice found. For GFP_NOIO (or actually !__GFP_IO), we also need
> to active and set reclaim for the dirty folios. It's a narrow case though.
>
> Following update should work:
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 8170aee096e9..342ba3afe77c 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4641,8 +4641,7 @@ static bool isolate_folio(struct lruvec *lruvec, struct folio *folio, struct sca
>
> /* swap constrained */
> if (!(sc->gfp_mask & __GFP_IO) &&
> - (folio_test_dirty(folio) ||
> - (folio_test_anon(folio) && !folio_test_swapcache(folio))))
> + (folio_test_anon(folio) && !folio_test_swapcache(folio)))

Or this check should just be removed. shrink_folio_list already has a
check for swap and a more accurate may_enter_fs check.