From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDE58C3601E for ; Sun, 13 Apr 2025 21:57:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2E94A6B014F; Sun, 13 Apr 2025 17:57:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 298F36B0155; Sun, 13 Apr 2025 17:57:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 04C546B0158; Sun, 13 Apr 2025 17:57:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id D98B26B014F for ; Sun, 13 Apr 2025 17:57:38 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 387A21A0FA2 for ; Sun, 13 Apr 2025 21:57:40 +0000 (UTC) X-FDA: 83330383080.24.DE86FC5 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf05.hostedemail.com (Postfix) with ESMTP id 4F28B100005 for ; Sun, 13 Apr 2025 21:57:38 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Rl5zlwdL; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744581458; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=B+KHsXWFX3rQhtFp9akHwtAVVobYH9F7JjLm3VRuREI=; b=linXZphj6HCMb+neBlL/GG/HbrsedDP33gzhS6L7QxbI1+Ke2seXuV8KIP5oj8yz45L7mI 2PDMbrIbN2Avavk4HYFZqzfMrC/KKeZ6JesyWwVtmb1360Cs03OxpEqchR6lva4Rx1lG+Q nwvWbGZcbLgTrDhPxicMfBpkcGA8v1Y= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Rl5zlwdL; spf=pass (imf05.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744581458; a=rsa-sha256; cv=none; b=BR/oCfa9QOse7+6e+9nqJtNS2YANb0r0DsSSTFzUMvKhmdeNhEMNgZYmov7gAXd8zcaQxr TWMM2LW7wRlE7RiPTQq3WQ0mtZjrSCYTpTDHTBel6qMWsOK6NSqi9x3XN0Hs7V8sTVlzf3 mzbV/VTbuepCFRBIHjqUN6LsXTH4Y9E= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-527b70bd90dso1632489e0c.3 for ; Sun, 13 Apr 2025 14:57:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1744581457; x=1745186257; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=B+KHsXWFX3rQhtFp9akHwtAVVobYH9F7JjLm3VRuREI=; b=Rl5zlwdLIHdYCNbZT77BsFemvh23jPx9E5IfR0DUgFU1J6d4uAsipHRPUwvxUImoj9 ap8lkb3GCOGsXQp7NULGt12RWg9+gsW091R4P1T37VfZyFF9P+z3RusTbOFy273hQNhz tat/KdL5jTVLaj6FVl+KPeJiM8wIDlpVY1J2uim3eGpMF8DCz5NfrC0YlPFJeh+PIvtI AvsI/BAB8q7hE28/7MWMtag2I6klWEUL9MGJem1Jld5yy+EEMulNGsTGK4Oyjbz3ZadS F2FXAAhNX1lwaQpw49MShS2M0Q/0pqz9LQV2udUVuCFyguEw4dn7ea4eE7QS7KWQZNU4 NKbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744581457; x=1745186257; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=B+KHsXWFX3rQhtFp9akHwtAVVobYH9F7JjLm3VRuREI=; b=sPHmPEa9GG0nhQpaVU0LroZ3BWLj80e0QLCxCAhwUI8Sr0hwJxnNkiOelzLzW2lIVB Rn1Ps5hzw2jhydhAlHEVTM3R6YEcO5B7yZsuq5LkPQ2+sRqH5TNltWz9hZBR6fH9iuKB O2u4X0osstCCLT11aiWl6vMg89OmwwXWuEMLtAJEzek7O/sKEujIvp0zITXlGPYocBjC 2pDekJweHm9EaTVVrXj8068Ga3Yxcx+n6km0cWPDGgONwAXPIbBMIEXc7ehsfgcdgr96 L8I+GBuPKQKDSvjt+rqo48mujyny3apkIY3T0niI2wWOeF1PWBf50eI2csjKrCR/Qyi6 Uulw== X-Forwarded-Encrypted: i=1; AJvYcCVHckHhdjfT9ItGlCjjcbcKQUvhU6wOqc1LGOhKWxH01595e5jrFtkfdpjjD0SpNozZ0zrubp/9gw==@kvack.org X-Gm-Message-State: AOJu0YwqKnIlYn7zNIs0/bvabviDwJcArW4RENU0MszKqGLaqCeMWAne i/pDUUULvUcGGXy2mN8M7hy2JWQ6jAQAkZM4sHBuBemOznoCyd81xjqw4sDbNKHWZEd/QuP5lGH K0on9kAA8/1xddJCmIDmh8EAuPME= X-Gm-Gg: ASbGncsZVN8XhvInMEtm/XVVMH+RZVnpf123x+DcTiPIIWuHNCDQan393gewaqKn9Cy bBZCE5ln+oPHKBGZ6AGHPZSS85RKxVLyqSySOYzNh+du8olnGNEXG2fsiCEQMKhKZOEKj1barYm 9K2jo7siVBidPk2jD+Qb1Q3w== X-Google-Smtp-Source: AGHT+IG3Zw4RW597XC+bS89jWCMvGOSdkanmkcT2FQo726Ddrdrwfc+6gui/G20yeRuv448PYMSB08LahlDIFRTzmrI= X-Received: by 2002:a05:6122:8c6:b0:526:720:704 with SMTP id 71dfb90a1353d-527c359743fmr5985825e0c.7.1744581457183; Sun, 13 Apr 2025 14:57:37 -0700 (PDT) MIME-Version: 1.0 References: <2d42decac5194c2c8d897b0424f0dcf3@honor.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Mon, 14 Apr 2025 09:57:26 +1200 X-Gm-Features: ATxdqUHfxB6zE_onPyS5JIkQClurElIiYFcoRX3HtdGibPrA2baUQ_IEg5mnvVM Message-ID: Subject: Re: [PATCH] mm: simplify zone_idx() To: gaoxu Cc: Mike Rapoport , Andrew Morton , "surenb@google.com" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , yipengxiang Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4F28B100005 X-Stat-Signature: 9djd6it13f8rc3wuauj3jatotihmrpwb X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1744581458-430122 X-HE-Meta: U2FsdGVkX18BdCcBJGqFpxqqUs26Yhl7ieMP11Viiq2bz4cklY4vhAWfPfMDaIyj/R8TFotPWcX70UvW7fLtY7mxlE/yzZQa/coAMJXhrJ2+r48pm+wstlHV+qtE13HSA4YOySyBe8T9ADAsAY3gDzsqrqMx4BDIap/mkRdyGlLQp5h3zpeItten/6Iv35mNaC332KEkN44UsOpyZo/S/TVB8Lysq/ZCX+QL/W9aeMA7NvG0AB7PtfkaD+efrJddg241QdcigxtMkR8WLjbrY4zGbRqKPyqGtDeuoqr9BxG4V3INZM4RiwEJIk8J/Q1JkLqI6hqqjC81KnO+HzMW+BKJtkzouAM4Uxyg4cDK856h/HCEMPSKyNBw1B/S1LE4dk99nt8+5SHU0bsFjFwHqRCLyX9N3Me0egxrufhwaLarmp5VhRbz4X/9KeCJH7YhUAYTLomvPwq6yirSQ8dCDZQGBKD3f3U+nfNj2aQBgwzyYMMAP2jM4Ant+AAXT9If0DApJFsCgu9bInrln5PnMQlALvfypRjAdC8WrCepYzdbewOXEhmzymXb9ywFAf2riTKuQEuSKhHLIR3QwH1vftiuedr9kts9KvK0p3mhbNwP35+nbLq6ZZrJ7R2MQqaucrbTSscqNE31MqKuUqIqs9H+0CBIgKtVFbf5NUUrneTK0IFZdzVDdIyy796oIuYmvl+DtkLFMuskGIpKVh3ijfONexcBc/dxTNlAiXqQe66y7UL/arIwP4l7o5MacWo6WfbC70HuNc3a2xgWaZvlwhT1N3fMPSTgadXhTPpYGebL/hN9rG1vTcbK7pHsWoj2GwYx96Oppzc7Fb7OOwm9hOj6AA7vkXFsjmKoxeVsV/dfjlZ9JNOuht2KAkMpK5OvOlzi/Ba59K7OYvqAGx7DM2VTUxBkU9R+BIwi9dAS/SumYpxo3c5t1rxHLK9TWtnsOZNYTgCQ4MZUmeGREun EEN6rLM9 AvFYCvDCt4C5uZ7TZjAjbeTmZ3jw7xHooxWopkk+gxEHmMMz7m/LlAle5VhCWeVhLo9M73XTRvmL2cGT8uTOvbsMSXkwkfet9xcdjYNEgpLwvPW2tPkJdHDi3FzO85eeUR4gsy32XVhKAw+iulqp+R010r3JdlZX61ODOC2C6ksVzhuABKy0cKWZ9dxlSECksFieXrRh0XLBH0LkHTohXSVT+WZ2ezDbQUAwAd2k70rdqbJ4N5+8BBaFBu7KJCsA42ieX7ukBISLyplkW6xLEkDpE8Sqs2Gw4wK/xQO5s1cVbGs4XIUuJVijObgn1sA3120eg1kKdyFS4CpDKe++GeJIFYqypHaPeXguMTwpn38oyHk+21FSFpirOWjBlQ9Fcm6Wr5pxdepCHiBH8tBRM5GDpPGE/mywfWEZK0JwnerYdPuZ9QpzAMzMtJjZpBWQkYjGk9MbBQXBYtyAiBnLmbC568bGy+kg8Kj9wKqD4TQTxLD8sjOoNzRPxOI+awJKyFm0KY89WjYzOr6/62aOIGARA0uvZ9Ar6dC8TwiZrfqZ/+us= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Apr 12, 2025 at 10:06=E2=80=AFPM gaoxu wrote: > > > > > On Sat, Apr 12, 2025 at 8:34=E2=80=AFPM gaoxu wrote: > > > > > > > > > > > On Fri, Apr 11, 2025 at 2:42=E2=80=AFAM Mike Rapoport wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Thu, Apr 10, 2025 at 12:03:00PM +0000, gaoxu wrote: > > > > > > store zone_idx directly in struct zone to simplify and optimize > > > > > > zone_idx() > > > > > > > > > > Do you see an actual speed up somewhere? > > > Almost negligible. my simple code tests showed the patch provides an = average > > improvement of ~0.02%. > > > Thus, in the Android 15-6.6 kernel, I confidently retained the origin= al zone_idx > > function. > > > (https://android-review.googlesource.com/c/kernel/common/+/3578322/2/= m > > > m/page_alloc.c#770) > > > > > > This patch only eliminates 2-3 assembly instructions, making it > > > challenging to observe measurable performance benefits. > > > However, since the zone struct includes CACHELINE_PADDING (reserving > > > unused space), adding a new member variable does not alter the size o= f > > > zone. This makes the patch effectively zero-cost while achieving a cl= eaner > > implementation of zone_idx. > > > > The struct zone contains many CONFIG_ options to include or exclude cer= tain > > fields. > > If we're confident that our new zone_idx doesn't increase cacheline usa= ge for all > > those cases, this seems like a worthwhile cleanup. Have you checked the > > numbers? > > The zone info obtained through T32 in the Android 15-6.6 system(arm64): > (struct zone) struct (1664 bytes, > [0] unsigned long [4] _watermark, > [32] unsigned long watermark_boost, > [40] unsigned long nr_reserved_highatomic, > [48] long [5] lowmem_reserve, > [88] struct pglist_data * zone_pgdat, > [96] struct per_cpu_pages * per_cpu_pageset, > [104] struct per_cpu_zonestat * per_cpu_zonestats, > [112] int pageset_high, > [116] int pageset_batch, > [120] unsigned long zone_start_pfn, > [128] atomic_long_t managed_pages, > [136] unsigned long spanned_pages, > [144] unsigned long present_pages, > [152] unsigned long present_early_pages, > [160] unsigned long cma_pages, > [168] const char * name, > [176] unsigned long nr_isolate_pageblock, > [184] seqlock_t span_seqlock, > [192] int order, > [196] int initialized, > [256] struct cacheline_padding _pad1_, > [256] struct free_area [11] free_area, > [1400] unsigned long flags, > [1408] spinlock_t lock, > [1472] struct cacheline_padding _pad2_, > [1472] unsigned long percpu_drift_mark, > [1480] unsigned long compact_cached_free_pfn, > [1488] unsigned long [2] compact_cached_migrate_pfn, > [1504] unsigned long compact_init_migrate_pfn, > [1512] unsigned long compact_init_free_pfn, > [1520] unsigned int compact_considered, > [1524] unsigned int compact_defer_shift, > [1528] int compact_order_failed, > [1532] bool compact_blockskip_flush, > [1533] bool contiguous, > [1536] struct cacheline_padding _pad3_, > [1536] atomic_long_t [11] vm_stat, > [1624] atomic_long_t [0] vm_numa_event) > > 1) It can be observed that there are 56B of free space in CACHELINE_PADDI= NG(pad1)=EF=BC=9B > 2) Before the variables in CACHELINE_PADDING(pad1), there are two CONFIGs= that are not enabled in Android 15-6.6: > #ifdef CONFIG_NUMA > int node; > #endif > > #ifndef CONFIG_SPARSEMEM > unsigned long *pageblock_flags; > #endif /* CONFIG_SPARSEMEM */ > These two CONFIGs occupy 16B. > 3) Compared to the latest Linux code, two new variables, unsigned long nr= _free_highatomic and int pageset_high_max, occupy an additional 16B; > Based on the above analysis, there are still 24B of free space before CAC= HELINE_PADDING(pad1). > (If I misunderstand, please point it out in a timely manner. Thank you!) > > It would be more appropriate to place the newly added variable zone_idx b= efore initialized. I don't have a strong opinion on whether we need `zone_idx`=E2=80=94I'm oka= y 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. > > > > > > > > > > +1. Curious if there's data indicating zone_idx is a hot path. > > > There are several functions in the memory management code that are > > > frequently executed and will call zone_idx: > > > rmqueue()->wakeup_kswapd()->zone_idx() > > > alloc_pages_bulk_noprof()->__count_zid_vm_events()->zone_idx() > > > > > > The patch > > > (https://lore.kernel.org/all/20240229183436.4110845-2-yuzhao@google.c= o > > > m/) 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() > > > > > > Although The patch > > > (https://lore.kernel.org/all/20240229183436.4110845-2-yuzhao@google.c= o > > > m/) has not yet merged into the Linux mainline; it is already include= d > > > in Android 15-6.6. > > > > > > > > > > > > > > > > > > Signed-off-by: gao xu > > > > > > --- > > > > > > include/linux/mmzone.h | 3 ++- > > > > > > mm/mm_init.c | 1 + > > > > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > > > > > > index 4c95fcc9e..7b14f577d 100644 > > > > > > --- a/include/linux/mmzone.h > > > > > > +++ b/include/linux/mmzone.h > > > > > > @@ -941,6 +941,7 @@ struct zone { #endif > > > > > > > > > > > > const char *name; > > > > > > + enum zone_type zone_idx; > > > > > > > > > > > > #ifdef CONFIG_MEMORY_ISOLATION > > > > > > /* > > > > > > @@ -1536,7 +1537,7 @@ static inline int local_memory_node(int > > > > > > node_id) > > > > { return node_id; }; > > > > > > /* > > > > > > * zone_idx() returns 0 for the ZONE_DMA zone, 1 for the > > > > > > ZONE_NORMAL > > > > zone, etc. > > > > > > */ > > > > > > -#define zone_idx(zone) ((zone) - > > > > (zone)->zone_pgdat->node_zones) > > > > > > +#define zone_idx(zone) ((zone)->zone_idx) > > > > > > > > > > > > #ifdef CONFIG_ZONE_DEVICE > > > > > > static inline bool zone_is_zone_device(struct zone *zone) diff > > > > > > --git a/mm/mm_init.c b/mm/mm_init.c index 9659689b8..a7f7264f1 > > > > > > 100644 > > > > > > --- a/mm/mm_init.c > > > > > > +++ b/mm/mm_init.c > > > > > > @@ -1425,6 +1425,7 @@ static void __meminit > > > > > > zone_init_internals(struct > > > > zone *zone, enum zone_type idx, > > > > > > atomic_long_set(&zone->managed_pages, remaining_pages); > > > > > > zone_set_nid(zone, nid); > > > > > > zone->name =3D zone_names[idx]; > > > > > > + zone->zone_idx =3D idx; > > > > > > zone->zone_pgdat =3D NODE_DATA(nid); > > > > > > spin_lock_init(&zone->lock); > > > > > > zone_seqlock_init(zone); > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > -- > > > > > Sincerely yours, > > > > > Mike. > > > > > > Thanks Barry