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?
