[PATCH v07 0/9] powerpc/hotplug: Update affinity for migrated CPUs

[PATCH v07 0/9] powerpc/hotplug: Update affinity for migrated CPUs

[Michael Bringmann]

The migration of LPARs across Power systems affects many attributes
including that of the associativity of CPUs.  The patches in this
set execute when a system is coming up fresh upon a migration target.
They are intended to,

* Recognize changes to the associativity of CPUs recorded in internal
  data structures when compared to the latest copies in the device tree.
* Generate calls to other code layers to reset the data structures
  related to associativity of the CPUs.
* Re-register the 'changed' entities into the target system.
  Re-registration of CPUs mostly entails acting as if they have been
  newly hot-added into the target system.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>

Michael Bringmann (9):
  hotplug/cpu: Conditionally acquire/release DRC index
  hotplug/cpu: Add operation queuing function
  hotplug/cpu: Provide CPU readd operation
  mobility/numa: Ensure numa update does not overlap
  numa: Disable/enable arch_update_cpu_topology
  pmt/numa: Disable arch_update_cpu_topology during CPU readd
  powerpc/rtas: Allow disabling rtas_event_scan
  hotplug/rtas: No rtas_event_scan during PMT update
  hotplug/pmt: Update topology after PMT
---
Changes in patch:
  -- Restructure and rearrange content of patches to co-locate
     similar or related modifications
  -- Rename pseries_update_drconf_cpu to pseries_update_processor
  -- Simplify code to update CPU nodes during mobility checks.
     Remove functions to generate extra HP_ELOG messages in favor
     of direct function calls to dlpar_cpu_readd_by_index.
  -- Revise code order in dlpar_cpu_readd_by_index() to present
     more appropriate error codes from underlying layers of the
     implementation.
  -- Add hotplug device lock around all property updates
  -- Add call to rebuild_sched_domains in case of changes
  -- Various code cleanups and compaction
  -- Rebase to 4.18-rc1 kernel
  -- Change operation to run CPU readd after end of migration store.
  -- Improve descriptive text
  -- Cleanup patch reference to outdated function
  -- Code cleanup a 'acquire_drc' check in dlpar_cpu_add.
  -- Code cleanup a 'release_drc' check in dlpar_cpu_remove.

[PATCH v07 1/9] hotplug/cpu: Conditionally acquire/release DRC index

[Michael Bringmann]

powerpc/cpu: Modify dlpar_cpu_add and dlpar_cpu_remove to allow the
skipping of DRC index acquire or release operations during the CPU
add or remove operations.  This is intended to support subsequent
changes to provide a 'CPU readd' operation.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in patch:
  -- Move new validity check added to pseries_smp_notifier
     to another patch
  -- Revise one of checks for 'acquire_drc' in dlpar_cpu_add.
  -- Revise one of checks for 'release_drc' in dlpar_cpu_remove.
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |   71 +++++++++++++++-----------
 1 file changed, 40 insertions(+), 31 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 6ef77ca..7ede3b0 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -432,7 +432,7 @@ static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
 	return found;
 }
 
-static ssize_t dlpar_cpu_add(u32 drc_index)
+static ssize_t dlpar_cpu_add(u32 drc_index, bool acquire_drc)
 {
 	struct device_node *dn, *parent;
 	int rc, saved_rc;
@@ -457,19 +457,22 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
 		return -EINVAL;
 	}
 
-	rc = dlpar_acquire_drc(drc_index);
-	if (rc) {
-		pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
-			rc, drc_index);
-		of_node_put(parent);
-		return -EINVAL;
+	if (acquire_drc) {
+		rc = dlpar_acquire_drc(drc_index);
+		if (rc) {
+			pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
+				rc, drc_index);
+			of_node_put(parent);
+			return -EINVAL;
+		}
 	}
 
 	dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
 	if (!dn) {
 		pr_warn("Failed call to configure-connector, drc index: %x\n",
 			drc_index);
-		dlpar_release_drc(drc_index);
+		if (acquire_drc)
+			dlpar_release_drc(drc_index);
 		of_node_put(parent);
 		return -EINVAL;
 	}
@@ -484,9 +487,11 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
 		pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
 			dn->name, rc, drc_index);
 
-		rc = dlpar_release_drc(drc_index);
-		if (!rc)
-			dlpar_free_cc_nodes(dn);
+		if (acquire_drc) {
+			rc = dlpar_release_drc(drc_index);
+			if (!rc)
+				dlpar_free_cc_nodes(dn);
+		}
 
 		return saved_rc;
 	}
@@ -498,7 +503,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
 			dn->name, rc, drc_index);
 
 		rc = dlpar_detach_node(dn);
-		if (!rc)
+		if (!rc && acquire_drc)
 			dlpar_release_drc(drc_index);
 
 		return saved_rc;
@@ -566,7 +571,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
 
 }
 
-static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
+static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index,
+				bool release_drc)
 {
 	int rc;
 
@@ -579,12 +585,14 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
 		return -EINVAL;
 	}
 
-	rc = dlpar_release_drc(drc_index);
-	if (rc) {
-		pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
-			drc_index, dn->name, rc);
-		dlpar_online_cpu(dn);
-		return rc;
+	if (release_drc) {
+		rc = dlpar_release_drc(drc_index);
+		if (rc) {
+			pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
+				drc_index, dn->name, rc);
+			dlpar_online_cpu(dn);
+			return rc;
+		}
 	}
 
 	rc = dlpar_detach_node(dn);
@@ -593,8 +601,9 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
 
 		pr_warn("Failed to detach CPU %s, rc: %d", dn->name, rc);
 
-		rc = dlpar_acquire_drc(drc_index);
-		if (!rc)
+		if (release_drc)
+			rc = dlpar_acquire_drc(drc_index);
+		if (!release_drc || !rc)
 			dlpar_online_cpu(dn);
 
 		return saved_rc;
@@ -622,7 +631,7 @@ static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
 	return dn;
 }
 
-static int dlpar_cpu_remove_by_index(u32 drc_index)
+static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
 {
 	struct device_node *dn;
 	int rc;
@@ -634,7 +643,7 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
 		return -ENODEV;
 	}
 
-	rc = dlpar_cpu_remove(dn, drc_index);
+	rc = dlpar_cpu_remove(dn, drc_index, release_drc);
 	of_node_put(dn);
 	return rc;
 }
@@ -699,7 +708,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
 	}
 
 	for (i = 0; i < cpus_to_remove; i++) {
-		rc = dlpar_cpu_remove_by_index(cpu_drcs[i]);
+		rc = dlpar_cpu_remove_by_index(cpu_drcs[i], true);
 		if (rc)
 			break;
 
@@ -710,7 +719,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
 		pr_warn("CPU hot-remove failed, adding back removed CPUs\n");
 
 		for (i = 0; i < cpus_removed; i++)
-			dlpar_cpu_add(cpu_drcs[i]);
+			dlpar_cpu_add(cpu_drcs[i], true);
 
 		rc = -EINVAL;
 	} else {
@@ -780,7 +789,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
 	}
 
 	for (i = 0; i < cpus_to_add; i++) {
-		rc = dlpar_cpu_add(cpu_drcs[i]);
+		rc = dlpar_cpu_add(cpu_drcs[i], true);
 		if (rc)
 			break;
 
@@ -791,7 +800,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
 		pr_warn("CPU hot-add failed, removing any added CPUs\n");
 
 		for (i = 0; i < cpus_added; i++)
-			dlpar_cpu_remove_by_index(cpu_drcs[i]);
+			dlpar_cpu_remove_by_index(cpu_drcs[i], true);
 
 		rc = -EINVAL;
 	} else {
@@ -817,7 +826,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
 		if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
 			rc = dlpar_cpu_remove_by_count(count);
 		else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
-			rc = dlpar_cpu_remove_by_index(drc_index);
+			rc = dlpar_cpu_remove_by_index(drc_index, true);
 		else
 			rc = -EINVAL;
 		break;
@@ -825,7 +834,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
 		if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
 			rc = dlpar_cpu_add_by_count(count);
 		else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
-			rc = dlpar_cpu_add(drc_index);
+			rc = dlpar_cpu_add(drc_index, true);
 		else
 			rc = -EINVAL;
 		break;
@@ -850,7 +859,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 	if (rc)
 		return -EINVAL;
 
-	rc = dlpar_cpu_add(drc_index);
+	rc = dlpar_cpu_add(drc_index, true);
 
 	return rc ? rc : count;
 }
