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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7u3xvh3sz54xg7nii7lakwggpifv3ez7ffgmxjudj54bq463un@iyjjt5zosmup>
Date: Thu, 3 Apr 2025 15:31:03 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Rik van Riel <riel@...riel.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, kernel-team@...a.com, Vinay Banakar <vny@...gle.com>, 
	liuye <liuye@...inos.cn>, Hugh Dickins <hughd@...gle.com>, 
	Mel Gorman <mgorman@...hsingularity.net>, Yu Zhao <yuzhao@...gle.com>
Subject: Re: [PATCH v2] mm/vmscan: batch TLB flush during memory reclaim

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@...riel.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.