On 2/2/21 3:42 AM, Oscar Salvador wrote:

quoted

+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?