@@ -871,7 +880,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 		return -EINVAL;
 	}
 
-	rc = dlpar_cpu_remove(dn, drc_index);
+	rc = dlpar_cpu_remove(dn, drc_index, true);
 	of_node_put(dn);
 
 	return rc ? rc : count;

[PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Bringmann]

migration/dlpar: This patch adds function dlpar_queue_action()
which will queued up information about a CPU/Memory 'readd'
operation according to resource type, action code, and DRC index.
At a subsequent point, the list of operations can be run/played
in series.  Examples of such oprations include 'readd' of CPU
and Memory blocks identified as having changed their associativity
during an LPAR migration event.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in patch:
  -- Correct drc_index before adding to pseries_hp_errorlog struct
  -- Correct text of notice
  -- Revise queuing model to save up all of the DLPAR actions for
     later execution.
  -- Restore list init statement missing from patch
  -- Move call to apply queued operations into 'mobility.c'
  -- Compress some code
  -- Rename some of queueing function APIs
  -- Revise implementation to push execution of queued operations
     to a workqueue task.
  -- Cleanup reference to outdated queuing operation.
---
 arch/powerpc/include/asm/rtas.h           |    2 +
 arch/powerpc/platforms/pseries/dlpar.c    |   61 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/mobility.c |    4 ++
 arch/powerpc/platforms/pseries/pseries.h  |    2 +
 4 files changed, 69 insertions(+)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 71e393c..4f601c7 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
 		struct { __be32 count, index; } ic;
 		char	drc_name[1];
 	} _drc_u;
+	struct list_head list;
 };
 
 #define PSERIES_HP_ELOG_RESOURCE_CPU	1
 #define PSERIES_HP_ELOG_RESOURCE_MEM	2
 #define PSERIES_HP_ELOG_RESOURCE_SLOT	3
 #define PSERIES_HP_ELOG_RESOURCE_PHB	4
+#define PSERIES_HP_ELOG_RESOURCE_PMT	5
 
 #define PSERIES_HP_ELOG_ACTION_ADD	1
 #define PSERIES_HP_ELOG_ACTION_REMOVE	2
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a0b20c0..7264b8e 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -25,6 +25,7 @@
 #include <asm/prom.h>
 #include <asm/machdep.h>
 #include <linux/uaccess.h>
+#include <linux/delay.h>
 #include <asm/rtas.h>
 
 static struct workqueue_struct *pseries_hp_wq;
@@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
 	return 0;
 }
 
+static int dlpar_pmt(struct pseries_hp_errorlog *work);
+
 static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
 {
 	int rc;
@@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
 	case PSERIES_HP_ELOG_RESOURCE_CPU:
 		rc = dlpar_cpu(hp_elog);
 		break;
+	case PSERIES_HP_ELOG_RESOURCE_PMT:
+		rc = dlpar_pmt(hp_elog);
+		break;
 	default:
 		pr_warn_ratelimited("Invalid resource (%d) specified\n",
 				    hp_elog->resource);
@@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
 	}
 }
 
+LIST_HEAD(dlpar_delayed_list);
+
+int dlpar_queue_action(int resource, int action, u32 drc_index)
+{
+	struct pseries_hp_errorlog *hp_errlog;
+
+	hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
+	if (!hp_errlog)
+		return -ENOMEM;
+
+	hp_errlog->resource = resource;
+	hp_errlog->action = action;
+	hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
+	hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
+
+	list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
+
+	return 0;
+}
+
+static int dlpar_pmt(struct pseries_hp_errorlog *work)
+{
+	struct list_head *pos, *q;
+
+	ssleep(15);
+
+	list_for_each_safe(pos, q, &dlpar_delayed_list) {
+		struct pseries_hp_errorlog *tmp;
+
+		tmp = list_entry(pos, struct pseries_hp_errorlog, list);
+		handle_dlpar_errorlog(tmp);
+
+		list_del(pos);
+		kfree(tmp);
+
+		ssleep(10);
+	}
+
+	return 0;
+}
+
+int dlpar_queued_actions_run(void)
+{
+	if (!list_empty(&dlpar_delayed_list)) {
+		struct pseries_hp_errorlog hp_errlog;
+
+		hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
+		hp_errlog.action = 0;
+		hp_errlog.id_type = 0;
+
+		queue_hotplug_event(&hp_errlog, 0, 0);
+	}
+	return 0;
+}
+
 static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
 {
 	char *arg;
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index f6364d9..d0d1cae 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
 		return rc;
 
 	post_mobility_fixup();
+
+	/* Apply any necessary changes identified during fixup */
+	dlpar_queued_actions_run();
+
 	return count;
 }
 
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 60db2ee..72ca996 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
 
 void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
 			 struct completion *hotplug_done, int *rc);
+int dlpar_queue_action(int resource, int action, u32 drc_index);
+int dlpar_queued_actions_run(void);
 #ifdef CONFIG_MEMORY_HOTPLUG
 int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
 #else

[PATCH v07 3/9] hotplug/cpu: Provide CPU readd operation

[Michael Bringmann]

powerpc/dlpar: Provide hotplug CPU 'readd by index' operation to
support LPAR Post Migration state updates.  When such changes are
invoked by the PowerPC 'mobility' code, they will be queued up so
that modifications to CPU properties will take place after the new
property value is written to the device-tree.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in patch:
  -- Add CPU validity check to pseries_smp_notifier
  -- Improve check on 'ibm,associativity' property
  -- Add check for cpu type to new update property entry
  -- Cleanup reference to outdated queuing function.
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |   58 ++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 7ede3b0..1906ee57 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -305,6 +305,36 @@ static int pseries_add_processor(struct device_node *np)
 	return err;
 }
 
