Subject: Re: [PATCH] powerpc/mm: Use jump label to speed up radix_enabled check

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Thu, 09 Jun 2016 21:28:29 +0530

Benjamin Herrenschmidt <[email-masked]> writes:

> On Wed, 2016-04-27 at 12:30 +0530, Aneesh Kumar K.V wrote:
>> Benjamin Herrenschmidt <[email-masked]> writes:
>> 
>> > 
>> > On Wed, 2016-04-27 at 11:00 +1000, Balbir Singh wrote:
>> > > 
>> > > Just basic testing across CPUs with various mm features 
>> > > enabled/disabled. Just for sanity
>> > I still don't think it's worth scattering the change. Either the jump
>> > label works or it doesn't ... The only problem is make sure we identify
>> > all the pre-boot ones but that's about it.
>> > 
>> There are two ways to do this. One is to follow the approach listed
>> below done by Kevin, which is to do the jump_label_init early during boot and
>> switch both cpu and mmu feature check to plain jump label.
>> 
>> http://mid.gmane.org/[email-masked]
>> 
>> I already found one use case of cpu_has_feature before that
>> jump_label_init. In this approach we need to carefully audit all the
>> cpu/mmu_has_feature calls to make sure they don't get called before
>> jump_label_init. A missed conversion mean we miss a cpu/mmu feature
>> check.
>> 
>> 
>> Other option is to follow the patch I posted above, with the simple
>> change of renaming mmu_feature_enabled to mmu_has_feature. So we can
>> use it in early boot without really worrying about when we init jump
>> label.
>> 
>> What do you suggest we follow ?
>
> So I really don't like your patch, sorry :-(
>
> It adds a whole new section "_in_c", duplicates a lot of infrastructure
> somewhat differently etc... ugh.
>
> I'd rather we follow Kevin's approach and convert all the CPU/MMU/...
> feature things to static keys in C. There aren't that many that
> need to be done really early on, we can audit them.
>
> I would suggest doing:
>
>  1- Add __mmu_has_feature/__cpu_has_feature/... which initially is
> identical to the current one (or just make the current one use the __ variant).
>
>  2- Convert selectively the early boot stuff to use __. There aren't *that*
> many, I can help you audit them
>
>  3- Add the static key version for all the non  __
>
> Do you have time or should I look into this ?
>

So how early can we call jump_label_init() ? What is the reason why we
do the existing feature fixups in setup_system and not before that ?.
The previous patch series did end up moving jump_label_init() to

 @@ -250,6 +250,8 @@ void __init early_setup(unsigned long dt_ptr)
         /* Initialize lockdep early or else spinlocks will blow */
         lockdep_init();

 	jump_label_init();

        /* -------- printk is now safe to use ------- */

is that safe to do ?

The challenge with doing it later is that we use some of the page table
update functions like set_pte_at in the early boot path and if we can't
use a jump label variant for that function, then we may loose the
benefit.

-aneesh