Messages in this thread Patch in this message | | | Subject | Re: [PATCH v3] pseries/drmem: update LMBs after LPM | From | Laurent Dufour <> | Date | Thu, 29 Apr 2021 14:38:56 +0200 |
| |
Le 29/04/2021 à 12:27, Aneesh Kumar K.V a écrit : > Laurent Dufour <ldufour@linux.ibm.com> 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@linux.ibm.com> >> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com> >> --- >> >> 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.
| |