On Tue, Jul 21, 2020 at 11:45:20AM +1000, Michael Ellerman wrote: > Nathan Lynch <nath...@linux.ibm.com> writes: > > "Aneesh Kumar K.V" <aneesh.ku...@linux.ibm.com> writes: > >> This is the next version of the fixes for memory unplug on radix. > >> The issues and the fix are described in the actual patches. > > > > I guess this isn't actually causing problems at runtime right now, but I > > notice calls to resize_hpt_for_hotplug() from arch_add_memory() and > > arch_remove_memory(), which ought to be mmu-agnostic: > > > > int __ref arch_add_memory(int nid, u64 start, u64 size, > > struct mhp_params *params) > > { > > unsigned long start_pfn = start >> PAGE_SHIFT; > > unsigned long nr_pages = size >> PAGE_SHIFT; > > int rc; > > > > resize_hpt_for_hotplug(memblock_phys_mem_size()); > > > > start = (unsigned long)__va(start); > > rc = create_section_mapping(start, start + size, nid, > > params->pgprot); > > ... > > Hmm well spotted. > > That does return early if the ops are not setup: > > int resize_hpt_for_hotplug(unsigned long new_mem_size) > { > unsigned target_hpt_shift; > > if (!mmu_hash_ops.resize_hpt) > return 0; > > > And: > > void __init hpte_init_pseries(void) > { > ... > if (firmware_has_feature(FW_FEATURE_HPT_RESIZE)) > mmu_hash_ops.resize_hpt = pseries_lpar_resize_hpt; > > And that comes in via ibm,hypertas-functions: > > {FW_FEATURE_HPT_RESIZE, "hcall-hpt-resize"}, > > > But firmware is not necessarily going to add/remove that call based on > whether we're using hash/radix.
Correct but hpte_init_pseries() will not be called for radix guests. > > So I think a follow-up patch is needed to make this more robust. > > Aneesh/Bharata what platform did you test this series on? I'm curious > how this didn't break. I have tested memory hotplug/unplug for radix guest on zz platform and sanity-tested this for hash guest on P8. As noted above, mmu_hash_ops.resize_hpt will not be set for radix guest and hence we won't see any breakage. However a separate patch to fix this will be good. Regards, Bharata.