+static int pseries_update_processor(struct of_reconfig_data *pr)
+{
+	int old_entries, new_entries, rc = 0;
+	__be32 *old_assoc, *new_assoc;
+
+	/* We only handle changes due to 'ibm,associativity' property
+	 */
+	old_assoc = pr->old_prop->value;
+	old_entries = be32_to_cpu(*old_assoc++);
+
+	new_assoc = pr->prop->value;
+	new_entries = be32_to_cpu(*new_assoc++);
+
+	if (old_entries == new_entries) {
+		int sz = old_entries * sizeof(int);
+
+		if (memcmp(old_assoc, new_assoc, sz))
+			rc = dlpar_queue_action(
+					PSERIES_HP_ELOG_RESOURCE_CPU,
+					PSERIES_HP_ELOG_ACTION_READD,
+					pr->dn->phandle);
+	} else {
+		rc = dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_CPU,
+					PSERIES_HP_ELOG_ACTION_READD,
+					pr->dn->phandle);
+	}
+
+	return rc;
+}
+
 /*
  * Update the present map for a cpu node which is going away, and set
  * the hard id in the paca(s) to -1 to be consistent with boot time
@@ -648,6 +678,26 @@ static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
 	return rc;
 }
 
+static int dlpar_cpu_readd_by_index(u32 drc_index)
+{
+	int rc = 0;
+
+	pr_info("Attempting to re-add CPU, drc index %x\n", drc_index);
+
+	rc = dlpar_cpu_remove_by_index(drc_index, false);
+	if (!rc)
+		rc = dlpar_cpu_add(drc_index, false);
+
+	if (rc)
+		pr_info("Failed to update cpu at drc_index %lx\n",
+				(unsigned long int)drc_index);
+	else
+		pr_info("CPU at drc_index %lx was updated\n",
+				(unsigned long int)drc_index);
+
+	return rc;
+}
+
 static int find_dlpar_cpus_to_remove(u32 *cpu_drcs, int cpus_to_remove)
 {
 	struct device_node *dn;
@@ -838,6 +888,9 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
 		else
 			rc = -EINVAL;
 		break;
+	case PSERIES_HP_ELOG_ACTION_READD:
+		rc = dlpar_cpu_readd_by_index(drc_index);
+		break;
 	default:
 		pr_err("Invalid action (%d) specified\n", hp_elog->action);
 		rc = -EINVAL;
@@ -901,6 +954,11 @@ static int pseries_smp_notifier(struct notifier_block *nb,
 	case OF_RECONFIG_DETACH_NODE:
 		pseries_remove_processor(rd->dn);
 		break;
+	case OF_RECONFIG_UPDATE_PROPERTY:
+		if (!strcmp(rd->dn->type, "cpu") &&
+		    !strcmp(rd->prop->name, "ibm,associativity"))
+			pseries_update_processor(rd);
+		break;
 	}
 	return notifier_from_errno(err);
 }

[PATCH v07 4/9] mobility/numa: Ensure numa update does not overlap

[Michael Bringmann]

mobility/numa: Ensure that numa_update_cpu_topology() can not be
entered multiple times concurrently.  It may be accessed through
many different paths / concurrent work functions, and the lock
ordering may be difficult to ensure otherwise.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index a789d57..b22e27a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1079,6 +1079,7 @@ struct topology_update_data {
 static int topology_timer_secs = 1;
 static int topology_inited;
 static int topology_update_needed;
+static struct mutex topology_update_lock;
 
 /*
  * Change polling interval for associativity changes.
@@ -1320,6 +1321,11 @@ int numa_update_cpu_topology(bool cpus_locked)
 	if (!updates)
 		return 0;
 
+	if (!mutex_trylock(&topology_update_lock)) {
+		kfree(updates);
+		return 0;
+	}
+
 	cpumask_clear(&updated_cpus);
 
 	for_each_cpu(cpu, &cpu_associativity_changes_mask) {
@@ -1424,6 +1430,7 @@ int numa_update_cpu_topology(bool cpus_locked)
 out:
 	kfree(updates);
 	topology_update_needed = 0;
+	mutex_unlock(&topology_update_lock);
 	return changed;
 }
 
@@ -1598,6 +1605,8 @@ static ssize_t topology_write(struct file *file, const char __user *buf,
 
 static int topology_update_init(void)
 {
+	mutex_init(&topology_update_lock);
+
 	/* Do not poll for changes if disabled at boot */
 	if (topology_updates_enabled)
 		start_topology_update();

[PATCH v07 5/9] numa: Disable/enable arch_update_cpu_topology

[Michael Bringmann]

numa: Provide mechanism to disable/enable operation of
arch_update_cpu_topology/numa_update_cpu_topology.  This is
a simple tool to eliminate some avenues for thread deadlock
observed during system execution.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/topology.h |   10 ++++++++++
 arch/powerpc/mm/numa.c              |   14 ++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 16b0778..d9ceba6 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -43,6 +43,8 @@ static inline int pcibus_to_node(struct pci_bus *bus)
 extern int sysfs_add_device_to_node(struct device *dev, int nid);
 extern void sysfs_remove_device_from_node(struct device *dev, int nid);
 extern int numa_update_cpu_topology(bool cpus_locked);
+extern void arch_update_cpu_topology_suspend(void);
+extern void arch_update_cpu_topology_resume(void);
 
 static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node)
 {
@@ -82,6 +84,14 @@ static inline int numa_update_cpu_topology(bool cpus_locked)
 	return 0;
 }
 
