On Wed, Jan 15, 2025 at 2:35 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jan 15, 2025 at 11:31:29AM +0200, Kirill A. Shutemov wrote:
> > -static void lru_deactivate_file(struct lruvec *lruvec, struct folio *folio)
> > -{
> > - bool active = folio_test_active(folio) || lru_gen_enabled();
> > - long nr_pages = folio_nr_pages(folio);
> > -
> > - if (folio_test_unevictable(folio))
> > - return;
> > -
> > - /* Some processes are using the folio */
> > - if (folio_mapped(folio))
> > - return;
> > -
> > - lruvec_del_folio(lruvec, folio);
> > - folio_clear_active(folio);
> > - folio_clear_referenced(folio);
> > -
> > - if (folio_test_writeback(folio) || folio_test_dirty(folio)) {
> > - /*
> > - * Setting the reclaim flag could race with
> > - * folio_end_writeback() and confuse readahead. But the
> > - * race window is _really_ small and it's not a critical
> > - * problem.
> > - */
> > - lruvec_add_folio(lruvec, folio);
> > - folio_set_reclaim(folio);
> > - } else {
> > - /*
> > - * The folio's writeback ended while it was in the batch.
> > - * We move that folio to the tail of the inactive list.
> > - */
> > - lruvec_add_folio_tail(lruvec, folio);
> > - __count_vm_events(PGROTATED, nr_pages);
> > - }
> > -
> > - if (active) {
> > - __count_vm_events(PGDEACTIVATE, nr_pages);
> > - __count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE,
> > - nr_pages);
> > - }
> > -}
>
> > +++ b/mm/truncate.c
> > @@ -486,7 +486,7 @@ unsigned long mapping_try_invalidate(struct address_space *mapping,
> > * of interest and try to speed up its reclaim.
> > */
> > if (!ret) {
> > - deactivate_file_folio(folio);
> > + folio_set_dropbehind(folio);
>
> brr.
>
> This is a fairly substantial change in semantics, and maybe it's fine.
>
> At a high level, we're trying to remove pages from an inode that aren't
> in use. But we might find that some of them are in use (eg they're
> mapped or under writeback). If they are mapped, we don't currently
> try to accelerate their reclaim, but now we're going to mark them
> as dropbehind. I think that's wrong.
>
> If they're dirty or under writeback, then yes, mark them as dropbehind, but
> I think we need to be a little more surgical here. Maybe preserve the
> unevictable check too.
Right -- deactivate_file_folio() does make sure the folio is not
unevictable or mapped. So probably something like below would the
change in semantics be close enough?
if (!folio_test_unevictable(folio) && !folio_mapped(folio))
folio_set_dropbehind(folio);
