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

Christophe Leroy christophe.leroy at csgroup.eu
Wed Feb 1 21:16:06 AEDT 2023



Le 31/01/2023 à 22:58, Benjamin Gray a écrit :
> 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.

I'm fine with that. But if it is the intention, don't focus on the bug 
it fixes, but more on what it implements and how. That's what I would 
like to read in the commit message. It is nice to explain that there is 
a problem with CLANG and what the problem is, but only as side benefit 
of the commit.

For instance, explain why you can use flush_range() to implement it, and 
why you use PAGE_SIZE and not psize, etc ...

Christophe


More information about the Linuxppc-dev mailing list