Re: [PATCH] mm: simplify zone_idx()

[gaoxu]

> 
> On Mon, Apr 14, 2025 at 09:57:26AM +1200, Barry Song wrote:
> > On Sat, Apr 12, 2025 at 10:06 PM gaoxu <gaoxu2@xxxxxxxxx> 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@xxxxxxxxxx/)
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()