Message ID | 20210312072940.598696-2-leobras.c@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | powerpc/mm/hash: Time improvements for memory hot(un)plug | expand |
Related | show |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (91966823812efbd175f904599e5cf2a854b39809) |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 61 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Fri, Mar 12, 2021 at 04:29:39AM -0300, Leonardo Bras wrote: > Because hypervisors may need to create HPTs without knowing the guest > page size, the smallest used page-size (4k) may be chosen, resulting in > a HPT that is possibly bigger than needed. > > On a guest with bigger page-sizes, the amount of entries for HTP may be > too high, causing the guest to ask for a HPT resize-down on the first > hotplug. > > This becomes a problem when HPT resize-down fails, and causes the > HPT resize to be performed on every LMB added, until HPT size is > compatible to guest memory size, causing a major slowdown. > > So, avoiding HPT resizing-down on hot-add significantly improves memory > hotplug times. > > As an example, hotplugging 256GB on a 129GB guest took 710s without this > patch, and 21s after applied. > > Signed-off-by: Leonardo Bras <leobras.c@gmail.com> I don't love this approach. Adding the extra flag at this level seems a bit inelegant, and it means we're passing up an easy opportunity to reduce our resource footprint on the host. But... maybe we'll have to do it. I'd like to see if we can get things to work well enough with just the "batching" to avoid multiple resize attempts first. > --- > arch/powerpc/mm/book3s64/hash_utils.c | 36 ++++++++++++++++----------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c > index 73b06adb6eeb..cfb3ec164f56 100644 > --- a/arch/powerpc/mm/book3s64/hash_utils.c > +++ b/arch/powerpc/mm/book3s64/hash_utils.c > @@ -794,7 +794,7 @@ static unsigned long __init htab_get_table_size(void) > } > > #ifdef CONFIG_MEMORY_HOTPLUG > -static int resize_hpt_for_hotplug(unsigned long new_mem_size) > +static int resize_hpt_for_hotplug(unsigned long new_mem_size, bool shrinking) > { > unsigned target_hpt_shift; > > @@ -803,19 +803,25 @@ static int resize_hpt_for_hotplug(unsigned long new_mem_size) > > target_hpt_shift = htab_shift_for_mem_size(new_mem_size); > > - /* > - * To avoid lots of HPT resizes if memory size is fluctuating > - * across a boundary, we deliberately have some hysterisis > - * here: we immediately increase the HPT size if the target > - * shift exceeds the current shift, but we won't attempt to > - * reduce unless the target shift is at least 2 below the > - * current shift > - */ > - if (target_hpt_shift > ppc64_pft_size || > - target_hpt_shift < ppc64_pft_size - 1) > - return mmu_hash_ops.resize_hpt(target_hpt_shift); > + if (shrinking) { > > - return 0; > + /* > + * To avoid lots of HPT resizes if memory size is fluctuating > + * across a boundary, we deliberately have some hysterisis > + * here: we immediately increase the HPT size if the target > + * shift exceeds the current shift, but we won't attempt to > + * reduce unless the target shift is at least 2 below the > + * current shift > + */ > + > + if (target_hpt_shift >= ppc64_pft_size - 1) > + return 0; > + > + } else if (target_hpt_shift <= ppc64_pft_size) { > + return 0; > + } > + > + return mmu_hash_ops.resize_hpt(target_hpt_shift); > } > > int hash__create_section_mapping(unsigned long start, unsigned long end, > @@ -828,7 +834,7 @@ int hash__create_section_mapping(unsigned long start, unsigned long end, > return -1; > } > > - resize_hpt_for_hotplug(memblock_phys_mem_size()); > + resize_hpt_for_hotplug(memblock_phys_mem_size(), false); > > rc = htab_bolt_mapping(start, end, __pa(start), > pgprot_val(prot), mmu_linear_psize, > @@ -847,7 +853,7 @@ int hash__remove_section_mapping(unsigned long start, unsigned long end) > int rc = htab_remove_mapping(start, end, mmu_linear_psize, > mmu_kernel_ssize); > > - if (resize_hpt_for_hotplug(memblock_phys_mem_size()) == -ENOSPC) > + if (resize_hpt_for_hotplug(memblock_phys_mem_size(), true) == -ENOSPC) > pr_warn("Hash collision while resizing HPT\n"); > > return rc;
Hello David, thanks for your feedback. On Mon, 2021-03-22 at 17:49 +1100, David Gibson wrote: > I don't love this approach. Adding the extra flag at this level seems > a bit inelegant, and it means we're passing up an easy opportunity to > reduce our resource footprint on the host. I understand, but trying to reduce resource footprint in host, and mostly failing is what causes hot-add and hot-remove to take so long. > But... maybe we'll have to do it. I'd like to see if we can get > things to work well enough with just the "batching" to avoid multiple > resize attempts first. This batching is something I had thought a lot about. Problem is that there are a lot of generic interfaces between memory hotplug and actually resizing HPT. I tried a simpler approach in patches 2 & 3, so I don't touch much stuff there. Best regards, Leonardo Bras
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c index 73b06adb6eeb..cfb3ec164f56 100644 --- a/arch/powerpc/mm/book3s64/hash_utils.c +++ b/arch/powerpc/mm/book3s64/hash_utils.c @@ -794,7 +794,7 @@ static unsigned long __init htab_get_table_size(void) } #ifdef CONFIG_MEMORY_HOTPLUG -static int resize_hpt_for_hotplug(unsigned long new_mem_size) +static int resize_hpt_for_hotplug(unsigned long new_mem_size, bool shrinking) { unsigned target_hpt_shift; @@ -803,19 +803,25 @@ static int resize_hpt_for_hotplug(unsigned long new_mem_size) target_hpt_shift = htab_shift_for_mem_size(new_mem_size); - /* - * To avoid lots of HPT resizes if memory size is fluctuating - * across a boundary, we deliberately have some hysterisis - * here: we immediately increase the HPT size if the target - * shift exceeds the current shift, but we won't attempt to - * reduce unless the target shift is at least 2 below the - * current shift - */ - if (target_hpt_shift > ppc64_pft_size || - target_hpt_shift < ppc64_pft_size - 1) - return mmu_hash_ops.resize_hpt(target_hpt_shift); + if (shrinking) { - return 0; + /* + * To avoid lots of HPT resizes if memory size is fluctuating + * across a boundary, we deliberately have some hysterisis + * here: we immediately increase the HPT size if the target + * shift exceeds the current shift, but we won't attempt to + * reduce unless the target shift is at least 2 below the + * current shift + */ + + if (target_hpt_shift >= ppc64_pft_size - 1) + return 0; + + } else if (target_hpt_shift <= ppc64_pft_size) { + return 0; + } + + return mmu_hash_ops.resize_hpt(target_hpt_shift); } int hash__create_section_mapping(unsigned long start, unsigned long end, @@ -828,7 +834,7 @@ int hash__create_section_mapping(unsigned long start, unsigned long end, return -1; } - resize_hpt_for_hotplug(memblock_phys_mem_size()); + resize_hpt_for_hotplug(memblock_phys_mem_size(), false); rc = htab_bolt_mapping(start, end, __pa(start), pgprot_val(prot), mmu_linear_psize, @@ -847,7 +853,7 @@ int hash__remove_section_mapping(unsigned long start, unsigned long end) int rc = htab_remove_mapping(start, end, mmu_linear_psize, mmu_kernel_ssize); - if (resize_hpt_for_hotplug(memblock_phys_mem_size()) == -ENOSPC) + if (resize_hpt_for_hotplug(memblock_phys_mem_size(), true) == -ENOSPC) pr_warn("Hash collision while resizing HPT\n"); return rc;
Because hypervisors may need to create HPTs without knowing the guest page size, the smallest used page-size (4k) may be chosen, resulting in a HPT that is possibly bigger than needed. On a guest with bigger page-sizes, the amount of entries for HTP may be too high, causing the guest to ask for a HPT resize-down on the first hotplug. This becomes a problem when HPT resize-down fails, and causes the HPT resize to be performed on every LMB added, until HPT size is compatible to guest memory size, causing a major slowdown. So, avoiding HPT resizing-down on hot-add significantly improves memory hotplug times. As an example, hotplugging 256GB on a 129GB guest took 710s without this patch, and 21s after applied. Signed-off-by: Leonardo Bras <leobras.c@gmail.com> --- arch/powerpc/mm/book3s64/hash_utils.c | 36 ++++++++++++++++----------- 1 file changed, 21 insertions(+), 15 deletions(-)