Re: [PATCH v3] pseries/drmem: update LMBs after LPM
From: Laurent Dufour
Date: Thu Apr 29 2021 - 08:39:30 EST
Le 29/04/2021 à 12:27, Aneesh Kumar K.V a écrit :
Laurent Dufour <ldufour@xxxxxxxxxxxxx> writes:
After a LPM, the device tree node ibm,dynamic-reconfiguration-memory may be
updated by the hypervisor in the case the NUMA topology of the LPAR's
memory is updated.
This is caught by the kernel, but the memory's node is updated because
there is no way to move a memory block between nodes.
If later a memory block is added or removed, drmem_update_dt() is called
and it is overwriting the DT node to match the added or removed LMB. But
the LMB's associativity node has not been updated after the DT node update
and thus the node is overwritten by the Linux's topology instead of the
hypervisor one.
Introduce a hook called when the ibm,dynamic-reconfiguration-memory node is
updated to force an update of the LMB's associativity.
Cc: Tyrel Datwyler <tyreld@xxxxxxxxxxxxx>
Signed-off-by: Laurent Dufour <ldufour@xxxxxxxxxxxxx>
---
V3:
- Check rd->dn->name instead of rd->dn->full_name
V2:
- Take Tyrel's idea to rely on OF_RECONFIG_UPDATE_PROPERTY instead of
introducing a new hook mechanism.
---
arch/powerpc/include/asm/drmem.h | 1 +
arch/powerpc/mm/drmem.c | 35 +++++++++++++++++++
.../platforms/pseries/hotplug-memory.c | 4 +++
3 files changed, 40 insertions(+)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index bf2402fed3e0..4265d5e95c2c 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -111,6 +111,7 @@ int drmem_update_dt(void);
int __init
walk_drmem_lmbs_early(unsigned long node, void *data,
int (*func)(struct drmem_lmb *, const __be32 **, void *));
+void drmem_update_lmbs(struct property *prop);
#endif
static inline void invalidate_lmb_associativity_index(struct drmem_lmb *lmb)
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 9af3832c9d8d..f0a6633132af 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -307,6 +307,41 @@ int __init walk_drmem_lmbs_early(unsigned long node, void *data,
return ret;
}
+/*
+ * Update the LMB associativity index.
+ */
+static int update_lmb(struct drmem_lmb *updated_lmb,
+ __maybe_unused const __be32 **usm,
+ __maybe_unused void *data)
+{
+ struct drmem_lmb *lmb;
+
+ /*
+ * Brut force there may be better way to fetch the LMB
+ */
+ for_each_drmem_lmb(lmb) {
+ if (lmb->drc_index != updated_lmb->drc_index)
+ continue;
+
+ lmb->aa_index = updated_lmb->aa_index;
+ break;
+ }
+ return 0;
+}
+
+/*
+ * Update the LMB associativity index.
+ *
+ * This needs to be called when the hypervisor is updating the
+ * dynamic-reconfiguration-memory node property.
+ */
+void drmem_update_lmbs(struct property *prop)
+{
+ if (!strcmp(prop->name, "ibm,dynamic-memory"))
+ __walk_drmem_v1_lmbs(prop->value, NULL, NULL, update_lmb);
+ else if (!strcmp(prop->name, "ibm,dynamic-memory-v2"))
+ __walk_drmem_v2_lmbs(prop->value, NULL, NULL, update_lmb);
+}
#endif
static int init_drmem_lmb_size(struct device_node *dn)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 8377f1f7c78e..672ffbee2e78 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -949,6 +949,10 @@ static int pseries_memory_notifier(struct notifier_block *nb,
case OF_RECONFIG_DETACH_NODE:
err = pseries_remove_mem_node(rd->dn);
break;
+ case OF_RECONFIG_UPDATE_PROPERTY:
+ if (!strcmp(rd->dn->name,
+ "ibm,dynamic-reconfiguration-memory"))
+ drmem_update_lmbs(rd->prop);
}
return notifier_from_errno(err);
How will this interact with DLPAR memory? When we dlpar memory,
ibm,configure-connector is used to fetch the new associativity details
and set drmem_lmb->aa_index correctly there. Once that is done kernel
then call drmem_update_dt() which will result in the above notifier
callback?
IIUC, the call back then will update drmem_lmb->aa_index again?
Thanks for pointing this Aneesh,
You're right I missed that callback and it was quite invisible during my test
because the value set back in the aa_index was the same.
When dmrem_update_dt() is called, there is no need to update the LMB back and
the DT modify notifier should be ignored.
As DLPAR operations are serialized (by lock_device_hotplug()), I'm proposing to
rely on a boolean static variable to do skip this notification, like this:
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index f0a6633132af..3c0130720086 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -18,6 +18,7 @@ static int n_root_addr_cells, n_root_size_cells;
static struct drmem_lmb_info __drmem_info;
struct drmem_lmb_info *drmem_info = &__drmem_info;
+static bool in_drmem_update;
u64 drmem_lmb_memory_max(void)
{
@@ -178,6 +179,11 @@ int drmem_update_dt(void)
if (!memory)
return -1;
+ /*
+ * Set in_drmem_update to prevent the notifier callback to process the
+ * DT property back since the change is coming from the LMB tree.
+ */
+ in_drmem_update = true;
prop = of_find_property(memory, "ibm,dynamic-memory", NULL);
if (prop) {
rc = drmem_update_dt_v1(memory, prop);
@@ -186,6 +192,7 @@ int drmem_update_dt(void)
if (prop)
rc = drmem_update_dt_v2(memory, prop);
}
+ in_drmem_update = false;
of_node_put(memory);
return rc;
@@ -337,6 +344,12 @@ static int update_lmb(struct drmem_lmb *updated_lmb,
*/
void drmem_update_lmbs(struct property *prop)
{
+ /*
+ * Don't update the LMBs If called from the update done in
+ * drmem_update_dt().
+ */
+ if (in_drmem_update)
+ return;
if (!strcmp(prop->name, "ibm,dynamic-memory"))
__walk_drmem_v1_lmbs(prop->value, NULL, NULL, update_lmb);
else if (!strcmp(prop->name, "ibm,dynamic-memory-v2"))
Any concern with this option?
Laurent.