On Fri, Aug 27, 2021 at 09:45:37AM +0000, Christophe Leroy wrote: > This RFC is to validate the concept of static_call on powerpc. > > Highly copied from x86. > > It replaces ppc_md.get_irq() which is called at every IRQ, by > a static call.
The code looks saner, but does it actually improve performance? I'm thinking the double branch also isn't free. > When updating the call, we just replace the instruction at the > trampoline address by a relative jump to the function. > > For the time being, the case of out-of-range functions is not handled. The paranoid in me would've made it: BUG_ON(patch_branch(...)); just to make sure to notice the target not fitting. Ohh, patch_branch() doesn't return the create_branch() error, perhaps that wants to be fixed? Did you see the arm64 variant that deals with out-of-range functions in their trampoline? https://lore.kernel.org/linux-arm-kernel/20201120082103.4840-1-a...@kernel.org/ Not exactly 'nice' but supposedly that works. > +#define ARCH_DEFINE_STATIC_CALL_NULL_TRAMP(name) \ > + asm(".pushsection .text, \"ax\" \n" \ > + ".align 4 \n" \ > + ".globl " STATIC_CALL_TRAMP_STR(name) " \n" \ > + STATIC_CALL_TRAMP_STR(name) ": \n" \ > + " blr \n" \ > + ".type " STATIC_CALL_TRAMP_STR(name) ", @function \n" \ > + ".size " STATIC_CALL_TRAMP_STR(name) ", . - " > STATIC_CALL_TRAMP_STR(name) " \n" \ > + ".popsection \n") > + Since you support CALL_NULL_TRAMP, your patch function below: > +void arch_static_call_transform(void *site, void *tramp, void *func, bool > tail) > +{ > + mutex_lock(&text_mutex); > + > + if (tramp) > + patch_branch(tramp, (unsigned long)func, 0); > + > + mutex_unlock(&text_mutex); > +} > +EXPORT_SYMBOL_GPL(arch_static_call_transform); Ought to patch in "blr" when !func to be consistent :-) I'm thinking that your core kernel text all fits in the native range and only modules need out-of-range ?