[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