Re: [PATCH v2] mm/vmscan: batch TLB flush during memory reclaim

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Apr 03, 2025 at 03:00:55PM -0700, Andrew Morton wrote:
> On Fri, 28 Mar 2025 14:20:55 -0400 Rik van Riel <riel@surriel.com> wrote:
> 
> > The current implementation in shrink_folio_list() performs a full TLB
> > flush for every individual folio reclaimed. This causes unnecessary
> > overhead during memory reclaim.
> > 
> > The current code:
> > 1. Clears PTEs and unmaps each page individually
> > 2. Performs a full TLB flush on every CPU the mm is running on
> > 
> > The new code:
> > 1. Clears PTEs and unmaps each page individually
> > 2. Adds each unmapped page to pageout_folios
> > 3. Flushes the TLB once before procesing pageout_folios
> > 
> > This reduces the number of TLB flushes issued by the memory reclaim
> > code by 1/N, where N is the number of mapped folios encountered in
> > the batch processed by shrink_folio_list.
> 
> Were any runtime benefits observable?

Andrew, can you hold off this patch for now? I provided some feedback
privately but let me put it here as well.

This patch is very very hard to review. shrink_folio_list() has become a
beast over the years. This patch is moving a code block within the same
function and skipping a lot of stuff happening between the old place and
the new place. I still couldn't figure out how the actual freeing of
folios are happening as the patch completely skips
mem_cgroup_uncharge_folios() & free_unref_folios(). Also the lazyfree
counters are skipped. In addition buffer head, swap free, mlocked
handling will be skipped as well.

I think there is a need to explain why this patch is correct even with
skipping all those functionalities.