Re: [PATCH v2] mm/mglru: use folio_mark_accessed to replace folio_set_active

[Barry Song]

> >
> > And do you think we can keep a folio_set_active if folio_test_anon ==
> > true in swap.c? Proactive activation relies on the assumption that
> > applications never expect an jitter for ordinary memory access, which
> > holds true for anon folios, but file folios don't follow that idea.
>
> I’m not entirely sure, but I see that removing folio_set_active()
> could also help reduce anon refaults. Also, having two separate
> branches for file and anon seems to make the code a bit ugly.

I did a quick test by restoring anon’s behavior as shown below:

--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -242,8 +242,9 @@ static inline unsigned long
lru_gen_folio_seq(const struct lruvec *lruvec,
                gen = MIN_NR_GENS - folio_test_workingset(folio);
        else if (reclaiming)
                gen = MAX_NR_GENS;
-       else if (folio_test_reclaim(folio) &&
-                 (folio_test_dirty(folio) || folio_test_writeback(folio)))
+       else if ((!folio_is_file_lru(folio) && !folio_test_swapcache(folio)) ||
+                (folio_test_reclaim(folio) &&
+                 (folio_test_dirty(folio) || folio_test_writeback(folio))))
                gen = MIN_NR_GENS;
        else
                gen = MAX_NR_GENS - (folio_test_workingset(folio) ||
folio_test_referenced(folio));

I can still see significant anon refault reduction, so I agree I can keep the
existing behavior for anon. However, there is no need to set active, since
(!folio_is_file_lru(folio) && !folio_test_swapcache(folio)) is hardcoded there.

I guess it is probably because file behavior is improved, which then indirectly
improves anon as well. So I feel there is no need to change the subject in v3.

Thanks
Barry