Le 26/01/2023 à 23:30, Benjamin Gray a écrit :
> On Wed, 2023-01-25 at 09:43 +0000, Christophe Leroy wrote:
>
>> By the way, are you should the problem is really BUILD_BUG() ?
>> Looking
>> at your patch I would think that the problem is because it is "static
>> inline". Have you tried 'static __always_inline' instead ?
>
> I did not try it, so I just did but it doesn't make a difference.
>
> Looking further, the failing config also enabled
> CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG, which causes the
> mmu_has_feature(MMU_FTR_TYPE_RADIX) call of radix_enabled() to be non-
> trivial. It must check static_key_initialized, and falls back to
> early_mmu_has_feature if it triggers. Clang apparently can't see that
> early_mmu_has_feature will also always return false for Radix, so
> doesn't see that everything guarded by radix_enabled() is dead code. I
> suppose it's complicated by the fact it still has to run
> mmu_has_feature for the assertion side effect despite the return value
> being knowable at compile time.
>
> So because of this weird interaction, the following should (and does)
> also prevent the compilation error by making the radix_enabled() return
> value more obvious to the compiler in the case where Radix is not
> implemented. (I've put the constant second here so the early usage
> assertion still runs).
But then, that's probably not the only place where we may get an issue
with radix_enabled() or any other use of mmu_has_feature() by the way.
We are in a trivial situation where the feature check is either always
true or always false. Is it worth checking for jump label init in that
case ?
I think the best solution should be to move the following trivial checks
above the static_key_initialised check:
if (MMU_FTRS_ALWAYS & feature)
return true;
if (!(MMU_FTRS_POSSIBLE & feature))
return false;
Christophe
>
> diff --git a/arch/powerpc/include/asm/mmu.h
> b/arch/powerpc/include/asm/mmu.h
> index 94b981152667..3592ff9a522b 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -327,7 +327,7 @@ static inline void assert_pte_locked(struct
> mm_struct *mm, unsigned long addr)
>
> static __always_inline bool radix_enabled(void)
> {
> - return mmu_has_feature(MMU_FTR_TYPE_RADIX);
> + return mmu_has_feature(MMU_FTR_TYPE_RADIX) &&
> IS_ENABLED(CONFIG_PPC_RADIX_MMU);
> }
>
> static __always_inline bool early_radix_enabled(void)
