Re: [PATCH v10 5/8] mm/demotion: Build demotion targets based on explicit memory tiers

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
	Wei Xu <weixugc@google.com>, Yang Shi <shy828301@gmail.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Tim C Chen <tim.c.chen@intel.com>,
	Michal Hocko <mhocko@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Hesham Almatary <hesham.almatary@huawei.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Alistair Popple <apopple@nvidia.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	jvgediya.oss@gmail.com
Subject: Re: [PATCH v10 5/8] mm/demotion: Build demotion targets based on explicit memory tiers
Date: Wed, 03 Aug 2022 08:48:19 +0530	[thread overview]
Message-ID: <878ro6rp50.fsf@linux.ibm.com> (raw)
In-Reply-To: <87h733uyc8.fsf@yhuang6-desk2.ccr.corp.intel.com>

"Huang, Ying" <ying.huang@intel.com> writes:

> Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> writes:
>
>> On 7/26/22 1:14 PM, Huang, Ying wrote:
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

....

>>> +
>>>>  static void init_node_memory_tier(int node)
>>>>  {
>>>>  	int perf_level;
>>>> @@ -84,11 +285,19 @@ static void init_node_memory_tier(int node)
>>>>  	mutex_lock(&memory_tier_lock);
>>>>  
>>>>  	memtier = __node_get_memory_tier(node);
>>>> +	/*
>>>> +	 * if node is already part of the tier proceed with the
>>>> +	 * current tier value, because we might want to establish
>>>> +	 * new migration paths now. The node might be added to a tier
>>>> +	 * before it was made part of N_MEMORY, hence estabilish_migration_targets
>>>> +	 * will have skipped this node.
>>>> +	 */
>>>>  	if (!memtier) {
>>>>  		perf_level = node_devices[node]->perf_level;
>>>>  		memtier = find_create_memory_tier(perf_level);
>>>>  		node_set(node, memtier->nodelist);
>>>>  	}
>>>> +	establish_migration_targets();
>>> 
>>> Why combines memory tiers establishing with demotion targets building?
>>> I think that it's better to separate them.   For example, if we move a
>>> set of NUMA node from one memory tier to another memory tier, we only
>>> need to run establish_migration_targets() once after moving all nodes.
>>> 
>>
>> Yes agree. I am not sure I followed your comment here. 
>>
>> Demotion target rebuilding is a separate helper. Any update to memory tiers needs rebuilding
>> of demotion targets. Also any change in node mask of memory tier needs
>> demotion target rebuild. Can you clarify the code change you are suggesting here?
>
> I think we should call establish_migration_targets() in
> migrate_on_reclaim_callback() directly.  As the example I mentioned
> above, sometimes, we don't need to call establish_migration_targets()
> for each node changing.
>

We need to hold memory_tier_lock while updating node's memory tier and
rebuilding demotion targets. All of that is done in the same function
here. An update node memory tier that allow for updating multiple node's
memory tier together would do what you mentioned above under
memory_tier_lock ie, update all the nodes memory tier and then call
establish_migration_targets() once.

-aneesh