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 6DC80C30653 for ; Thu, 4 Jul 2024 07:57:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EBF7A6B00B5; Thu, 4 Jul 2024 03:57:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E71456B00B6; Thu, 4 Jul 2024 03:57:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D10496B00B7; Thu, 4 Jul 2024 03:57:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B48526B00B5 for ; Thu, 4 Jul 2024 03:57:51 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 2A31E1408FB for ; Thu, 4 Jul 2024 07:57:51 +0000 (UTC) X-FDA: 82301316342.25.C4E50EE Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.18]) by imf09.hostedemail.com (Postfix) with ESMTP id D680B140006 for ; Thu, 4 Jul 2024 07:57:48 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="QI/uwYg7"; spf=pass (imf09.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.18 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720079845; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5j7r6LvOcnwmSbuKNWTcI8GEEYNu9yVIMtNqr5GCQ64=; b=x8jZDk6MrOzNgKhTGrPidkTGCSZTBU90GPBWWw2Mg6826wB1hRAodPIFF4Cq43zJkZ0Mno HXu6kncF8yIASrt7M/Na3Nmo7EUNzxE6QRzfR8mxxRVVZXseQITw2cR8FCWP0Pg5MDqYtr RugHVxIbzkYNb0E9QuZ+7RjcB3jYrW0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720079845; a=rsa-sha256; cv=none; b=6xwfgJzF6fsQJRUP3JhXgvghdhEoGVwcQ7EAkxDvjzUzseqqxd5hPaaNWTk5wawzkXKp83 +m0on6IC0cBU1DXMOlMNmw55Ddlr/ouza/iDJpu7GKsCutzpehafUbL6QBispiqltFk4aX bI4MLgRjAvGwyPmT56scR8PmNjLUPrc= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="QI/uwYg7"; spf=pass (imf09.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.18 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720079869; x=1751615869; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=+nlEZm29FY0yjN+onfpT9t6OyQeU1Kx/26D0jQA3u8A=; b=QI/uwYg7OCntj5U4EMgxSUXaQpYxdy8HlwrsfeWou4OQtpmxYi57Ie7L eXl2qiDtZHBuD0v1fB+M4G9Co1oeAvynVQudC6tfu+Qr3fR0JFpZBm/Z+ n4HQHuYBXaDiVc7im/4zlqKXTQCyiI6o3iMPXPhhNwS6HlQPvXmPVqkcd O7ZCwGvL+HV+/qhrq6jJ+UqJIXX5BjOxCfn0TVZ96UHxJVt2ZZSvTl+Ub 5SYuIFFIz/NIOasJ6TYZsuoI1jSkLK4TBGcbyFbf0vEaZOCKSogyDoXef MsFq+KIYm/Of2FQ/OVAU/Fsc9b4d/Aex7pEZlbO2t5KD0Vh3aK6x3LIue w==; X-CSE-ConnectionGUID: iXgNabwiRbGsu4OkRslcxg== X-CSE-MsgGUID: TGh5DpQBTDae1Hxsh8nCLA== X-IronPort-AV: E=McAfee;i="6700,10204,11122"; a="17484924" X-IronPort-AV: E=Sophos;i="6.09,183,1716274800"; d="scan'208";a="17484924" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa110.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jul 2024 00:57:40 -0700 X-CSE-ConnectionGUID: zZCUaaKITQGaYrZ7AzctJg== X-CSE-MsgGUID: 2WZ2FzmURDqjsTbeqHjHSw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,183,1716274800"; d="scan'208";a="46932661" Received: from unknown (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jul 2024 00:57:30 -0700 From: "Huang, Ying" To: "Ho-Ren (Jack) Chuang" Cc: "Jonathan Cameron" , "Gregory Price" , aneesh.kumar@linux.ibm.com, mhocko@suse.com, tj@kernel.org, john@jagalactic.com, "Eishan Mirakhur" , "Vinicius Tavares Petrucci" , "Ravis OpenSrc" , "Alistair Popple" , "Srinivasulu Thanneeru" , "SeongJae Park" , "Rafael J. Wysocki" , Len Brown , Andrew Morton , Dave Jiang , Dan Williams , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, "Ho-Ren (Jack) Chuang" , "Ho-Ren (Jack) Chuang" , "Ho-Ren (Jack) Chuang" , linux-cxl@vger.kernel.org, qemu-devel@nongnu.org Subject: Re: [PATCH v3] memory tier: consolidate the initialization of memory tiers In-Reply-To: <20240704072646.437579-1-horen.chuang@linux.dev> (Ho-Ren Chuang's message of "Thu, 4 Jul 2024 07:26:44 +0000") References: <20240704072646.437579-1-horen.chuang@linux.dev> Date: Thu, 04 Jul 2024 15:55:39 +0800 Message-ID: <87tth5h9qc.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: D680B140006 X-Stat-Signature: cwa4wkewuz5c669wkigkbqo8who16i68 X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1720079868-31718 X-HE-Meta: U2FsdGVkX1+/cXK4sf69dMi5G+5hEWG4bfI7Nj7ObcIRuhmCp89JWb4l9qCVceGQT9f2deq6gJdLz9gDqnqwn1uUNUHd9CRdL26qqLdH7yr25cp+k/QX5WY/brryGE6lxhpOTEp617MXGLVR80qJ9GZgu12gTH5QqKjOIT3KddAR8CZ15W2B7CqLDCLBGBvHol7WMVMLuLJntxhaHdbkiqnI5e5d7mEUn7glaMc6Ifu9ttp8TMLWXbAS4bG0iWYPai+RuuyV6vx3SJkQ2yb6sug+7XD3qUFv5ESllZATUI8USnw5LBEo168GYxYHoOJ3PC0k6TaxpeMaXMYqtT11UzACr/Ye3hr62yUvFw6DeHP9/vjL6igggYVBkAnVqAOk4jXaLqm0xsfOUDPWZyClUylaSgloWXCWqbsEBlos6DLXmzP9DiM4J7WpqNmXEutCwUgBaQl+yAD2Y7pQwv5YZC3/13eg72duN7qZkWQJsd6cYljgy6mjftGUpd81lk4zrrmfrmUkNDK35xQi0YnW7pLxWo4F3t9Ba9Z6K7zQfSsfukek9wcmpGT6GaxSSrOU5gSHcTbyigYvmgUGOf+9hs6SMP9WNrwo+M+jG66I/EnxvgA9N70+qvoA0WWAkZmG6JLAK6khmq8Sn8gBHTSjqhRzApFggAp0fkyEYKkosYUOPmGVA09O1t+xDURj1Np4kakLbu2ucATkD+lsb065q7AhVDbb3ilNbx3scXh0JAmVPKUyghY0kyV3cdtA/bVlup53vISGq4poqQNxb9reRswveoRxQmCq2khyZZwxdNxJgZf0FxbzBR1hoOWBfQt4vkC+EhKFG5JtC4XVktt1zMr3YGHiuIqpbcS2DcpgxHx/66VEEJItEQa4w1AVU4KCko+8hQaO9YdUX6njws+TK/T+9O6D3/i6UESKxMkX3b5qODG+eqdbhXrPFQfKIxOmi9mA4JiV0ejVyQGJPz5 MFNP9aZ/ dfxMWmtkTzQnEUr082/MwbyohBU77YHYAy9x94mIv/5YgFeqMiZuRAI6pCzIVwhPrDNfOiW2E07FIojtIdwlNwlbsg6PI5/Z1jT1Iz+cHopgQLga4HJa4YILUsFqAN7NLX/RuJmOZwod1/xkuZTIN8a71rFEGn02F04YdS8H2UmKubUCJn2eX1+mQDWz5w5u6ezfafsTQoNXK2uSWxkbSRt7WzSXCwNuBQAqX2aXNTpx43vianhMhQ1E1V/47oZEsi06fCmHB5GxgMeYOG9cgdxQHIjh3q91sKZm7EPpge6OkyFJR1wjLzgzkacul1CR3vNBcIyPsLkmFNSR+cqEhnFVwSeZgokHR26Lndpl4cogNhiqTbwYXHWEQCDiQTXo5UMl2 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: "Ho-Ren (Jack) Chuang" writes: > The current memory tier initialization process is distributed across > two different functions, memory_tier_init() and memory_tier_late_init(). > This design is hard to maintain. Thus, this patch is proposed to reduce > the possible code paths by consolidating different > initialization patches into one. > > The earlier discussion with Jonathan and Ying is listed here: > https://lore.kernel.org/lkml/20240405150244.00004b49@Huawei.com/ > > If we want to put these two initializations together, they must be > placed together in the later function. Because only at that time, > the HMAT information will be ready, adist between nodes can be > calculated, and memory tiering can be established based on the adist. > So we position the initialization at memory_tier_init() to the > memory_tier_late_init() call. Moreover, it's natural to keep > memory_tier initialization in drivers at device_initcall() level. > > If we simply move the set_node_memory_tier() from memory_tier_init() > to late_initcall(), it will result in HMAT not registering > the mt_adistance_algorithm callback function, because > set_node_memory_tier() is not performed during the memory tiering > initialization phase, leading to a lack of correct default_dram > information. > > Therefore, we introduced a nodemask to pass the information of the > default DRAM nodes. The reason for not choosing to reuse > default_dram_type->nodes is that it is not clean enough. So in the end, > we use a __initdata variable, which is a variable that is released once > initialization is complete, including both CPU and memory nodes for HMAT > to iterate through. > > This patchset is based on commits ("memory tier: create > CPUless memory tiers after obtaining HMAT info") and > ("memory tier: dax/kmem: introduce an abstract layer for > finding, allocating, and putting memory types"): > [0/2] https://lkml.kernel.org/r/20240405000707.2670063-1-horenchuang@bytedance.com > [1/2] https://lkml.kernel.org/r/20240405000707.2670063-2-horenchuang@bytedance.com > [1/2] https://lkml.kernel.org/r/20240405000707.2670063-3-horenchuang@bytedance.com > > Signed-off-by: Ho-Ren (Jack) Chuang > Suggested-by: Jonathan Cameron LGTM, Thanks! Reviewed-by: "Huang, Ying" > --- > - v3: > * Combine the cover letter and the patch. > * Remove extern default_dram_type > * Remove __initdata in a extern variable > * Roll back coding style, fix comment description > - v2: > Thanks to Huang, Ying's and Andrew's comments > * Add a cover letter > * Add Suggested-by: Jonathan Cameron > * Add get/put_online_mems() protection in memory_tier_late_init() > * If memtype is set, skip initializing its node > * Remove redundant code/comments or rewrite code in a cleaner manner > * https://lore.kernel.org/all/20240628060925.303309-1-horen.chuang@linux.dev/ > - v1: > * https://lore.kernel.org/all/20240621044833.3953055-1-horen.chuang@linux.dev/ > > - Open to discussion: > * Should we use `continue` or `break` if set_node_memory_tier() fails in > memory_tier_late_init()? I've changed it to `continue` in v3. > * Compute the default_dram_nodes mask in hmat when needed? Currently, > it still remains in memory-tiers.c > * Instead of using guard(mutex)(), use mutex_lock() as it was to make a > cleaner diff? > > Thanks, > Ho-Ren (Jack) Chuang > > drivers/acpi/numa/hmat.c | 5 +--- > include/linux/memory-tiers.h | 2 ++ > mm/memory-tiers.c | 54 ++++++++++++++---------------------- > 3 files changed, 24 insertions(+), 37 deletions(-) > > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > index 2c8ccc91ebe6..a2f9e7a4b479 100644 > --- a/drivers/acpi/numa/hmat.c > +++ b/drivers/acpi/numa/hmat.c > @@ -940,10 +940,7 @@ static int hmat_set_default_dram_perf(void) > struct memory_target *target; > struct access_coordinate *attrs; > > - if (!default_dram_type) > - return -EIO; > - > - for_each_node_mask(nid, default_dram_type->nodes) { > + for_each_node_mask(nid, default_dram_nodes) { > pxm = node_to_pxm(nid); > target = find_mem_target(pxm); > if (!target) > diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h > index 0d70788558f4..0dc0cf2863e2 100644 > --- a/include/linux/memory-tiers.h > +++ b/include/linux/memory-tiers.h > @@ -38,6 +38,7 @@ struct access_coordinate; > #ifdef CONFIG_NUMA > extern bool numa_demotion_enabled; > extern struct memory_dev_type *default_dram_type; > +extern nodemask_t default_dram_nodes; > struct memory_dev_type *alloc_memory_type(int adistance); > void put_memory_type(struct memory_dev_type *memtype); > void init_node_memory_type(int node, struct memory_dev_type *default_type); > @@ -76,6 +77,7 @@ static inline bool node_is_toptier(int node) > > #define numa_demotion_enabled false > #define default_dram_type NULL > +#define default_dram_nodes NODE_MASK_NONE > /* > * CONFIG_NUMA implementation returns non NULL error. > */ > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c > index 6632102bd5c9..4775b3a3dabe 100644 > --- a/mm/memory-tiers.c > +++ b/mm/memory-tiers.c > @@ -43,6 +43,7 @@ static LIST_HEAD(memory_tiers); > static LIST_HEAD(default_memory_types); > static struct node_memory_type_map node_memory_types[MAX_NUMNODES]; > struct memory_dev_type *default_dram_type; > +nodemask_t default_dram_nodes __initdata = NODE_MASK_NONE; > > static const struct bus_type memory_tier_subsys = { > .name = "memory_tiering", > @@ -671,28 +672,35 @@ EXPORT_SYMBOL_GPL(mt_put_memory_types); > > /* > * This is invoked via `late_initcall()` to initialize memory tiers for > - * CPU-less memory nodes after driver initialization, which is > - * expected to provide `adistance` algorithms. > + * memory nodes, both with and without CPUs. After the initialization of > + * firmware and devices, adistance algorithms are expected to be provided. > */ > static int __init memory_tier_late_init(void) > { > int nid; > + struct memory_tier *memtier; > > + get_online_mems(); > guard(mutex)(&memory_tier_lock); > + > + /* Assign each uninitialized N_MEMORY node to a memory tier. */ > for_each_node_state(nid, N_MEMORY) { > /* > - * Some device drivers may have initialized memory tiers > - * between `memory_tier_init()` and `memory_tier_late_init()`, > - * potentially bringing online memory nodes and > - * configuring memory tiers. Exclude them here. > + * Some device drivers may have initialized > + * memory tiers, potentially bringing memory nodes > + * online and configuring memory tiers. > + * Exclude them here. > */ > if (node_memory_types[nid].memtype) > continue; > > - set_node_memory_tier(nid); > + memtier = set_node_memory_tier(nid); > + if (IS_ERR(memtier)) > + continue; > } > > establish_demotion_targets(); > + put_online_mems(); > > return 0; > } > @@ -875,8 +883,7 @@ static int __meminit memtier_hotplug_callback(struct notifier_block *self, > > static int __init memory_tier_init(void) > { > - int ret, node; > - struct memory_tier *memtier; > + int ret; > > ret = subsys_virtual_register(&memory_tier_subsys, NULL); > if (ret) > @@ -887,7 +894,8 @@ static int __init memory_tier_init(void) > GFP_KERNEL); > WARN_ON(!node_demotion); > #endif > - mutex_lock(&memory_tier_lock); > + > + guard(mutex)(&memory_tier_lock); > /* > * For now we can have 4 faster memory tiers with smaller adistance > * than default DRAM tier. > @@ -897,29 +905,9 @@ static int __init memory_tier_init(void) > if (IS_ERR(default_dram_type)) > panic("%s() failed to allocate default DRAM tier\n", __func__); > > - /* > - * Look at all the existing N_MEMORY nodes and add them to > - * default memory tier or to a tier if we already have memory > - * types assigned. > - */ > - for_each_node_state(node, N_MEMORY) { > - if (!node_state(node, N_CPU)) > - /* > - * Defer memory tier initialization on > - * CPUless numa nodes. These will be initialized > - * after firmware and devices are initialized. > - */ > - continue; > - > - memtier = set_node_memory_tier(node); > - if (IS_ERR(memtier)) > - /* > - * Continue with memtiers we are able to setup > - */ > - break; > - } > - establish_demotion_targets(); > - mutex_unlock(&memory_tier_lock); > + /* Record nodes with memory and CPU to set default DRAM performance. */ > + nodes_and(default_dram_nodes, node_states[N_MEMORY], > + node_states[N_CPU]); > > hotplug_memory_notifier(memtier_hotplug_callback, MEMTIER_HOTPLUG_PRI); > return 0;