Re: [PATCH v7 mm-unstable] mm: vmscan: retry folios written back while isolated for traditional LRU

[Johannes Weiner <hannes@cmpxchg.org>]

On Tue, Jan 14, 2025 at 08:59:04PM -0700, Yu Zhao wrote:
> On Mon, Jan 13, 2025 at 8:52 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Sat, Jan 11, 2025 at 09:15:04AM +0000, Chen Ridong wrote:
> > > @@ -5706,6 +5706,44 @@ static void lru_gen_shrink_node(struct pglist_data *pgdat, struct scan_control *
> > >
> > >  #endif /* CONFIG_LRU_GEN */
> > >
> > > +/**
> > > + * find_folios_written_back - Find and move the written back folios to a new list.
> > > + * @list: filios list
> > > + * @clean: the written back folios list
> > > + * @lruvec: the lruvec
> > > + * @type: LRU_GEN_ANON/LRU_GEN_FILE, only for multi-gen LRU
> > > + * @skip_retry: whether skip retry.
> > > + */
> > > +static inline void find_folios_written_back(struct list_head *list,
> > > +             struct list_head *clean, struct lruvec *lruvec, int type, bool skip_retry)
> > > +{
> > > +     struct folio *folio;
> > > +     struct folio *next;
> > > +
> > > +     list_for_each_entry_safe_reverse(folio, next, list, lru) {
> > > +#ifdef CONFIG_LRU_GEN
> > > +             DEFINE_MIN_SEQ(lruvec);
> > > +#endif
> > > +             if (!folio_evictable(folio)) {
> > > +                     list_del(&folio->lru);
> > > +                     folio_putback_lru(folio);
> > > +                     continue;
> > > +             }
> > > +
> > > +             /* retry folios that may have missed folio_rotate_reclaimable() */
> > > +             if (!skip_retry && !folio_test_active(folio) && !folio_mapped(folio) &&
> > > +                 !folio_test_dirty(folio) && !folio_test_writeback(folio)) {
> > > +                     list_move(&folio->lru, clean);
> > > +                     continue;
> > > +             }
> > > +#ifdef CONFIG_LRU_GEN
> > > +             /* don't add rejected folios to the oldest generation */
> > > +             if (lru_gen_enabled() && lru_gen_folio_seq(lruvec, folio, false) == min_seq[type])
> > > +                     set_mask_bits(&folio->flags, LRU_REFS_FLAGS, BIT(PG_active));
> > > +#endif
> > > +     }
> >
> > Can't this solved much more easily by acting on the flag in the
> > generic LRU add/putback path? Instead of walking the list again.
> >
> > Especially with Kirill's "[PATCH 0/8] mm: Remove PG_reclaim" that
> > removes the PG_readahead ambiguity.
> 
> I don't follow -- my understanding is that with Kirill's series, there
> is no need to do anything for the generic path. (I'll remove the retry
> in MGLRU which Kirill left behind.)

Dropbehind trylocks the folio, so there is still a chance of misses
when the IO submission is from reclaim (which also relocks the folio
after IO submission).

Granted, that race window is much smaller compared to the window
between rotation and batched LRU putback. Probably best to retest on
top of all the pending patches.