+static inline void arch_update_cpu_topology_suspend(void)
+{
+}
+
+static inline void arch_update_cpu_topology_resume(void)
+{
+}
+
 static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node) {}
 
 #endif /* CONFIG_NUMA */
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b22e27a..2352489 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1079,6 +1079,7 @@ struct topology_update_data {
 static int topology_timer_secs = 1;
 static int topology_inited;
 static int topology_update_needed;
+static int topology_update_enabled = 1;
 static struct mutex topology_update_lock;
 
 /*
@@ -1313,6 +1314,9 @@ int numa_update_cpu_topology(bool cpus_locked)
 		return 0;
 	}
 
+	if (!topology_update_enabled)
+		return 0;
+
 	weight = cpumask_weight(&cpu_associativity_changes_mask);
 	if (!weight)
 		return 0;
@@ -1439,6 +1443,16 @@ int arch_update_cpu_topology(void)
 	return numa_update_cpu_topology(true);
 }
 
+void arch_update_cpu_topology_suspend(void)
+{
+	topology_update_enabled = 0;
+}
+
+void arch_update_cpu_topology_resume(void)
+{
+	topology_update_enabled = 1;
+}
+
 static void topology_work_fn(struct work_struct *work)
 {
 	rebuild_sched_domains();

[PATCH v07 6/9] pmt/numa: Disable arch_update_cpu_topology during CPU readd

[Michael Bringmann]

pmt/numa: Disable arch_update_cpu_topology during post migration
CPU readd updates when evaluating device-tree changes after LPM
to avoid thread deadlocks trying to update node assignments.
System timing between all of the threads and timers restarted in
a migrated system overlapped frequently allowing tasks to start
acquiring resources (get_online_cpus) needed by rebuild_sched_domains.
Defer the operation of that function until after the CPU readd has
completed.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 1906ee57..df1791b 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -26,6 +26,7 @@
 #include <linux/sched.h>	/* for idle_task_exit */
 #include <linux/sched/hotplug.h>
 #include <linux/cpu.h>
+#include <linux/cpuset.h>
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <asm/prom.h>
@@ -684,9 +685,15 @@ static int dlpar_cpu_readd_by_index(u32 drc_index)
 
 	pr_info("Attempting to re-add CPU, drc index %x\n", drc_index);
 
+	arch_update_cpu_topology_suspend();
 	rc = dlpar_cpu_remove_by_index(drc_index, false);
-	if (!rc)
+	arch_update_cpu_topology_resume();
+
+	if (!rc) {
+		arch_update_cpu_topology_suspend();
 		rc = dlpar_cpu_add(drc_index, false);
+		arch_update_cpu_topology_resume();
+	}
 
 	if (rc)
 		pr_info("Failed to update cpu at drc_index %lx\n",

[PATCH v07 7/9] powerpc/rtas: Allow disabling rtas_event_scan

[Michael Bringmann]

powerpc/rtas: Provide mechanism by which the rtas_event_scan can
be disabled/re-enabled by other portions of the powerpc code.
Among other things, this simplifies the usage of locking mechanisms
for shared kernel resources.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/rtas.h |    4 ++++
 arch/powerpc/kernel/rtasd.c     |   14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 4f601c7..4ab605a 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -386,8 +386,12 @@ extern int early_init_dt_scan_rtas(unsigned long node,
 
 #ifdef CONFIG_PPC_RTAS_DAEMON
 extern void rtas_cancel_event_scan(void);
+extern void rtas_event_scan_disable(void);
+extern void rtas_event_scan_enable(void);
 #else
 static inline void rtas_cancel_event_scan(void) { }
+static inline void rtas_event_scan_disable(void) { }
+static inline void rtas_event_scan_enable(void) { }
 #endif
 
 /* Error types logged.  */
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 44d66c33d..af69e44 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -455,11 +455,25 @@ static void do_event_scan(void)
  */
 static unsigned long event_scan_delay = 1*HZ;
 static int first_pass = 1;
+static int res_enable = 1;
+
+void rtas_event_scan_disable(void)
+{
+	res_enable = 0;
+}
+
+void rtas_event_scan_enable(void)
+{
+	res_enable = 1;
+}
 
 static void rtas_event_scan(struct work_struct *w)
 {
 	unsigned int cpu;
 
+	if (!res_enable)
+		return;
+
 	do_event_scan();
 
 	get_online_cpus();

[PATCH v07 8/9] hotplug/rtas: No rtas_event_scan during PMT update

[Michael Bringmann]

hotplug/rtas: Disable rtas_event_scan during device-tree property
updates after migration to reduce conflicts with changes propagated
to other parts of the kernel configuration, such as CPUs or memory.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index df1791b..09633de 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -685,14 +685,18 @@ static int dlpar_cpu_readd_by_index(u32 drc_index)
 
 	pr_info("Attempting to re-add CPU, drc index %x\n", drc_index);
 
+	rtas_event_scan_disable();
 	arch_update_cpu_topology_suspend();
 	rc = dlpar_cpu_remove_by_index(drc_index, false);
 	arch_update_cpu_topology_resume();
+	rtas_event_scan_enable();
 
 	if (!rc) {
+		rtas_event_scan_disable();
 		arch_update_cpu_topology_suspend();
 		rc = dlpar_cpu_add(drc_index, false);
 		arch_update_cpu_topology_resume();
+		rtas_event_scan_enable();
 	}
 
 	if (rc)

[PATCH v07 9/9] hotplug/pmt: Update topology after PMT

[Michael Bringmann]

hotplug/pmt: Call rebuild_sched_domains after applying changes
to update CPU associativity i.e. 'readd' CPUs.  This is to
ensure that the deferred calls to arch_update_cpu_topology are
now reflected in the system data structures.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 7264b8e..ea3c08a 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -16,6 +16,7 @@
 #include <linux/notifier.h>
 #include <linux/spinlock.h>
 #include <linux/cpu.h>
+#include <linux/cpuset.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 
@@ -451,6 +452,9 @@ static int dlpar_pmt(struct pseries_hp_errorlog *work)
 		ssleep(10);
 	}
 
+	ssleep(5);
+	rebuild_sched_domains();
+
 	return 0;
 }
 

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[John Allen]

On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote:
>migration/dlpar: This patch adds function dlpar_queue_action()
>which will queued up information about a CPU/Memory 'readd'
>operation according to resource type, action code, and DRC index.
>At a subsequent point, the list of operations can be run/played
>in series.  Examples of such oprations include 'readd' of CPU
>and Memory blocks identified as having changed their associativity
>during an LPAR migration event.
>
>Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>---
>Changes in patch:
>  -- Correct drc_index before adding to pseries_hp_errorlog struct
>  -- Correct text of notice
>  -- Revise queuing model to save up all of the DLPAR actions for
>     later execution.
>  -- Restore list init statement missing from patch
>  -- Move call to apply queued operations into 'mobility.c'
>  -- Compress some code
>  -- Rename some of queueing function APIs
>  -- Revise implementation to push execution of queued operations
>     to a workqueue task.
>  -- Cleanup reference to outdated queuing operation.
>---
> arch/powerpc/include/asm/rtas.h           |    2 +
> arch/powerpc/platforms/pseries/dlpar.c    |   61 +++++++++++++++++++++++++++++
> arch/powerpc/platforms/pseries/mobility.c |    4 ++
> arch/powerpc/platforms/pseries/pseries.h  |    2 +
> 4 files changed, 69 insertions(+)
>
>diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>index 71e393c..4f601c7 100644
>--- a/arch/powerpc/include/asm/rtas.h
>+++ b/arch/powerpc/include/asm/rtas.h
>@@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
> 		struct { __be32 count, index; } ic;
> 		char	drc_name[1];
> 	} _drc_u;
>+	struct list_head list;
> };
>
> #define PSERIES_HP_ELOG_RESOURCE_CPU	1
> #define PSERIES_HP_ELOG_RESOURCE_MEM	2
> #define PSERIES_HP_ELOG_RESOURCE_SLOT	3
> #define PSERIES_HP_ELOG_RESOURCE_PHB	4
>+#define PSERIES_HP_ELOG_RESOURCE_PMT	5
>
> #define PSERIES_HP_ELOG_ACTION_ADD	1
> #define PSERIES_HP_ELOG_ACTION_REMOVE	2
>diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
>index a0b20c0..7264b8e 100644
>--- a/arch/powerpc/platforms/pseries/dlpar.c
>+++ b/arch/powerpc/platforms/pseries/dlpar.c
>@@ -25,6 +25,7 @@
> #include <asm/prom.h>
> #include <asm/machdep.h>
> #include <linux/uaccess.h>
>+#include <linux/delay.h>
> #include <asm/rtas.h>
>
> static struct workqueue_struct *pseries_hp_wq;
>@@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
> 	return 0;
> }
>
>+static int dlpar_pmt(struct pseries_hp_errorlog *work);
>+
> static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
> {
> 	int rc;
>@@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
> 	case PSERIES_HP_ELOG_RESOURCE_CPU:
> 		rc = dlpar_cpu(hp_elog);
> 		break;
>+	case PSERIES_HP_ELOG_RESOURCE_PMT:
>+		rc = dlpar_pmt(hp_elog);
>+		break;
> 	default:
> 		pr_warn_ratelimited("Invalid resource (%d) specified\n",
> 				    hp_elog->resource);
>@@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> 	}
> }
>
>+LIST_HEAD(dlpar_delayed_list);
>+
>+int dlpar_queue_action(int resource, int action, u32 drc_index)
>+{
>+	struct pseries_hp_errorlog *hp_errlog;
>+
>+	hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
>+	if (!hp_errlog)
>+		return -ENOMEM;
>+
>+	hp_errlog->resource = resource;
>+	hp_errlog->action = action;
>+	hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
>+	hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
>+
>+	list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
>+
>+	return 0;
>+}
>+
>+static int dlpar_pmt(struct pseries_hp_errorlog *work)
>+{
>+	struct list_head *pos, *q;
>+
>+	ssleep(15);

Why do we need to sleep for so long here?

-John

>+
>+	list_for_each_safe(pos, q, &dlpar_delayed_list) {
>+		struct pseries_hp_errorlog *tmp;
>+
>+		tmp = list_entry(pos, struct pseries_hp_errorlog, list);
>+		handle_dlpar_errorlog(tmp);
>+
>+		list_del(pos);
>+		kfree(tmp);
>+
>+		ssleep(10);
>+	}
>+
>+	return 0;
>+}
>+
>+int dlpar_queued_actions_run(void)
>+{
>+	if (!list_empty(&dlpar_delayed_list)) {
>+		struct pseries_hp_errorlog hp_errlog;
>+
>+		hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
>+		hp_errlog.action = 0;
>+		hp_errlog.id_type = 0;
>+
>+		queue_hotplug_event(&hp_errlog, 0, 0);
>+	}
>+	return 0;
>+}
>+
> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
> {
> 	char *arg;
>diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>index f6364d9..d0d1cae 100644
>--- a/arch/powerpc/platforms/pseries/mobility.c
>+++ b/arch/powerpc/platforms/pseries/mobility.c
>@@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
> 		return rc;
>
> 	post_mobility_fixup();
>+
>+	/* Apply any necessary changes identified during fixup */
>+	dlpar_queued_actions_run();
>+
> 	return count;
> }
>
>diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>index 60db2ee..72ca996 100644
>--- a/arch/powerpc/platforms/pseries/pseries.h
>+++ b/arch/powerpc/platforms/pseries/pseries.h
>@@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
>
> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> 			 struct completion *hotplug_done, int *rc);
>+int dlpar_queue_action(int resource, int action, u32 drc_index);
>+int dlpar_queued_actions_run(void);
> #ifdef CONFIG_MEMORY_HOTPLUG
> int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
> #else
>

