Message ID | 20190313102528.4632-1-lvivier@redhat.com (mailing list archive) |
---|---|
State | Accepted |
Commit | f172acbfae1a78b1a3c775f78e8d0dcd15b9d768 |
Headers | show |
Series | [v3] powerpc/mm: move warning from resize_hpt_for_hotplug() | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (9580b71b5a7863c24a9bd18bcd2ad759b86b1eff) |
snowpatch_ozlabs/build-ppc64le | success | build succeeded & removed 0 sparse warning(s) |
snowpatch_ozlabs/build-ppc64be | success | build succeeded & removed 0 sparse warning(s) |
snowpatch_ozlabs/build-ppc64e | success | build succeeded & removed 0 sparse warning(s) |
snowpatch_ozlabs/build-pmac32 | success | build succeeded & removed 0 sparse warning(s) |
snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 72 lines checked |
Le 13/03/2019 à 11:25, Laurent Vivier a écrit : > resize_hpt_for_hotplug() reports a warning when it cannot > resize the hash page table ("Unable to resize hash page > table to target order") but in some cases it's not a problem > and can make user thinks something has not worked properly. > > This patch moves the warning to arch_remove_memory() to > only report the problem when it is needed. > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > > Notes: > v3: move "||" to above line and remove parenthesis > v2: add warning messages for H_PARAMETER and H_RESOURCE > > arch/powerpc/include/asm/sparsemem.h | 4 ++-- > arch/powerpc/mm/hash_utils_64.c | 19 +++++++------------ > arch/powerpc/mm/mem.c | 3 ++- > arch/powerpc/platforms/pseries/lpar.c | 3 ++- > 4 files changed, 13 insertions(+), 16 deletions(-) > > diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h > index 68da49320592..3192d454a733 100644 > --- a/arch/powerpc/include/asm/sparsemem.h > +++ b/arch/powerpc/include/asm/sparsemem.h > @@ -17,9 +17,9 @@ extern int create_section_mapping(unsigned long start, unsigned long end, int ni > extern int remove_section_mapping(unsigned long start, unsigned long end); > > #ifdef CONFIG_PPC_BOOK3S_64 > -extern void resize_hpt_for_hotplug(unsigned long new_mem_size); > +extern int resize_hpt_for_hotplug(unsigned long new_mem_size); > #else > -static inline void resize_hpt_for_hotplug(unsigned long new_mem_size) { } > +static inline int resize_hpt_for_hotplug(unsigned long new_mem_size) { return 0; } > #endif > > #ifdef CONFIG_NUMA > diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c > index 0cc7fbc3bd1c..5aa7594ee71b 100644 > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -755,12 +755,12 @@ static unsigned long __init htab_get_table_size(void) > } > > #ifdef CONFIG_MEMORY_HOTPLUG > -void resize_hpt_for_hotplug(unsigned long new_mem_size) > +int resize_hpt_for_hotplug(unsigned long new_mem_size) > { > unsigned target_hpt_shift; > > if (!mmu_hash_ops.resize_hpt) > - return; > + return 0; > > target_hpt_shift = htab_shift_for_mem_size(new_mem_size); > > @@ -772,16 +772,11 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size) > * 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))) { > - int rc; > - > - rc = mmu_hash_ops.resize_hpt(target_hpt_shift); > - if (rc && (rc != -ENODEV)) > - printk(KERN_WARNING > - "Unable to resize hash page table to target order %d: %d\n", > - target_hpt_shift, rc); > - } > + if (target_hpt_shift > ppc64_pft_size || > + target_hpt_shift < ppc64_pft_size - 1) > + return mmu_hash_ops.resize_hpt(target_hpt_shift); > + > + return 0; > } > > int hash__create_section_mapping(unsigned long start, unsigned long end, int nid) > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 33cc6f676fa6..0d40d970cf4a 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -169,7 +169,8 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 size, > */ > vm_unmap_aliases(); > > - resize_hpt_for_hotplug(memblock_phys_mem_size()); > + if (resize_hpt_for_hotplug(memblock_phys_mem_size()) == -ENOSPC) > + pr_warn("Hash collision while resizing HPT\n"); > > return ret; > } > diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c > index f2a9f0adc2d3..1034ef1fe2b4 100644 > --- a/arch/powerpc/platforms/pseries/lpar.c > +++ b/arch/powerpc/platforms/pseries/lpar.c > @@ -901,8 +901,10 @@ static int pseries_lpar_resize_hpt(unsigned long shift) > break; > > case H_PARAMETER: > + pr_warn("Invalid argument from H_RESIZE_HPT_PREPARE\n"); > return -EINVAL; > case H_RESOURCE: > + pr_warn("Operation not permitted from H_RESIZE_HPT_PREPARE\n"); > return -EPERM; > default: > pr_warn("Unexpected error %d from H_RESIZE_HPT_PREPARE\n", rc); > @@ -918,7 +920,6 @@ static int pseries_lpar_resize_hpt(unsigned long shift) > if (rc != 0) { > switch (state.commit_rc) { > case H_PTEG_FULL: > - pr_warn("Hash collision while resizing HPT\n"); > return -ENOSPC; > > default: >
Hi Michael, as it seems good now, could you pick up this patch for merging? Thanks, Laurent On 13/03/2019 11:25, Laurent Vivier wrote: > resize_hpt_for_hotplug() reports a warning when it cannot > resize the hash page table ("Unable to resize hash page > table to target order") but in some cases it's not a problem > and can make user thinks something has not worked properly. > > This patch moves the warning to arch_remove_memory() to > only report the problem when it is needed. > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > > Notes: > v3: move "||" to above line and remove parenthesis > v2: add warning messages for H_PARAMETER and H_RESOURCE > > arch/powerpc/include/asm/sparsemem.h | 4 ++-- > arch/powerpc/mm/hash_utils_64.c | 19 +++++++------------ > arch/powerpc/mm/mem.c | 3 ++- > arch/powerpc/platforms/pseries/lpar.c | 3 ++- > 4 files changed, 13 insertions(+), 16 deletions(-) > > diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h > index 68da49320592..3192d454a733 100644 > --- a/arch/powerpc/include/asm/sparsemem.h > +++ b/arch/powerpc/include/asm/sparsemem.h > @@ -17,9 +17,9 @@ extern int create_section_mapping(unsigned long start, unsigned long end, int ni > extern int remove_section_mapping(unsigned long start, unsigned long end); > > #ifdef CONFIG_PPC_BOOK3S_64 > -extern void resize_hpt_for_hotplug(unsigned long new_mem_size); > +extern int resize_hpt_for_hotplug(unsigned long new_mem_size); > #else > -static inline void resize_hpt_for_hotplug(unsigned long new_mem_size) { } > +static inline int resize_hpt_for_hotplug(unsigned long new_mem_size) { return 0; } > #endif > > #ifdef CONFIG_NUMA > diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c > index 0cc7fbc3bd1c..5aa7594ee71b 100644 > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -755,12 +755,12 @@ static unsigned long __init htab_get_table_size(void) > } > > #ifdef CONFIG_MEMORY_HOTPLUG > -void resize_hpt_for_hotplug(unsigned long new_mem_size) > +int resize_hpt_for_hotplug(unsigned long new_mem_size) > { > unsigned target_hpt_shift; > > if (!mmu_hash_ops.resize_hpt) > - return; > + return 0; > > target_hpt_shift = htab_shift_for_mem_size(new_mem_size); > > @@ -772,16 +772,11 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size) > * 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))) { > - int rc; > - > - rc = mmu_hash_ops.resize_hpt(target_hpt_shift); > - if (rc && (rc != -ENODEV)) > - printk(KERN_WARNING > - "Unable to resize hash page table to target order %d: %d\n", > - target_hpt_shift, rc); > - } > + if (target_hpt_shift > ppc64_pft_size || > + target_hpt_shift < ppc64_pft_size - 1) > + return mmu_hash_ops.resize_hpt(target_hpt_shift); > + > + return 0; > } > > int hash__create_section_mapping(unsigned long start, unsigned long end, int nid) > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 33cc6f676fa6..0d40d970cf4a 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -169,7 +169,8 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 size, > */ > vm_unmap_aliases(); > > - resize_hpt_for_hotplug(memblock_phys_mem_size()); > + if (resize_hpt_for_hotplug(memblock_phys_mem_size()) == -ENOSPC) > + pr_warn("Hash collision while resizing HPT\n"); > > return ret; > } > diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c > index f2a9f0adc2d3..1034ef1fe2b4 100644 > --- a/arch/powerpc/platforms/pseries/lpar.c > +++ b/arch/powerpc/platforms/pseries/lpar.c > @@ -901,8 +901,10 @@ static int pseries_lpar_resize_hpt(unsigned long shift) > break; > > case H_PARAMETER: > + pr_warn("Invalid argument from H_RESIZE_HPT_PREPARE\n"); > return -EINVAL; > case H_RESOURCE: > + pr_warn("Operation not permitted from H_RESIZE_HPT_PREPARE\n"); > return -EPERM; > default: > pr_warn("Unexpected error %d from H_RESIZE_HPT_PREPARE\n", rc); > @@ -918,7 +920,6 @@ static int pseries_lpar_resize_hpt(unsigned long shift) > if (rc != 0) { > switch (state.commit_rc) { > case H_PTEG_FULL: > - pr_warn("Hash collision while resizing HPT\n"); > return -ENOSPC; > > default: >
Laurent Vivier <lvivier@redhat.com> writes: > Hi Michael, > > as it seems good now, could you pick up this patch for merging? I'll start picking up patches for next starting after rc2, so next week. If you think it's a bug fix I can put it into fixes now, but I don't think it's a bug fix is it? cheers
On 20/03/2019 13:47, Michael Ellerman wrote: > Laurent Vivier <lvivier@redhat.com> writes: >> Hi Michael, >> >> as it seems good now, could you pick up this patch for merging? > > I'll start picking up patches for next starting after rc2, so next week. > > If you think it's a bug fix I can put it into fixes now, but I don't > think it's a bug fix is it? No, it's only cosmetic. Thanks, Laurent
On 20/03/2019 13:47, Michael Ellerman wrote: > Laurent Vivier <lvivier@redhat.com> writes: >> Hi Michael, >> >> as it seems good now, could you pick up this patch for merging? > > I'll start picking up patches for next starting after rc2, so next week. > > If you think it's a bug fix I can put it into fixes now, but I don't > think it's a bug fix is it? Any news about this patch? Thanks, Laurent
diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h index 68da49320592..3192d454a733 100644 --- a/arch/powerpc/include/asm/sparsemem.h +++ b/arch/powerpc/include/asm/sparsemem.h @@ -17,9 +17,9 @@ extern int create_section_mapping(unsigned long start, unsigned long end, int ni extern int remove_section_mapping(unsigned long start, unsigned long end); #ifdef CONFIG_PPC_BOOK3S_64 -extern void resize_hpt_for_hotplug(unsigned long new_mem_size); +extern int resize_hpt_for_hotplug(unsigned long new_mem_size); #else -static inline void resize_hpt_for_hotplug(unsigned long new_mem_size) { } +static inline int resize_hpt_for_hotplug(unsigned long new_mem_size) { return 0; } #endif #ifdef CONFIG_NUMA diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c index 0cc7fbc3bd1c..5aa7594ee71b 100644 --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c @@ -755,12 +755,12 @@ static unsigned long __init htab_get_table_size(void) } #ifdef CONFIG_MEMORY_HOTPLUG -void resize_hpt_for_hotplug(unsigned long new_mem_size) +int resize_hpt_for_hotplug(unsigned long new_mem_size) { unsigned target_hpt_shift; if (!mmu_hash_ops.resize_hpt) - return; + return 0; target_hpt_shift = htab_shift_for_mem_size(new_mem_size); @@ -772,16 +772,11 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size) * 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))) { - int rc; - - rc = mmu_hash_ops.resize_hpt(target_hpt_shift); - if (rc && (rc != -ENODEV)) - printk(KERN_WARNING - "Unable to resize hash page table to target order %d: %d\n", - target_hpt_shift, rc); - } + if (target_hpt_shift > ppc64_pft_size || + target_hpt_shift < ppc64_pft_size - 1) + return mmu_hash_ops.resize_hpt(target_hpt_shift); + + return 0; } int hash__create_section_mapping(unsigned long start, unsigned long end, int nid) diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index 33cc6f676fa6..0d40d970cf4a 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -169,7 +169,8 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 size, */ vm_unmap_aliases(); - resize_hpt_for_hotplug(memblock_phys_mem_size()); + if (resize_hpt_for_hotplug(memblock_phys_mem_size()) == -ENOSPC) + pr_warn("Hash collision while resizing HPT\n"); return ret; } diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c index f2a9f0adc2d3..1034ef1fe2b4 100644 --- a/arch/powerpc/platforms/pseries/lpar.c +++ b/arch/powerpc/platforms/pseries/lpar.c @@ -901,8 +901,10 @@ static int pseries_lpar_resize_hpt(unsigned long shift) break; case H_PARAMETER: + pr_warn("Invalid argument from H_RESIZE_HPT_PREPARE\n"); return -EINVAL; case H_RESOURCE: + pr_warn("Operation not permitted from H_RESIZE_HPT_PREPARE\n"); return -EPERM; default: pr_warn("Unexpected error %d from H_RESIZE_HPT_PREPARE\n", rc); @@ -918,7 +920,6 @@ static int pseries_lpar_resize_hpt(unsigned long shift) if (rc != 0) { switch (state.commit_rc) { case H_PTEG_FULL: - pr_warn("Hash collision while resizing HPT\n"); return -ENOSPC; default: