Re: [RFC][PATCH 06/13] mm/migrate: update migration order during on hotplug events

[Dave Hansen <dave.hansen@intel.com>]

On 2/2/21 3:42 AM, Oscar Salvador wrote:
>> +static int __meminit migrate_on_reclaim_callback(struct notifier_block *self,
>> +						 unsigned long action, void *arg)
>> +{
>> +	switch (action) {
>> +	case MEM_GOING_OFFLINE:
>> +		/*
>> +		 * Make sure there are not transient states where
>> +		 * an offline node is a migration target.  This
>> +		 * will leave migration disabled until the offline
>> +		 * completes and the MEM_OFFLINE case below runs.
>> +		 */
>> +		disable_all_migrate_targets();
>> +		break;
>> +	case MEM_OFFLINE:
>> +	case MEM_ONLINE:
>> +		/*
>> +		 * Recalculate the target nodes once the node
>> +		 * reaches its final state (online or offline).
>> +		 */
>> +		__set_migration_target_nodes();
>> +		break;
>> +	case MEM_CANCEL_OFFLINE:
>> +		/*
>> +		 * MEM_GOING_OFFLINE disabled all the migration
>> +		 * targets.  Reenable them.
>> +		 */
>> +		__set_migration_target_nodes();
>> +		break;
>> +	case MEM_GOING_ONLINE:
>> +	case MEM_CANCEL_ONLINE:
>> +		break;
>> +	}
>> +
>> +	return notifier_from_errno(0);
>> +}
> This looks good, and I kinda like it.
> But in this case, all we care about is whether NUMA node does or does
> not have memory, so we have to remove/added into the demotion list.
> So, would make more sense to have a kinda helper in
> node_states_{set,clear}_node that calls the respective functions
> (disable_all_migrate_targets and __set_migration_target_nodes)?

Of, you're saying that we could do this in the hotplug code itself
instead of from a notifier?  I agree, we *could*.  That would be more
efficient.  But, I do like the idea of doing this from a notifier
because it's a bit less brittle.

Do you feel strongly about this one?