[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8735ezdlgg.fsf@nvdebian.thelocal>
Date: Mon, 18 Jul 2022 15:41:39 +1000
From: Alistair Popple <apopple@...dia.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>
Cc: linux-mm@...ck.org, akpm@...ux-foundation.org,
Wei Xu <weixugc@...gle.com>, Huang Ying <ying.huang@...el.com>,
Yang Shi <shy828301@...il.com>,
Davidlohr Bueso <dave@...olabs.net>,
Tim C Chen <tim.c.chen@...el.com>,
Michal Hocko <mhocko@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Hesham Almatary <hesham.almatary@...wei.com>,
Dave Hansen <dave.hansen@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Dan Williams <dan.j.williams@...el.com>,
Johannes Weiner <hannes@...xchg.org>, jvgediya.oss@...il.com
Subject: Re: [PATCH v9 5/8] mm/demotion: Build demotion targets based on
explicit memory tiers
"Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com> writes:
> Alistair Popple <apopple@...dia.com> writes:
> ....
>
>> + */
>>> +static void establish_migration_targets(void)
>>> +{
>>> + struct memory_tier *memtier;
>>> + struct demotion_nodes *nd;
>>> + int target = NUMA_NO_NODE, node;
>>> + int distance, best_distance;
>>> + nodemask_t used;
>>> +
>>> + if (!node_demotion || !IS_ENABLED(CONFIG_MIGRATION))
>>
>> Does it make sense to include the memory tiering/demotion code if
>> CONFIG_MIGRATION isn't enabled? From what I can tell none of the
>> information established here is used if CONFIG_MIGRATION isn't enabled,
>> so it would be better to remove the IS_ENABLED checks and not include
>> the code at all.
>
> We use the same function/codepath for updating top_tier details. We
> would want to get node_is_toptier() to work even with CONFIG_MIGRATION
> disabled?
Why though? As far as I can tell node_is_toptier() only makes a
difference if CONFIG_MIGRATION is enabled, so it could just return a
static value if CONFIG_MIGRATION isn't enabled.
- Alistair
>>
>>> + return;
>>> +
>>> + disable_all_migrate_targets();
>>> +
>>> + for_each_node_state(node, N_MEMORY) {
>>> + best_distance = -1;
>>> + nd = &node_demotion[node];
>>> +
>>> + memtier = __node_get_memory_tier(node);
>>> + if (!memtier || list_is_last(&memtier->list, &memory_tiers))
>>> + continue;
>>> + /*
>>> + * Get the next memtier to find the demotion node list.
>>> + */
>>> + memtier = list_next_entry(memtier, list);
>>> +
>>> + /*
>>> + * find_next_best_node, use 'used' nodemask as a skip list.
>>> + * Add all memory nodes except the selected memory tier
>>> + * nodelist to skip list so that we find the best node from the
>>> + * memtier nodelist.
>>> + */
>>> + nodes_andnot(used, node_states[N_MEMORY], memtier->nodelist);
>>> +
>>> + /*
>>> + * Find all the nodes in the memory tier node list of same best distance.
>>> + * add them to the preferred mask. We randomly select between nodes
>>> + * in the preferred mask when allocating pages during demotion.
>>> + */
>>> + do {
>>> + target = find_next_best_node(node, &used);
>>> + if (target == NUMA_NO_NODE)
>>> + break;
>>> +
>>> + distance = node_distance(node, target);
>>> + if (distance == best_distance || best_distance == -1) {
>>> + best_distance = distance;
>>> + node_set(target, nd->preferred);
>>> + } else {
>>> + break;
>>> + }
>>> + } while (1);
>>> + }
>>> +}
>>> +
>
> .....
>
> -aneesh
Powered by blists - more mailing lists