Re: [PATCH v07 1/9] hotplug/cpu: Conditionally acquire/release DRC index

[Nathan Fontenot]

On 07/13/2018 03:17 PM, Michael Bringmann wrote:
> powerpc/cpu: Modify dlpar_cpu_add and dlpar_cpu_remove to allow the
> skipping of DRC index acquire or release operations during the CPU
> add or remove operations.  This is intended to support subsequent
> changes to provide a 'CPU readd' operation.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in patch:
>    -- Move new validity check added to pseries_smp_notifier
>       to another patch
>    -- Revise one of checks for 'acquire_drc' in dlpar_cpu_add.
>    -- Revise one of checks for 'release_drc' in dlpar_cpu_remove.
> ---
>   arch/powerpc/platforms/pseries/hotplug-cpu.c |   71 +++++++++++++++-----------
>   1 file changed, 40 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 6ef77ca..7ede3b0 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -432,7 +432,7 @@ static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
>   	return found;
>   }
> 
> -static ssize_t dlpar_cpu_add(u32 drc_index)
> +static ssize_t dlpar_cpu_add(u32 drc_index, bool acquire_drc)
>   {
>   	struct device_node *dn, *parent;
>   	int rc, saved_rc;
> @@ -457,19 +457,22 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>   		return -EINVAL;
>   	}
> 
> -	rc = dlpar_acquire_drc(drc_index);
> -	if (rc) {
> -		pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
> -			rc, drc_index);
> -		of_node_put(parent);
> -		return -EINVAL;
> +	if (acquire_drc) {
> +		rc = dlpar_acquire_drc(drc_index);
> +		if (rc) {
> +			pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
> +				rc, drc_index);
> +			of_node_put(parent);
> +			return -EINVAL;
> +		}
>   	}
> 
>   	dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
>   	if (!dn) {
>   		pr_warn("Failed call to configure-connector, drc index: %x\n",
>   			drc_index);
> -		dlpar_release_drc(drc_index);
> +		if (acquire_drc)
> +			dlpar_release_drc(drc_index);
>   		of_node_put(parent);
>   		return -EINVAL;
>   	}
> @@ -484,9 +487,11 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>   		pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
>   			dn->name, rc, drc_index);
> 
> -		rc = dlpar_release_drc(drc_index);
> -		if (!rc)
> -			dlpar_free_cc_nodes(dn);
> +		if (acquire_drc) {
> +			rc = dlpar_release_drc(drc_index);
> +			if (!rc)
> +				dlpar_free_cc_nodes(dn);
> +		}
> 
>   		return saved_rc;
>   	}
> @@ -498,7 +503,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>   			dn->name, rc, drc_index);
> 
>   		rc = dlpar_detach_node(dn);
> -		if (!rc)
> +		if (!rc && acquire_drc)
>   			dlpar_release_drc(drc_index);
> 
>   		return saved_rc;
> @@ -566,7 +571,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
> 
>   }
> 
> -static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
> +static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index,
> +				bool release_drc)
>   {
>   	int rc;
> 
> @@ -579,12 +585,14 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
>   		return -EINVAL;
>   	}
> 
> -	rc = dlpar_release_drc(drc_index);
> -	if (rc) {
> -		pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
> -			drc_index, dn->name, rc);
> -		dlpar_online_cpu(dn);
> -		return rc;
> +	if (release_drc) {
> +		rc = dlpar_release_drc(drc_index);
> +		if (rc) {
> +			pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
> +				drc_index, dn->name, rc);
> +			dlpar_online_cpu(dn);
> +			return rc;
> +		}
>   	}
> 
>   	rc = dlpar_detach_node(dn);
> @@ -593,8 +601,9 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
> 
>   		pr_warn("Failed to detach CPU %s, rc: %d", dn->name, rc);
> 
> -		rc = dlpar_acquire_drc(drc_index);
> -		if (!rc)
> +		if (release_drc)
> +			rc = dlpar_acquire_drc(drc_index);
> +		if (!release_drc || !rc)
>   			dlpar_online_cpu(dn);

This is likely wrong. At this point you're in a if (rc) so rc is already
non-zero. If release_drc is false this checks an invalid rc state.

-Nathan

> 
>   		return saved_rc;
> @@ -622,7 +631,7 @@ static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
>   	return dn;
>   }
> 
> -static int dlpar_cpu_remove_by_index(u32 drc_index)
> +static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
>   {
>   	struct device_node *dn;
>   	int rc;
> @@ -634,7 +643,7 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>   		return -ENODEV;
>   	}
> 
> -	rc = dlpar_cpu_remove(dn, drc_index);
> +	rc = dlpar_cpu_remove(dn, drc_index, release_drc);
>   	of_node_put(dn);
>   	return rc;
>   }
> @@ -699,7 +708,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
>   	}
> 
>   	for (i = 0; i < cpus_to_remove; i++) {
> -		rc = dlpar_cpu_remove_by_index(cpu_drcs[i]);
> +		rc = dlpar_cpu_remove_by_index(cpu_drcs[i], true);
>   		if (rc)
>   			break;
> 
> @@ -710,7 +719,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
>   		pr_warn("CPU hot-remove failed, adding back removed CPUs\n");
> 
>   		for (i = 0; i < cpus_removed; i++)
> -			dlpar_cpu_add(cpu_drcs[i]);
> +			dlpar_cpu_add(cpu_drcs[i], true);
> 
>   		rc = -EINVAL;
>   	} else {
> @@ -780,7 +789,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>   	}
> 
>   	for (i = 0; i < cpus_to_add; i++) {
> -		rc = dlpar_cpu_add(cpu_drcs[i]);
> +		rc = dlpar_cpu_add(cpu_drcs[i], true);
>   		if (rc)
>   			break;
> 
> @@ -791,7 +800,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>   		pr_warn("CPU hot-add failed, removing any added CPUs\n");
> 
>   		for (i = 0; i < cpus_added; i++)
> -			dlpar_cpu_remove_by_index(cpu_drcs[i]);
> +			dlpar_cpu_remove_by_index(cpu_drcs[i], true);
> 
>   		rc = -EINVAL;
>   	} else {
> @@ -817,7 +826,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
>   		if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
>   			rc = dlpar_cpu_remove_by_count(count);
>   		else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
> -			rc = dlpar_cpu_remove_by_index(drc_index);
> +			rc = dlpar_cpu_remove_by_index(drc_index, true);
>   		else
>   			rc = -EINVAL;
>   		break;
> @@ -825,7 +834,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
>   		if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
>   			rc = dlpar_cpu_add_by_count(count);
>   		else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
> -			rc = dlpar_cpu_add(drc_index);
> +			rc = dlpar_cpu_add(drc_index, true);
>   		else
>   			rc = -EINVAL;
>   		break;
> @@ -850,7 +859,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>   	if (rc)
>   		return -EINVAL;
> 
> -	rc = dlpar_cpu_add(drc_index);
> +	rc = dlpar_cpu_add(drc_index, true);
> 
>   	return rc ? rc : count;
>   }
> @@ -871,7 +880,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>   		return -EINVAL;
>   	}
> 
> -	rc = dlpar_cpu_remove(dn, drc_index);
> +	rc = dlpar_cpu_remove(dn, drc_index, true);
>   	of_node_put(dn);
> 
>   	return rc ? rc : count;
> 

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Nathan Fontenot]

