[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240704180951.00005ca6@Huawei.com>
Date: Thu, 4 Jul 2024 18:09:51 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: "Huang, Ying" <ying.huang@...el.com>
CC: "Ho-Ren (Jack) Chuang" <horen.chuang@...ux.dev>, Gregory Price
<gourry.memverge@...il.com>, <aneesh.kumar@...ux.ibm.com>, <mhocko@...e.com>,
<tj@...nel.org>, <john@...alactic.com>, Eishan Mirakhur
<emirakhur@...ron.com>, Vinicius Tavares Petrucci <vtavarespetr@...ron.com>,
Ravis OpenSrc <Ravis.OpenSrc@...ron.com>, Alistair Popple
<apopple@...dia.com>, Srinivasulu Thanneeru <sthanneeru@...ron.com>, SeongJae
Park <sj@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown
<lenb@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, Dave Jiang
<dave.jiang@...el.com>, Dan Williams <dan.j.williams@...el.com>,
<linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-mm@...ck.org>, "Ho-Ren (Jack) Chuang" <horenc@...edu>, "Ho-Ren (Jack)
Chuang" <horenchuang@...edance.com>, "Ho-Ren (Jack) Chuang"
<horenchuang@...il.com>, <linux-cxl@...r.kernel.org>, <qemu-devel@...gnu.org>
Subject: Re: [PATCH v2 1/1] memory tier: consolidate the initialization of
memory tiers
On Wed, 03 Jul 2024 16:51:40 +0800
"Huang, Ying" <ying.huang@...el.com> wrote:
> Jonathan Cameron <Jonathan.Cameron@...wei.com> writes:
>
> > On Fri, 28 Jun 2024 06:09:23 +0000
> > "Ho-Ren (Jack) Chuang" <horen.chuang@...ux.dev> wrote:
>
> [snip]
>
> >> @@ -875,8 +886,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 +897,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);
> >
> > If this was safe to do without the rest of the change (I think so)
> > then better to pull that out as a trivial precursor so less noise
> > in here.
> >
> >> /*
> >> * For now we can have 4 faster memory tiers with smaller adistance
> >> * than default DRAM tier.
> >> @@ -897,29 +908,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]);
> >
> > There are systems where (for various esoteric reasons, such as describing an
> > association with some other memory that isn't DRAM where the granularity
> > doesn't match) the CPU nodes contain no DRAM but rather it's one node away.
> > Handling that can be a job for another day though.
> >
> > Why does this need to be computed here? Why not do it in
> > hmat_set_default_dram_perf? Doesn't seem to be used anywhere else.
>
> IMO, which node is default dram node is a general concept instead of
> HMAT specific. So, I think that it's better to decide that in the
> general code (memory-tiers.c).
That makes sense given I'd imagine this will spread to other firmware
types in time.
Thanks,
Jonathan
>
> >>
> >> hotplug_memory_notifier(memtier_hotplug_callback, MEMTIER_HOTPLUG_PRI);
> >> return 0;
>
> --
> Best Regards,
> Huang, Ying
>
Powered by blists - more mailing lists