Bharata B Rao <bhar...@linux.ibm.com> writes:
> 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.
Yeah, duh. You'd think the function name would have been a sufficient
clue for me :)
>> 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.
OK.
That's probably fine as it is then. Or maybe just a comment in
resize_hpt_for_hotplug() pointing out that resize_hpt will be NULL if
we're using radix.
cheers