On 07/13/2018 03:18 PM, Michael Bringmann wrote:
> migration/dlpar: This patch adds function dlpar_queue_action()
> which will queued up information about a CPU/Memory 'readd'
> operation according to resource type, action code, and DRC index.
> At a subsequent point, the list of operations can be run/played
> in series.  Examples of such oprations include 'readd' of CPU
> and Memory blocks identified as having changed their associativity
> during an LPAR migration event. >
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in patch:
>    -- Correct drc_index before adding to pseries_hp_errorlog struct
>    -- Correct text of notice
>    -- Revise queuing model to save up all of the DLPAR actions for
>       later execution.
>    -- Restore list init statement missing from patch
>    -- Move call to apply queued operations into 'mobility.c'
>    -- Compress some code
>    -- Rename some of queueing function APIs
>    -- Revise implementation to push execution of queued operations
>       to a workqueue task.
>    -- Cleanup reference to outdated queuing operation.
> ---
>   arch/powerpc/include/asm/rtas.h           |    2 +
>   arch/powerpc/platforms/pseries/dlpar.c    |   61 +++++++++++++++++++++++++++++
>   arch/powerpc/platforms/pseries/mobility.c |    4 ++
>   arch/powerpc/platforms/pseries/pseries.h  |    2 +
>   4 files changed, 69 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 71e393c..4f601c7 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
>   		struct { __be32 count, index; } ic;
>   		char	drc_name[1];
>   	} _drc_u;
> +	struct list_head list;
>   };
> 
>   #define PSERIES_HP_ELOG_RESOURCE_CPU	1
>   #define PSERIES_HP_ELOG_RESOURCE_MEM	2
>   #define PSERIES_HP_ELOG_RESOURCE_SLOT	3
>   #define PSERIES_HP_ELOG_RESOURCE_PHB	4
> +#define PSERIES_HP_ELOG_RESOURCE_PMT	5
> 
>   #define PSERIES_HP_ELOG_ACTION_ADD	1
>   #define PSERIES_HP_ELOG_ACTION_REMOVE	2
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a0b20c0..7264b8e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -25,6 +25,7 @@
>   #include <asm/prom.h>
>   #include <asm/machdep.h>
>   #include <linux/uaccess.h>
> +#include <linux/delay.h>
>   #include <asm/rtas.h>
> 
>   static struct workqueue_struct *pseries_hp_wq;
> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
>   	return 0;
>   }
> 
> +static int dlpar_pmt(struct pseries_hp_errorlog *work);
> +
>   static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>   {
>   	int rc;
> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>   	case PSERIES_HP_ELOG_RESOURCE_CPU:
>   		rc = dlpar_cpu(hp_elog);
>   		break;
> +	case PSERIES_HP_ELOG_RESOURCE_PMT:
> +		rc = dlpar_pmt(hp_elog);
> +		break;
>   	default:
>   		pr_warn_ratelimited("Invalid resource (%d) specified\n",
>   				    hp_elog->resource);
> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>   	}
>   }
> 
> +LIST_HEAD(dlpar_delayed_list);
> +
> +int dlpar_queue_action(int resource, int action, u32 drc_index)
> +{
> +	struct pseries_hp_errorlog *hp_errlog;
> +
> +	hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
> +	if (!hp_errlog)
> +		return -ENOMEM;
> +
> +	hp_errlog->resource = resource;
> +	hp_errlog->action = action;
> +	hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
> +	hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
> +
> +	list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
> +
> +	return 0;
> +}
> +
> +static int dlpar_pmt(struct pseries_hp_errorlog *work)
> +{
> +	struct list_head *pos, *q;
> +
> +	ssleep(15);
> +
> +	list_for_each_safe(pos, q, &dlpar_delayed_list) {
> +		struct pseries_hp_errorlog *tmp;
> +
> +		tmp = list_entry(pos, struct pseries_hp_errorlog, list);
> +		handle_dlpar_errorlog(tmp);
> +
> +		list_del(pos);
> +		kfree(tmp);
> +
> +		ssleep(10);
> +	}
> +
> +	return 0;
> +}
> +
> +int dlpar_queued_actions_run(void)
> +{
> +	if (!list_empty(&dlpar_delayed_list)) {
> +		struct pseries_hp_errorlog hp_errlog;
> +
> +		hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
> +		hp_errlog.action = 0;
> +		hp_errlog.id_type = 0;
> +
> +		queue_hotplug_event(&hp_errlog, 0, 0); > +	}
> +	return 0;
> +}

I'm a bit confused by this. Is there a reason this needs to queue a
hotplug event instead of just walking the list as is done in dlpar_pmt?

-Nathan

> +
>   static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
>   {
>   	char *arg;
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index f6364d9..d0d1cae 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
>   		return rc;
> 
>   	post_mobility_fixup();
> +
> +	/* Apply any necessary changes identified during fixup */
> +	dlpar_queued_actions_run();
> +
>   	return count;
>   }
> 
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..72ca996 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
> 
>   void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>   			 struct completion *hotplug_done, int *rc);
> +int dlpar_queue_action(int resource, int action, u32 drc_index);
> +int dlpar_queued_actions_run(void);
>   #ifdef CONFIG_MEMORY_HOTPLUG
>   int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>   #else
> 

Re: [PATCH v07 6/9] pmt/numa: Disable arch_update_cpu_topology during CPU readd

[Nathan Fontenot]

On 07/13/2018 03:18 PM, Michael Bringmann wrote:
> pmt/numa: Disable arch_update_cpu_topology during post migration
> CPU readd updates when evaluating device-tree changes after LPM
> to avoid thread deadlocks trying to update node assignments.
> System timing between all of the threads and timers restarted in
> a migrated system overlapped frequently allowing tasks to start
> acquiring resources (get_online_cpus) needed by rebuild_sched_domains.
> Defer the operation of that function until after the CPU readd has
> completed.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>   arch/powerpc/platforms/pseries/hotplug-cpu.c |    9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 1906ee57..df1791b 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -26,6 +26,7 @@
>   #include <linux/sched.h>	/* for idle_task_exit */
>   #include <linux/sched/hotplug.h>
>   #include <linux/cpu.h>
> +#include <linux/cpuset.h>
>   #include <linux/of.h>
>   #include <linux/slab.h>
>   #include <asm/prom.h>
> @@ -684,9 +685,15 @@ static int dlpar_cpu_readd_by_index(u32 drc_index)
> 
>   	pr_info("Attempting to re-add CPU, drc index %x\n", drc_index);
> 
> +	arch_update_cpu_topology_suspend();
>   	rc = dlpar_cpu_remove_by_index(drc_index, false);
> -	if (!rc)
> +	arch_update_cpu_topology_resume();
> +
> +	if (!rc) {
> +		arch_update_cpu_topology_suspend();
>   		rc = dlpar_cpu_add(drc_index, false);
> +		arch_update_cpu_topology_resume();
> +	}
> 

A couple of questions...Why not disable across the entire remove and add
operations instead of disabling for each operation?

Also, what about other CPU add/remove routines, do they need to do
similar disabling?

-Nathan

>   	if (rc)
>   		pr_info("Failed to update cpu at drc_index %lx\n",
> 

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Bringmann]

See below.

