| Message ID | 20240325224848.20987-3-bgray@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] powerpc/code-patching: Introduce open_patch_window()/close_patch_window() | expand |
Le 25/03/2024 à 23:48, Benjamin Gray a écrit : > The new open/close abstraction makes it more difficult for a > compiler to optimise. This causes 10% worse performance on > ppc32 as in [1]. Restoring the page alignment mask and inlining > the helpers allows the compiler to better reason about the address > alignment, allowing more optimised cache flushing selection. This should be squashed into patch 1. There is no point in having that as a separate patch when in the same series. > > [1]: https://lore.kernel.org/all/77fdcdeb-4af5-4ad0-a4c6-57bf0762dc65@csgroup.eu/ > > Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> > > --- > > v2: * New in v2 > > I think Suggested-by is an appropriate tag. The patch is Christophe's > from the link, I just added the commit description, so it could well > be better to change the author to Christophe completely. > --- > arch/powerpc/lib/code-patching.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > index b3a644290369..d089da115987 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -282,13 +282,13 @@ struct patch_window { > * Interrupts must be disabled for the entire duration of the patching. The PIDR > * is potentially changed during this time. > */ > -static int open_patch_window(void *addr, struct patch_window *ctx) > +static __always_inline int open_patch_window(void *addr, struct patch_window *ctx) > { > unsigned long pfn = get_patch_pfn(addr); > > lockdep_assert_irqs_disabled(); > > - ctx->text_poke_addr = (unsigned long)__this_cpu_read(cpu_patching_context.addr); > + ctx->text_poke_addr = (unsigned long)__this_cpu_read(cpu_patching_context.addr) & PAGE_MASK; > > if (!mm_patch_enabled()) { > ctx->ptep = __this_cpu_read(cpu_patching_context.pte); > @@ -331,7 +331,7 @@ static int open_patch_window(void *addr, struct patch_window *ctx) > return 0; > } > > -static void close_patch_window(struct patch_window *ctx) > +static __always_inline void close_patch_window(struct patch_window *ctx) > { > lockdep_assert_irqs_disabled(); >
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index b3a644290369..d089da115987 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -282,13 +282,13 @@ struct patch_window { * Interrupts must be disabled for the entire duration of the patching. The PIDR * is potentially changed during this time. */ -static int open_patch_window(void *addr, struct patch_window *ctx) +static __always_inline int open_patch_window(void *addr, struct patch_window *ctx) { unsigned long pfn = get_patch_pfn(addr); lockdep_assert_irqs_disabled(); - ctx->text_poke_addr = (unsigned long)__this_cpu_read(cpu_patching_context.addr); + ctx->text_poke_addr = (unsigned long)__this_cpu_read(cpu_patching_context.addr) & PAGE_MASK; if (!mm_patch_enabled()) { ctx->ptep = __this_cpu_read(cpu_patching_context.pte); @@ -331,7 +331,7 @@ static int open_patch_window(void *addr, struct patch_window *ctx) return 0; } -static void close_patch_window(struct patch_window *ctx) +static __always_inline void close_patch_window(struct patch_window *ctx) { lockdep_assert_irqs_disabled();
The new open/close abstraction makes it more difficult for a compiler to optimise. This causes 10% worse performance on ppc32 as in [1]. Restoring the page alignment mask and inlining the helpers allows the compiler to better reason about the address alignment, allowing more optimised cache flushing selection. [1]: https://lore.kernel.org/all/77fdcdeb-4af5-4ad0-a4c6-57bf0762dc65@csgroup.eu/ Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- v2: * New in v2 I think Suggested-by is an appropriate tag. The patch is Christophe's from the link, I just added the commit description, so it could well be better to change the author to Christophe completely. --- arch/powerpc/lib/code-patching.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)