Re: [PATCH v2 00/26] mm/fbatch: drain lru_add_drain() and _all()

[JP Kobryn <jp.kobryn@linux.dev>]

Hi Hugh,

On 9/9/26 2:39 AM, Hugh Dickins wrote:
> [PATCH v2 00/26] mm/fbatch: drain lru_add_drain() and _all()
> 
> This series was prompted by lru_add_drain_all() appearing in watchdog
> backtraces: not to blame, but blocked on an unresponsive CPU to run
> its workqueue. lru_add_drain_all() is a heavyweight operation, which
> we often hope to avoid by a much lighter lru_add_drain(); but even
> those local drains can aggravate the lruvec lock contention which
> per-cpu fbatches are intended to ease.
> 
> Now let the folios on the lru_add and other per-cpu fbatches remain
> there, isolatable, with PG_lru set and refcount unraised, just like
> when on an actual LRU. Then many calls to lru_add_drain() and _all()
> can be removed.
> 
> It's something I've wanted to do for years, tried several times, but
> only hit on a good way to do it a few weeks ago: now it's uncomfortable
> to watch others wrestling with the drainage, while I'm sitting on this.
> 
> The key idea came from the "if (!folio_test_clear_lru(folio)) continue;".
> If we don't mind missing to take an action on rare occasions, then maybe
> we won't mind taking an action on the wrong folio on rare occasions, so
> long as it is a folio consenting to PG_lru rules.
> 
> No new locking, but relies on folio_try_get() and folio_test_clear_lru()
> even on the lru_add fbatch; with use of bits not set in aligned pointers,
> and some try_cmpxchg()ing.  Speculative references to folios are already
> accepted: this adds another source of them.
> 
> Performance? You (and the bots) tell me. I'm considering this as a
> cleanup, to make life easier for developers. I expect that some loads
> will show improvement, but also expect some disappointments (perhaps
> I go too far against lru_cache_disable()? or not far enough).
> 
I found one source of regression but I think it's fixable.

Using a test of 4 processes repeatedly write faulting anon pages then
discarding with MADV_DONTNEED, the perf A/B below shows that the series
(minus patch 27) spends more time in __page_cache_release due to new
lru_lock traffic.

baseline (7891fbb9512f):
  0.03%  folio-churn  [kernel.kallsyms]  [k] queued_spin_lock_slowpath
  0.26%  folio-churn  [kernel.kallsyms]  [k] _raw_spin_lock_irqsave
  0.01%  folio-churn  [kernel.kallsyms]  [k] folio_lruvec_lock_irqsave
  0.00%  folio-churn  [kernel.kallsyms]  [k] folio_lruvec_relock_irqsave
  0.03%  folio-churn  [kernel.kallsyms]  [k] __page_cache_release

series (without patch 27):
      6.97%  folio-churn  [kernel.kallsyms]  [k] _raw_spin_lock_irqsave
             |
             ---_raw_spin_lock_irqsave
                |
                 --6.96%--folio_lruvec_lock_irqsave
                           folio_lruvec_relock_irqsave
                           |
                            --6.72%--__page_cache_release
                                      folios_put_refs
                                      free_pages_and_swap_cache
                                      tlb_flush_mmu
                                      tlb_finish_mmu
                                      do_madvise
                                      __x64_sys_madvise
                                      do_syscall_64
                                      entry_SYSCALL_64_after_hwframe
                                      __madvise

      6.62%  folio-churn  [kernel.kallsyms] [k] queued_spin_lock_slowpath
             |
             ---queued_spin_lock_slowpath
                _raw_spin_lock_irqsave
                folio_lruvec_lock_irqsave
                folio_lruvec_relock_irqsave
                |
                 --6.43%--__page_cache_release
                           folios_put_refs
                           free_pages_and_swap_cache
                           tlb_flush_mmu
                           tlb_finish_mmu
                           do_madvise
                           __x64_sys_madvise
                           do_syscall_64
                           entry_SYSCALL_64_after_hwframe
                           __madvise

There was a pre-existing dead folio optimization [0] which allowed
caller-released folios remaining on batches to be cleaned up at drain
time without lru_lock acquisition. This series removes it which makes
sense since the batch reference concept is gone. But now that callers
effectively invoke the cleanup when refcounts go directly to zero,
there's a point in which an lru_lock will be taken for pending folios
that are dead (refcount zero but still on batch). That's seen in the
call graphs above and is unnecessary since these folios haven't been
inserted into the LRU. Luckily It looks like the new API call
lru_add_del_folio() can be used to avoid this, since it returns true
after cleaning up the pending LRU batch state. I applied this diff and
re-ran the same benchmark. The new locking overhead was nearly all
eliminated. The perf output afterward shows the fractional amount that
remains.

@@ -64,8 +64,10 @@ static void __page_cache_release(struct folio *folio, 
struct lruvec **lruvecp,
                 unsigned long *flagsp)
  {
         if (folio_test_lru(folio)) {
-               folio_lruvec_relock_irqsave(folio, lruvecp, flagsp);
-               lruvec_del_folio(*lruvecp, folio);
+               if (!lru_add_del_folio(folio)) {
+                       folio_lruvec_relock_irqsave(folio, lruvecp, flagsp);
+                       lruvec_del_folio(*lruvecp, folio);
+               }
                 __folio_clear_lru_flags(folio);
         }
  }

series modified with patch above (without patch 27):
      0.77%  folio-churn  [kernel.kallsyms]  [k] queued_spin_lock_slowpath
             |
             ---queued_spin_lock_slowpath
                _raw_spin_lock_irqsave
                |
                 --0.77%--folio_lruvec_lock_irqsave
                           folio_lruvec_relock_irqsave
                           |
                            --0.52%--__page_cache_release
                                      folios_put_refs
                                      free_pages_and_swap_cache
                                      tlb_flush_mmu
                                      tlb_finish_mmu
                                      do_madvise
                                      __x64_sys_madvise
                                      do_syscall_64
                                      entry_SYSCALL_64_after_hwframe
                                      __madvise

      0.37% folio-churn [kernel.kallsyms] [k] _raw_spin_lock_irqsave
      0.04% folio-churn [kernel.kallsyms] [k] folio_lruvec_lock_irqsave
      0.01% folio-churn [kernel.kallsyms] [k] folio_lruvec_relock_irqsave

[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/mm?id=9669b87065a6fe96198f3df2c3d125c5f5c1f210