On 07/23/2018 10:54 AM, John Allen wrote:
> On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote:
>> migration/dlpar: This patch adds function dlpar_queue_action()
>> which will queued up information about a CPU/Memory 'readd'
>> operation according to resource type, action code, and DRC index.
>> At a subsequent point, the list of operations can be run/played
>> in series.  Examples of such oprations include 'readd' of CPU
>> and Memory blocks identified as having changed their associativity
>> during an LPAR migration event.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in patch:
>>  -- Correct drc_index before adding to pseries_hp_errorlog struct
>>  -- Correct text of notice
>>  -- Revise queuing model to save up all of the DLPAR actions for
>>     later execution.
>>  -- Restore list init statement missing from patch
>>  -- Move call to apply queued operations into 'mobility.c'
>>  -- Compress some code
>>  -- Rename some of queueing function APIs
>>  -- Revise implementation to push execution of queued operations
>>     to a workqueue task.
>>  -- Cleanup reference to outdated queuing operation.
>> ---
>> arch/powerpc/include/asm/rtas.h           |    2 +
>> arch/powerpc/platforms/pseries/dlpar.c    |   61 +++++++++++++++++++++++++++++
>> arch/powerpc/platforms/pseries/mobility.c |    4 ++
>> arch/powerpc/platforms/pseries/pseries.h  |    2 +
>> 4 files changed, 69 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>> index 71e393c..4f601c7 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
>>         struct { __be32 count, index; } ic;
>>         char    drc_name[1];
>>     } _drc_u;
>> +    struct list_head list;
>> };
>>
>> #define PSERIES_HP_ELOG_RESOURCE_CPU    1
>> #define PSERIES_HP_ELOG_RESOURCE_MEM    2
>> #define PSERIES_HP_ELOG_RESOURCE_SLOT    3
>> #define PSERIES_HP_ELOG_RESOURCE_PHB    4
>> +#define PSERIES_HP_ELOG_RESOURCE_PMT    5
>>
>> #define PSERIES_HP_ELOG_ACTION_ADD    1
>> #define PSERIES_HP_ELOG_ACTION_REMOVE    2
>> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
>> index a0b20c0..7264b8e 100644
>> --- a/arch/powerpc/platforms/pseries/dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/dlpar.c
>> @@ -25,6 +25,7 @@
>> #include <asm/prom.h>
>> #include <asm/machdep.h>
>> #include <linux/uaccess.h>
>> +#include <linux/delay.h>
>> #include <asm/rtas.h>
>>
>> static struct workqueue_struct *pseries_hp_wq;
>> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
>>     return 0;
>> }
>>
>> +static int dlpar_pmt(struct pseries_hp_errorlog *work);
>> +
>> static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>> {
>>     int rc;
>> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>>     case PSERIES_HP_ELOG_RESOURCE_CPU:
>>         rc = dlpar_cpu(hp_elog);
>>         break;
>> +    case PSERIES_HP_ELOG_RESOURCE_PMT:
>> +        rc = dlpar_pmt(hp_elog);
>> +        break;
>>     default:
>>         pr_warn_ratelimited("Invalid resource (%d) specified\n",
>>                     hp_elog->resource);
>> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>>     }
>> }
>>
>> +LIST_HEAD(dlpar_delayed_list);
>> +
>> +int dlpar_queue_action(int resource, int action, u32 drc_index)
>> +{
>> +    struct pseries_hp_errorlog *hp_errlog;
>> +
>> +    hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
>> +    if (!hp_errlog)
>> +        return -ENOMEM;
>> +
>> +    hp_errlog->resource = resource;
>> +    hp_errlog->action = action;
>> +    hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
>> +    hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
>> +
>> +    list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
>> +
>> +    return 0;
>> +}
>> +
>> +static int dlpar_pmt(struct pseries_hp_errorlog *work)
>> +{
>> +    struct list_head *pos, *q;
>> +
>> +    ssleep(15);
> 
> Why do we need to sleep for so long here?

One or more drivers were finishing their initialization about the same
time as the 'migration_store' operation was completing.  E.g. 'ibmvscsi'
When we did 'dlpar cpu readd' operations at the same time early on in
testing, the errors / warnings were inconsistent.  I put in a delay to
get beyond the driver re-init after migration.  15 seconds was an estimate.
As it occurs after the completion of the 'migration store' operation,
it does not delay any responses returned to the HMC.

We can rerun tests with reduced / removed delays to see if they are still
necessary given some other corrections in the mix.

> 
> -John

Michael

>> +
>> +    list_for_each_safe(pos, q, &dlpar_delayed_list) {
>> +        struct pseries_hp_errorlog *tmp;
>> +
>> +        tmp = list_entry(pos, struct pseries_hp_errorlog, list);
>> +        handle_dlpar_errorlog(tmp);
>> +
>> +        list_del(pos);
>> +        kfree(tmp);
>> +
>> +        ssleep(10);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int dlpar_queued_actions_run(void)
>> +{
>> +    if (!list_empty(&dlpar_delayed_list)) {
>> +        struct pseries_hp_errorlog hp_errlog;
>> +
>> +        hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
>> +        hp_errlog.action = 0;
>> +        hp_errlog.id_type = 0;
>> +
>> +        queue_hotplug_event(&hp_errlog, 0, 0);
>> +    }
>> +    return 0;
>> +}
>> +
>> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
>> {
>>     char *arg;
>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>> index f6364d9..d0d1cae 100644
>> --- a/arch/powerpc/platforms/pseries/mobility.c
>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
>>         return rc;
>>
>>     post_mobility_fixup();
>> +
>> +    /* Apply any necessary changes identified during fixup */
>> +    dlpar_queued_actions_run();
>> +
>>     return count;
>> }
>>
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 60db2ee..72ca996 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
>>
>> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>>              struct completion *hotplug_done, int *rc);
>> +int dlpar_queue_action(int resource, int action, u32 drc_index);
>> +int dlpar_queued_actions_run(void);
>> #ifdef CONFIG_MEMORY_HOTPLUG
>> int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>> #else
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Bringmann]

See below.

