linux-kernel - Re: [PATCH] mm: simplify zone

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: <b2bde1ca046949599d74cb9e88c21ba3@honor.com>
Date: Tue, 15 Apr 2025 12:34:27 +0000
From: gaoxu <gaoxu2@...or.com>
To: Matthew Wilcox <willy@...radead.org>, Barry Song <21cnbao@...il.com>
CC: Mike Rapoport <rppt@...nel.org>, Andrew Morton
	<akpm@...ux-foundation.org>, "surenb@...gle.com" <surenb@...gle.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, yipengxiang <yipengxiang@...or.com>
Subject: Re: [PATCH] mm: simplify zone_idx()

> 
> On Mon, Apr 14, 2025 at 09:57:26AM +1200, Barry Song wrote:
> > On Sat, Apr 12, 2025 at 10:06 PM gaoxu <gaoxu2@...or.com> wrote:
> > > The zone info obtained through T32 in the Android 15-6.6 system(arm64):
> > > (struct zone) struct (1664 bytes,
> >
> > I don't have a strong opinion on whether we need `zone_idx`—I'm okay
> > with having it or not. If you'd like to add it, feel free to send out
> > a v2 noting that it doesn't increase the struct size. If no one
> > objects, it might be a nice cleanup.
> 
> Plus it's already 1664 bytes!  And we have, what, 4 zones per NUMA node?
> Growing it doesn't feel like a big deal.  Although "saves two assembly
> instructions" is also not exactly a big win.  If it saved a cacheline reference,
> that might be more interesting, but it seems like it's more likely to introduce a
> cacheline reference than save one.  Maybe just not worth doing?

Zone, zone_pgdat, and node_zones are all considered hot data; most of the time,
they reside in the cache. In contrast, zone_idx in the patch is not hot data,
and executing ((zone)->zone_idx) will add a new cache line.
Am I understanding this correctly?

If the heat of the zone_idx function increases, this modification will become
worthwhile. For example, this patch will increase the heat of the zone_idx function.
The patch (https://lore.kernel.org/all/20240229183436.4110845-2-yuzhao@google.com/)
will add new hotspot paths, with the details as follows:
__zone_watermark_ok()->zone_is_suitable()->zone_idx()
zone_watermark_fast()->zone_is_suitable()->zone_idx()
get_page_from_freelist()->zone_is_suitable()->zone_idx()
__free_one_page()->zone_max_order()->zone_idx()