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 026ABC54798 for ; Tue, 5 Mar 2024 09:29:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 92D8C6B006E; Tue, 5 Mar 2024 04:29:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8DDAB6B0096; Tue, 5 Mar 2024 04:29:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7A53F6B009E; Tue, 5 Mar 2024 04:29:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 6C8F76B006E for ; Tue, 5 Mar 2024 04:29:10 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 46EF8A0D25 for ; Tue, 5 Mar 2024 09:29:10 +0000 (UTC) X-FDA: 81862461660.26.3FC5F26 Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) by imf16.hostedemail.com (Postfix) with ESMTP id 53335180013 for ; Tue, 5 Mar 2024 09:29:07 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=Ua8ITR23; spf=pass (imf16.hostedemail.com: domain of horenchuang@bytedance.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=horenchuang@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709630948; 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=79M1BRzVOMDaKqMxEGUh5q94dnwcGvlodjX5ei2a/R4=; b=ox6SexKQEq9Gak97mv6dRcvRyVoiZGJ/LgssrZqO71BGXkUYytqxknJRa91jdjhpl1pmTi os66L5Cuuu+SI0orOh00EQ+p+AodljCA0EoUBpk0zSsoanedLi01zGAuPIMPiyP+PEoFwM L9FJGMEo8STrfVuaerR8AYnGe7vCp2k= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709630948; a=rsa-sha256; cv=none; b=6456isJzHBOvJJqq0a9W+CBS98H0I2rMQhC6JdCsUikNB4mHoNqkSlaofZG2gxKqRxy6ir HrIbzrzSgXvN8dAe5NeyRSmG9Qi1wPrDWyV173+BW+aP0a9QFqfidh08zj1ClurAQRFLvI xO/LSsrPVobFR9Hx5HA7qLp29AbooyA= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=Ua8ITR23; spf=pass (imf16.hostedemail.com: domain of horenchuang@bytedance.com designates 209.85.222.44 as permitted sender) smtp.mailfrom=horenchuang@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com Received: by mail-ua1-f44.google.com with SMTP id a1e0cc1a2514c-7d5bbbe592dso3022959241.0 for ; Tue, 05 Mar 2024 01:29:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1709630946; x=1710235746; 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=79M1BRzVOMDaKqMxEGUh5q94dnwcGvlodjX5ei2a/R4=; b=Ua8ITR23zH6l7dvyniw21JeKsCYFwrFVjA5JdEFA2Pdp/1y/m2itVt6J5oLTfGQ/AA pADBHK5oCjymz4gJqZ3DlQOyZMRU1AVe/qex/m4oKkwLXlC1IredWI82dA6QG4GcXMcN BTCQfT4Z/zCzKXGymgsglWV8Rvo6VSPbr+atZmuJ/f3En3nM6LD//lNcYwIr2MCjTSMO B0soeh8fbEllnK/j4qYKyh5at8HHw3S040SA4BLLWhdOOBjGcDDtVYBOqcFojHsBh428 TDSsM67Fn14voUFh3u23VJr0CeLFINr9yTc1qkG2UeN9H3Ed2dzAAig4xdg7hCuVV4ft eaEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709630946; x=1710235746; 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=79M1BRzVOMDaKqMxEGUh5q94dnwcGvlodjX5ei2a/R4=; b=TkwvkWoxJI8Pv8/N5D64I8PgK0mJYbAgBega/s6RB3mHaovwv5qnuhHlBWKE5/qHei 3PA/t/Z7V6gZrp2qpMP1eOwYMJS9jnUqviXcezRzBUYDjT5lXhzvRhZrqwi3K+3vzn26 1y+lZaWDXw0CGdA8jhh0x+LCm0IG7Kkn/cGGUrCKwkdKNW2p59viLYv/JAjO8cUZhqkq kE494zwBj+W4MoAtDO0yZ/PhAs1lfDOOjNKG+UIDUY308soZTOXiRXPsKfqLyGkuYlkL A8q20l52nZYX1obciTu2xzuC5BLlgqkGFwesFd/lESoW2kf8xnyz61NGZRqavnRnJdui PUpw== X-Forwarded-Encrypted: i=1; AJvYcCWidX1ovT3fqxjql5ALnXUwDiHD9i4myFi7tHcKQ1hLdgbD145ZbHVyb43RZVNzHRW+dBml0pdam2Zr5RjadzH+Q0o= X-Gm-Message-State: AOJu0Yyk/582qwX9zsLjgPnm3lQyDYxrdtXYnuz7g5V6LGRNB2ApjAx+ CHbuwTKeOvtdzvCkYDIxRzeQDzHNsdlCPilLOKkYudVxUXGgqNvah9H195L0DSfkPsgsCkvEXH1 eSxB3GQS3MN6ZeEHPIntr9s9t/pAvqvmpZnyVTQ== X-Google-Smtp-Source: AGHT+IGeG8eiA+gbTAW4lP8+EW0Wh/ImGvq3CgHfBV04xBBfvRQuXg2vDyo36TnOBu+4pDZXWZHNMilzpagoOwujU5k= X-Received: by 2002:a05:6102:21ba:b0:472:c844:804d with SMTP id i26-20020a05610221ba00b00472c844804dmr1040181vsb.18.1709630946252; Tue, 05 Mar 2024 01:29:06 -0800 (PST) MIME-Version: 1.0 References: <20240301082248.3456086-1-horenchuang@bytedance.com> <20240301082248.3456086-2-horenchuang@bytedance.com> <87jzmibtyh.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87jzmibtyh.fsf@yhuang6-desk2.ccr.corp.intel.com> From: "Ho-Ren (Jack) Chuang" Date: Tue, 5 Mar 2024 01:28:55 -0800 Message-ID: Subject: Re: [External] Re: [PATCH v1 1/1] memory tier: acpi/hmat: create CPUless memory tiers after obtaining HMAT info To: "Huang, Ying" Cc: 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 , "Rafael J. Wysocki" , Len Brown , Andrew Morton , Dave Jiang , Dan Williams , Jonathan Cameron , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, "Ho-Ren (Jack) Chuang" , "Ho-Ren (Jack) Chuang" , linux-cxl@vger.kernel.org, qemu-devel@nongnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: w9c6tt5s54qbxchh8op58fjm8kfi1zwu X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 53335180013 X-Rspam-User: X-HE-Tag: 1709630947-522349 X-HE-Meta: U2FsdGVkX18m3aV/sd3q3ZHF/tnKoLqPMuaLIj5A1+uEVn6GA1a4wnCF/zOa11VPhjK5O1VMoJFsjOo0g8homAjudOfPuLVN2slPEwTfp62YmDndZkkiJ1HaGxjI+gZGZ62Y00YmZVrQnTM36Q7z/n1xUjpIDi32Gz1VH9YmemafxhsgjlgzpZvSnFGF+L2dXuG8JdnkI4ekQs8QtZ9GEIVFjFo5YqRCkoCGJRORo3eDE0z0lV5j/9hHeN0Z4iweFaiU3CyR5IUAlR99vxN4HjQYOaS5MrCwpvqrl1rOSd25zIXuorIkKvaW9mAzbCNQa1zdyR17OKXK5woJkhwEX3UsFFKqupDcxztdeEukREOeNpUpyAbwzR/PTxG3m3kJclEEJFbY3/y7f2en6jag9oh9Fo/o0m0GXLlII6iwQkRYbkk/8/phc1iR0I29ZAXrGcVeJTSm/LqJD7dNi6nt2do371xtI5AepEKLKWqvp+JQvshrY/WVxuso7QPL6nF8XtiNEI2gY1wjkaEzSI3yMrkVNSLpdtjSAJUYARGTQ76z8pD7MjGu0zOv0W9a932U7N8XuvsEGIYUKmsJkmnuAbvY+BSzsZEAX3UbBgA5+g+/XjMP5PuB9rh1oSd9dA27n0tENDQ8KmC9xk1i6lziAqcHpfL+pNmcloXTCJsJ7ykdvPPLJf5QLsagRdptqhAFewhw8Y44bJFE5w9IywNC2YBJrA+4NKVOs6vG7kfOjVmGZVddrlDhcM5HXx676rNTwp7fOzGwlQsvEo6BID8pq3X2L0cH+/RBYOEWpe3UvWu0/zrdZZeouV2qqoTgcJyIZKqt62oBITlN4EApHUAaZKaiRhhXHcpLcctlebj+3OpOWM0fH7IukdI+uHawP6HXcry3kAQeEG3zVv2jOJhMbjxhhk6Uhqw063cUT4FDQMUpxt/9WgQnEmEf3BacOoc4zDForxhZcxI7Pwkfymf aCs/TOaJ lhVsqeoZowvNJdke/BFQmyQS2HjTcSo8HkrbcMH9exz5Rr2wdmAlhlJg6VqnU7Pr0dwh80uetTuBh4B9ljfCGGtgKQRkczN0HHZGcTkL7yJMi4hMe5a/5KcPTdJ3pKeYcCb3tvfcN7V+hcU5o+T4xWR8VWzBAHK0GCgE2Ve9KlbnlDYyFVcdds4TKPmegctT7TQsLEIzogUH4NrYnIYTVXWFb0M5ewQjPXNfdo8Ah1K+4qwSsAxFX22zq8ljCh91fiRK+h3yNa06VVlSdGLHRs7kI5Ch04Yx+Mw8bRkhSINt48j/NI683cDucbAzEU7CNMTLyThjUm9nhMcEUQX+z+ALVRo3yv8n8pDBcgFsjkvO+mkxzIKhLw6B3RryTS92gdGy0nbFax89qt2s= 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 Sun, Mar 3, 2024 at 6:42=E2=80=AFPM Huang, Ying w= rote: > > Hi, Jack, > > "Ho-Ren (Jack) Chuang" writes: > > > * Introduce `mt_init_with_hmat()` > > We defer memory tier initialization for those CPUless NUMA nodes > > until acquiring HMAT info. `mt_init_with_hmat()` is introduced to > > post-create CPUless memory tiers after obtaining HMAT info. > > It iterates through each CPUless memory node, creating memory tiers if > > necessary. Finally, it calculates demotion tables again at the end. > > > > * Introduce `hmat_find_alloc_memory_type()` > > Find or allocate a memory type in the `hmat_memory_types` list. > > > > * Make `set_node_memory_tier()` more generic > > This function can also be used for setting other memory types for a nod= e. > > To do so, a new argument is added to specify a memory type. > > > > * Handle cases where there is no HMAT when creating memory tiers > > If no HMAT is specified, it falls back to using `default_dram_type`. > > > > * Change adist calculation code to use another new lock, mt_perf_lock. > > Iterating through CPUlist nodes requires holding the `memory_tier_lock`= . > > However, `mt_calc_adistance()` will end up trying to acquire the same l= ock, > > leading to a potential deadlock. Therefore, we propose introducing a > > standalone `mt_perf_lock` to protect `default_dram_perf`. This approach= not > > only avoids deadlock but also prevents holding a large lock simultaneou= sly. > > The patch description is used to described why we need the change, and > how we do that, but not what we do. People can tell what is done from > the code itself. > Got it. Thanks. Will rewrite it after the code is finalized. > > Signed-off-by: Ho-Ren (Jack) Chuang > > Signed-off-by: Hao Xiang > > --- > > drivers/acpi/numa/hmat.c | 3 ++ > > include/linux/memory-tiers.h | 6 +++ > > mm/memory-tiers.c | 76 ++++++++++++++++++++++++++++++++---- > > 3 files changed, 77 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c > > index d6b85f0f6082..9f57338b3cb5 100644 > > --- a/drivers/acpi/numa/hmat.c > > +++ b/drivers/acpi/numa/hmat.c > > @@ -1038,6 +1038,9 @@ static __init int hmat_init(void) > > if (!hmat_set_default_dram_perf()) > > register_mt_adistance_algorithm(&hmat_adist_nb); > > > > + /* Post-create CPUless memory tiers after getting HMAT info */ > > + mt_init_with_hmat(); > > + > > return 0; > > out_put: > > hmat_free_structures(); > > diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.= h > > index 69e781900082..2f845e90c033 100644 > > --- a/include/linux/memory-tiers.h > > +++ b/include/linux/memory-tiers.h > > @@ -48,6 +48,7 @@ int mt_calc_adistance(int node, int *adist); > > int mt_set_default_dram_perf(int nid, struct access_coordinate *perf, > > const char *source); > > int mt_perf_to_adistance(struct access_coordinate *perf, int *adist); > > +void mt_init_with_hmat(void); > > HMAT isn't universally available. It's a driver in fact. So, don't put > driver specific code in general code. > Please see below regarding "move code to hmat.c" > > #ifdef CONFIG_MIGRATION > > int next_demotion_node(int node); > > void node_get_allowed_targets(pg_data_t *pgdat, nodemask_t *targets); > > @@ -136,5 +137,10 @@ static inline int mt_perf_to_adistance(struct acce= ss_coordinate *perf, int *adis > > { > > return -EIO; > > } > > + > > +static inline void mt_init_with_hmat(void) > > +{ > > + > > +} > > #endif /* CONFIG_NUMA */ > > #endif /* _LINUX_MEMORY_TIERS_H */ > > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c > > index 0537664620e5..7a0a579b3deb 100644 > > --- a/mm/memory-tiers.c > > +++ b/mm/memory-tiers.c > > @@ -35,7 +35,9 @@ struct node_memory_type_map { > > }; > > > > static DEFINE_MUTEX(memory_tier_lock); > > +static DEFINE_MUTEX(mt_perf_lock); > > static LIST_HEAD(memory_tiers); > > +static LIST_HEAD(hmat_memory_types); > > static struct node_memory_type_map node_memory_types[MAX_NUMNODES]; > > struct memory_dev_type *default_dram_type; > > > > @@ -502,7 +504,7 @@ static inline void __init_node_memory_type(int node= , struct memory_dev_type *mem > > } > > } > > > > -static struct memory_tier *set_node_memory_tier(int node) > > +static struct memory_tier *set_node_memory_tier(int node, struct memor= y_dev_type *new_memtype) > > No. memory_dev_type are passed to the function via node_memory_types[node= ].memtype. > Got it. Will mimic the way kmem.c does. Thanks. > > { > > struct memory_tier *memtier; > > struct memory_dev_type *memtype; > > @@ -514,7 +516,7 @@ static struct memory_tier *set_node_memory_tier(int= node) > > if (!node_state(node, N_MEMORY)) > > return ERR_PTR(-EINVAL); > > > > - __init_node_memory_type(node, default_dram_type); > > + __init_node_memory_type(node, new_memtype); > > > > memtype =3D node_memory_types[node].memtype; > > node_set(node, memtype->nodes); > > @@ -623,6 +625,56 @@ void clear_node_memory_type(int node, struct memor= y_dev_type *memtype) > > } > > EXPORT_SYMBOL_GPL(clear_node_memory_type); > > > > +static struct memory_dev_type *hmat_find_alloc_memory_type(int adist) > > Similar function existed in drivers/dax/kmem.c. Please abstract them > and move them here. > Got it. Will try. Thanks. > > +{ > > + bool found =3D false; > > + struct memory_dev_type *mtype; > > + > > + list_for_each_entry(mtype, &hmat_memory_types, list) { > > + if (mtype->adistance =3D=3D adist) { > > + found =3D true; > > + break; > > + } > > + } > > + if (!found) { > > + mtype =3D alloc_memory_type(adist); > > + if (!IS_ERR(mtype)) > > + list_add(&mtype->list, &hmat_memory_types); > > + } > > + return mtype; > > +} > > + > > +static void mt_create_with_hmat(int node) > > +{ > > + struct memory_dev_type *mtype =3D NULL; > > + int adist =3D MEMTIER_ADISTANCE_DRAM; > > + > > + mt_calc_adistance(node, &adist); > > + if (adist !=3D MEMTIER_ADISTANCE_DRAM) { > > + mtype =3D hmat_find_alloc_memory_type(adist); > > + if (IS_ERR(mtype)) > > + pr_err("%s() failed to allocate a tier\n", __func= __); > > + } else { > > + mtype =3D default_dram_type; > > + } > > + > > + set_node_memory_tier(node, mtype); > > +} > > + > > +void mt_init_with_hmat(void) > > +{ > > + int nid; > > + > > + mutex_lock(&memory_tier_lock); > > + for_each_node_state(nid, N_MEMORY) > > + if (!node_state(nid, N_CPU)) > > + mt_create_with_hmat(nid); > > + > > + establish_demotion_targets(); > > + mutex_unlock(&memory_tier_lock); > > +} > > +EXPORT_SYMBOL_GPL(mt_init_with_hmat); > > + > > I guess that we can put most hmat related code above in hmat.c. > To put the heat-related code to hmat.c I will need to export some static functions in memory-tiers.c, like set_node_memory_tier() and establish_demotion_targets(). Is that ok? > > static void dump_hmem_attrs(struct access_coordinate *coord, const cha= r *prefix) > > { > > pr_info( > > @@ -636,7 +688,7 @@ int mt_set_default_dram_perf(int nid, struct access= _coordinate *perf, > > { > > int rc =3D 0; > > > > - mutex_lock(&memory_tier_lock); > > + mutex_lock(&mt_perf_lock); > > if (default_dram_perf_error) { > > rc =3D -EIO; > > goto out; > > @@ -684,7 +736,7 @@ int mt_set_default_dram_perf(int nid, struct access= _coordinate *perf, > > } > > > > out: > > - mutex_unlock(&memory_tier_lock); > > + mutex_unlock(&mt_perf_lock); > > return rc; > > } > > > > @@ -700,7 +752,7 @@ int mt_perf_to_adistance(struct access_coordinate *= perf, int *adist) > > perf->read_bandwidth + perf->write_bandwidth =3D=3D 0) > > return -EINVAL; > > > > - mutex_lock(&memory_tier_lock); > > + mutex_lock(&mt_perf_lock); > > /* > > * The abstract distance of a memory node is in direct proportion= to > > * its memory latency (read + write) and inversely proportional t= o its > > @@ -713,7 +765,7 @@ int mt_perf_to_adistance(struct access_coordinate *= perf, int *adist) > > (default_dram_perf.read_latency + default_dram_perf.write= _latency) * > > (default_dram_perf.read_bandwidth + default_dram_perf.wri= te_bandwidth) / > > (perf->read_bandwidth + perf->write_bandwidth); > > - mutex_unlock(&memory_tier_lock); > > + mutex_unlock(&mt_perf_lock); > > > > return 0; > > } > > @@ -797,7 +849,7 @@ static int __meminit memtier_hotplug_callback(struc= t notifier_block *self, > > break; > > case MEM_ONLINE: > > mutex_lock(&memory_tier_lock); > > - memtier =3D set_node_memory_tier(arg->status_change_nid); > > + memtier =3D set_node_memory_tier(arg->status_change_nid, = default_dram_type); > > if (!IS_ERR(memtier)) > > establish_demotion_targets(); > > mutex_unlock(&memory_tier_lock); > > @@ -836,7 +888,15 @@ static int __init memory_tier_init(void) > > * types assigned. > > */ > > for_each_node_state(node, N_MEMORY) { > > - memtier =3D set_node_memory_tier(node); > > + if (!node_state(node, N_CPU)) > > + /* > > + * Defer memory tier initialization on CPUless nu= ma nodes. > > + * These will be initialized when HMAT informatio= n is > > + * available. > > + */ > > + continue; > > + > > + memtier =3D set_node_memory_tier(node, default_dram_type)= ; > > On system with HMAT, how to fall back CPU-less node to > default_dram_type? I found your description, but I don't find it in code= . > I assume you meant without HMAT, if so, because if no HMAT, adist will not be updated in mt_calc_adistance(): + int adist =3D MEMTIER_ADISTANCE_DRAM; + + mt_calc_adistance(node, &adist); + if (adist !=3D MEMTIER_ADISTANCE_DRAM) { =E2=80=A6 + } else { + mtype =3D default_dram_type; + } + + set_node_memory_tier(node, mtype); > > if (IS_ERR(memtier)) > > /* > > * Continue with memtiers we are able to setup > > -- > Best Regards, > Huang, Ying --=20 Best regards, Ho-Ren (Jack) Chuang =E8=8E=8A=E8=B3=80=E4=BB=BB