Le 31/01/2023 à 03:58, Benjamin Gray a écrit :
> The commit introducing this function implemented it as a build bug on this
> platform to make the compiler happy, as the only use in the code is guarded
> behind a radix_enabled() conditional.
>
> GCC recognises that cpu_has_feature(MMU_FTR_TYPE_RADIX) returns false on this
> platform and eliminates the build bug as dead code. However, when
> CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG is enabled, the assertion and possible
> call to early_cpu_... prevents Clang from eliminating any code that's
> conditional on the return value. So Clang triggers the build bug as reported
> by the kernel test robot:
I still think it is not the correct fix. You are putting the problem
under the carpet instead of fixing it. There are many other places where
radix_enabled() or other mmu_has_feature() are used with the expectation
that one leg will be eliminated at build time.
As written in previous thread, have you considered reworking
mmu_has_feature() to move the CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
after the below block:
if (MMU_FTRS_ALWAYS & feature)
return true;
if (!(MMU_FTRS_POSSIBLE & feature))
return false;
And as this looks like a Clang bug or limitation, can you provide us
with a link to the Clang issue you have opened for it ?
Looking into it in more details, I'm even more puzzled. As far as I can
see, local_flush_tlb_page_psize() is used only at one place, that is
function __do_patch_instruction_mm(). So if Clang fails to identify it
as a dead leg, it is the full __do_patch_instruction_mm() which is kept
for no reason.
On the other hand, I see that local_flush_tlb_page_psize() implemented
for nohash/32, so yes we can also implement it for book3s/32. But then
the commit log should explain things differently.
By the way, I also see that local_flush_tlb_page_psize() for book3s/64
does just nothing at all for non Radix. Is that correct ?
Thanks
Christophe
>
> https://lore.kernel.org/llvm/202301212348.edkowvff-...@intel.com
>
> Fixes: 274d842fa1ef ("powerpc/tlb: Add local flush for page given mm_struct
> and psize")
> Reported-by: kernel test robot <l...@intel.com>
> Signed-off-by: Benjamin Gray <bg...@linux.ibm.com>
> ---
>
> Supersedes
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20230124215424.9068-1-bg...@linux.ibm.com/
>
> ---
> arch/powerpc/include/asm/book3s/32/tlbflush.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
> b/arch/powerpc/include/asm/book3s/32/tlbflush.h
> index 4be572908124..cde3b6f5d563 100644
> --- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
> +++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
> @@ -2,8 +2,6 @@
> #ifndef _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
> #define _ASM_POWERPC_BOOK3S_32_TLBFLUSH_H
>
> -#include <linux/build_bug.h>
> -
> #define MMU_NO_CONTEXT (0)
> /*
> * TLB flushing for "classic" hash-MMU 32-bit CPUs, 6xx, 7xx, 7xxx
> @@ -80,7 +78,7 @@ static inline void local_flush_tlb_page(struct
> vm_area_struct *vma,
> static inline void local_flush_tlb_page_psize(struct mm_struct *mm,
> unsigned long vmaddr, int psize)
> {
> - BUILD_BUG();
> + flush_range(mm, vmaddr, vmaddr + PAGE_SIZE);
> }
>
> static inline void local_flush_tlb_mm(struct mm_struct *mm)
>
> base-commit: ca272751ba18ca8f137af631cbc9f3f987fab6e3
> --
> 2.39.1
