Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> writes:
> On Fri, 28 Jun 2024 06:09:23 +0000
> "Ho-Ren (Jack) Chuang" <horen.chuang@xxxxxxxxx> 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).
>>
>> hotplug_memory_notifier(memtier_hotplug_callback, MEMTIER_HOTPLUG_PRI);
>> return 0;
--
Best Regards,
Huang, Ying
