Re: [PATCH v2 3/3] powerpc/mm/hash: Avoid multiple HPT resize-downs on memory hotunplug
From: Leonardo Brás <hidden>
Date: 2021-06-09 05:30:26
Also in:
lkml
On Mon, 2021-06-07 at 15:20 +1000, David Gibson wrote:
On Fri, Apr 30, 2021 at 11:36:10AM -0300, Leonardo Bras wrote:quoted
During memory hotunplug, after each LMB is removed, the HPT may be resized-down if it would map a max of 4 times the current amount of memory. (2 shifts, due to introduced histeresis) It usually is not an issue, but it can take a lot of time if HPT resizing-down fails. This happens because resize-down failures usually repeat at each LMB removal, until there are no more bolted entries conflict, which can take a while to happen. This can be solved by doing a single HPT resize at the end of memory hotunplug, after all requested entries are removed. To make this happen, it's necessary to temporarily disable all HPT resize-downs before hotunplug, re-enable them after hotunplug ends, and then resize-down HPT to the current memory size. As an example, hotunplugging 256GB from a 385GB guest took 621s without this patch, and 100s after applied. Signed-off-by: Leonardo Bras <redacted>Hrm. This looks correct, but it seems overly complicated. AFAICT, the resize calls that this adds should in practice be the *only* times we call resize, all the calls from the lower level code should be suppressed.
That's correct.
In which case can't we just remove those calls entirely, and not deal with the clunky locking and exclusion here. That should also remove the need for the 'shrinking' parameter in 1/3.
If I get your suggestion correctly, you suggest something like: 1 - Never calling resize_hpt_for_hotplug() in hash__remove_section_mapping(), thus not needing the srinking parameter. 2 - Functions in hotplug-memory.c that call dlpar_remove_lmb() would in fact call another function to do the batch resize_hpt_for_hotplug() for them If so, that assumes that no other function that currently calls resize_hpt_for_hotplug() under another path, or if they do, it does not need to actually resize the HPT. Is the above correct? There are some examples of functions that currently call resize_hpt_for_hotplug() by another path: add_memory_driver_managed virtio_mem_add_memory dev_dax_kmem_probe reserve_additional_memory balloon_process add_ballooned_pages __add_memory probe_store __remove_memory pseries_remove_memblock remove_memory dev_dax_kmem_remove virtio_mem_remove_memory memunmap_pages pci_p2pdma_add_resource virtio_fs_setup_dax Best regards, Leonardo Bras