Re: [PATCH] powerpc/tlb: Implement book3s/32/tlbflush.h local_flush_tlb_page_psize

[Benjamin Gray <bgray@linux.ibm.com>]

On Tue, 2023-01-31 at 06:33 +0000, Christophe Leroy wrote:
> 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.

And none of them are actively causing build failures AFAIK. I agree
that there may be a pre-existing optimisation problem, but I'm not
trying to address it in this patch. I'm just trying to fix the build I
broke. As such I haven't opened an issue with Clang yet either.

> 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;
> 

Yes, I did. I also discussed with Michael Ellerman what he would
prefer, and he indicated he still would still like to just implement
the function.


> 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.

Right, because that is the function that's guarded behind
radix_enabled().

> 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 ?

That is how the other local page flushes are implemented. If there's
some undocumented exception here I'd be relying on review on the list
from people who have experience with details of how Hash mmu is handled
on this platform.