linux-kernel - Re: [PATCH v2 1/1] memory tier: consolidate the initialization of memory tiers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a5iykgdf.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Wed, 03 Jul 2024 16:51:40 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.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

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).

>>  
>>  	hotplug_memory_notifier(memtier_hotplug_callback, MEMTIER_HOTPLUG_PRI);
>>  	return 0;

--
Best Regards,
Huang, Ying