On 07/23/2018 12:51 PM, Nathan Fontenot wrote:
> On 07/13/2018 03:18 PM, Michael Bringmann wrote:
>> migration/dlpar: This patch adds function dlpar_queue_action()
>> which will queued up information about a CPU/Memory 'readd'
>> operation according to resource type, action code, and DRC index.
>> At a subsequent point, the list of operations can be run/played
>> in series.  Examples of such oprations include 'readd' of CPU
>> and Memory blocks identified as having changed their associativity
>> during an LPAR migration event. >
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in patch:
>>    -- Correct drc_index before adding to pseries_hp_errorlog struct
>>    -- Correct text of notice
>>    -- Revise queuing model to save up all of the DLPAR actions for
>>       later execution.
>>    -- Restore list init statement missing from patch
>>    -- Move call to apply queued operations into 'mobility.c'
>>    -- Compress some code
>>    -- Rename some of queueing function APIs
>>    -- Revise implementation to push execution of queued operations
>>       to a workqueue task.
>>    -- Cleanup reference to outdated queuing operation.
>> ---
>>   arch/powerpc/include/asm/rtas.h           |    2 +
>>   arch/powerpc/platforms/pseries/dlpar.c    |   61 +++++++++++++++++++++++++++++
>>   arch/powerpc/platforms/pseries/mobility.c |    4 ++
>>   arch/powerpc/platforms/pseries/pseries.h  |    2 +
>>   4 files changed, 69 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>> index 71e393c..4f601c7 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog {
>>           struct { __be32 count, index; } ic;
>>           char    drc_name[1];
>>       } _drc_u;
>> +    struct list_head list;
>>   };
>>
>>   #define PSERIES_HP_ELOG_RESOURCE_CPU    1
>>   #define PSERIES_HP_ELOG_RESOURCE_MEM    2
>>   #define PSERIES_HP_ELOG_RESOURCE_SLOT    3
>>   #define PSERIES_HP_ELOG_RESOURCE_PHB    4
>> +#define PSERIES_HP_ELOG_RESOURCE_PMT    5
>>
>>   #define PSERIES_HP_ELOG_ACTION_ADD    1
>>   #define PSERIES_HP_ELOG_ACTION_REMOVE    2
>> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
>> index a0b20c0..7264b8e 100644
>> --- a/arch/powerpc/platforms/pseries/dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/dlpar.c
>> @@ -25,6 +25,7 @@
>>   #include <asm/prom.h>
>>   #include <asm/machdep.h>
>>   #include <linux/uaccess.h>
>> +#include <linux/delay.h>
>>   #include <asm/rtas.h>
>>
>>   static struct workqueue_struct *pseries_hp_wq;
>> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index)
>>       return 0;
>>   }
>>
>> +static int dlpar_pmt(struct pseries_hp_errorlog *work);
>> +
>>   static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>>   {
>>       int rc;
>> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
>>       case PSERIES_HP_ELOG_RESOURCE_CPU:
>>           rc = dlpar_cpu(hp_elog);
>>           break;
>> +    case PSERIES_HP_ELOG_RESOURCE_PMT:
>> +        rc = dlpar_pmt(hp_elog);
>> +        break;
>>       default:
>>           pr_warn_ratelimited("Invalid resource (%d) specified\n",
>>                       hp_elog->resource);
>> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>>       }
>>   }
>>
>> +LIST_HEAD(dlpar_delayed_list);
>> +
>> +int dlpar_queue_action(int resource, int action, u32 drc_index)
>> +{
>> +    struct pseries_hp_errorlog *hp_errlog;
>> +
>> +    hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL);
>> +    if (!hp_errlog)
>> +        return -ENOMEM;
>> +
>> +    hp_errlog->resource = resource;
>> +    hp_errlog->action = action;
>> +    hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
>> +    hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index);
>> +
>> +    list_add_tail(&hp_errlog->list, &dlpar_delayed_list);
>> +
>> +    return 0;
>> +}
>> +
>> +static int dlpar_pmt(struct pseries_hp_errorlog *work)
>> +{
>> +    struct list_head *pos, *q;
>> +
>> +    ssleep(15);
>> +
>> +    list_for_each_safe(pos, q, &dlpar_delayed_list) {
>> +        struct pseries_hp_errorlog *tmp;
>> +
>> +        tmp = list_entry(pos, struct pseries_hp_errorlog, list);
>> +        handle_dlpar_errorlog(tmp);
>> +
>> +        list_del(pos);
>> +        kfree(tmp);
>> +
>> +        ssleep(10);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int dlpar_queued_actions_run(void)
>> +{
>> +    if (!list_empty(&dlpar_delayed_list)) {
>> +        struct pseries_hp_errorlog hp_errlog;
>> +
>> +        hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT;
>> +        hp_errlog.action = 0;
>> +        hp_errlog.id_type = 0;
>> +
>> +        queue_hotplug_event(&hp_errlog, 0, 0); > +    }
>> +    return 0;
>> +}
> 
> I'm a bit confused by this. Is there a reason this needs to queue a
> hotplug event instead of just walking the list as is done in dlpar_pmt?

Up to this point, the operations have only been added to 'dlpar_delayed_list'.
This function separates the execution of the CPU readd and Memory readd
operations from the execution of 'migration store'.  If we walk the list
here, then we add the execution time of all of the readd operations to
the time of 'migration store'.  This is not a large problem in small
systems like we have in the Kernel Team.  This may be a major issue though,
for production SAP HANA systems where we may be readding thousands of pages
of memory.  By pushing the execution of the CPU readd and Memory readd
operations after, and separate, from the execution of 'migration store',
we do not delay the end of the operation or the return of the completion
status to an associated HMC.

> 
> -Nathan

Michael

> 
>> +
>>   static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
>>   {
>>       char *arg;
>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>> index f6364d9..d0d1cae 100644
>> --- a/arch/powerpc/platforms/pseries/mobility.c
>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class,
>>           return rc;
>>
>>       post_mobility_fixup();
>> +
>> +    /* Apply any necessary changes identified during fixup */
>> +    dlpar_queued_actions_run();
>> +
>>       return count;
>>   }
>>
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 60db2ee..72ca996 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32,
>>
>>   void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>>                struct completion *hotplug_done, int *rc);
>> +int dlpar_queue_action(int resource, int action, u32 drc_index);
>> +int dlpar_queued_actions_run(void);
>>   #ifdef CONFIG_MEMORY_HOTPLUG
>>   int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>>   #else
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Ellerman]

Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
> On 07/23/2018 10:54 AM, John Allen wrote:
>> On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote:
>>> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlo=
g *hp_errlog,
...
>>> +
>>> +static int dlpar_pmt(struct pseries_hp_errorlog *work)
>>> +{
>>> +=C2=A0=C2=A0=C2=A0 struct list_head *pos, *q;
>>> +
>>> +=C2=A0=C2=A0=C2=A0 ssleep(15);
>>=20
>> Why do we need to sleep for so long here?
>
> One or more drivers were finishing their initialization about the same
> time as the 'migration_store' operation was completing.  E.g. 'ibmvscsi'
> When we did 'dlpar cpu readd' operations at the same time early on in
> testing, the errors / warnings were inconsistent.  I put in a delay to
> get beyond the driver re-init after migration.  15 seconds was an estimat=
e.
> As it occurs after the completion of the 'migration store' operation,
> it does not delay any responses returned to the HMC.
>
> We can rerun tests with reduced / removed delays to see if they are still
> necessary given some other corrections in the mix.

Please do.

I'm not inclined to merge code with "sleep 15" in it, unless there's
some incredibly good reason for it.

cheers

Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Ellerman]

Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
> On 07/23/2018 12:51 PM, Nathan Fontenot wrote:
>> On 07/13/2018 03:18 PM, Michael Bringmann wrote:
...
>>> +
>>> +int dlpar_queued_actions_run(void)
>>> +{
>>> +=C2=A0=C2=A0=C2=A0 if (!list_empty(&dlpar_delayed_list)) {
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct pseries_hp_errorlog =
hp_errlog;
>>> +
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.resource =3D PSER=
IES_HP_ELOG_RESOURCE_PMT;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.action =3D 0;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.id_type =3D 0;
>>> +
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 queue_hotplug_event(&hp_err=
log, 0, 0); > +=C2=A0=C2=A0=C2=A0 }
>>> +=C2=A0=C2=A0=C2=A0 return 0;
>>> +}
>>=20
>> I'm a bit confused by this. Is there a reason this needs to queue a
>> hotplug event instead of just walking the list as is done in dlpar_pmt?
>
> Up to this point, the operations have only been added to 'dlpar_delayed_l=
ist'.
> This function separates the execution of the CPU readd and Memory readd
> operations from the execution of 'migration store'.  If we walk the list
> here, then we add the execution time of all of the readd operations to
> the time of 'migration store'.  This is not a large problem in small
> systems like we have in the Kernel Team.  This may be a major issue thoug=
h,
> for production SAP HANA systems where we may be readding thousands of pag=
es
> of memory.  By pushing the execution of the CPU readd and Memory readd
> operations after, and separate, from the execution of 'migration store',
> we do not delay the end of the operation or the return of the completion
> status to an associated HMC.

But you can't return completion status, because the operation hasn't
completed.

The migration isn't finished until we've updated the topology for the
new system.

If you decouple things like this you now have no way of reporting
progress or an error to the caller.

So I'm unconvinced this is the right solution.

cheers