Re: [PATCH 2/2] x86: replace this_cpu() uses in mapcache_current_vcpu()

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Tue, Oct 13, 2026 at 11:06:27AM +0200, Jan Beulich wrote:
> We have accumulated three of them, plus a hidden similar 4th construct
> from smp_processor_id(). Latch the result of the latter into a local
> variable and use the slightly more efficient per_cpu() instead.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Oh, so that's kind of what I've suggested in the previous patch.

> ---
> A 5th such construct is "current", yet to replace that we'd need to open-
> code both it (i.e. get_current()) and then also smp_processor_id(). I
> wasn't quite sure whether that would be deemed acceptable.

See below, we can reorder and use the processor ID value stashed in
current?

> Like indicated in the earlier patch, this similarly leads to the function
> actually being inlined (and hence generated code size actually growing) in
> gcc16 debug builds. In release builds (with older gcc) the function is
> being inlined no matter what. Overall, having looked at generated code
> differences, I'm not quite sure anymore whether the earlier observed
> inefficiency with repeated this_cpu() really is much of an issue (here or
> maybe even more generally).

I think we sometimes focus too much on this kind of
micro-optimizations when the cost of the operation itself is orders of
magnitude bigger than any performance difference the optimization
might cause.

In this case I don't think the resulting code is bad or harder to
read, so I don't mind adjusting the logic.  But that's a question of
taste, if someone else has a preference for the original form I
wouldn't change it.

> --- a/xen/arch/x86/domain_page.c
> +++ b/xen/arch/x86/domain_page.c
> @@ -20,7 +20,8 @@
>  
>  static inline struct vcpu *mapcache_current_vcpu(void)
>  {
> -    const struct vcpu *v = this_cpu(pgtable_vcpu);
> +    unsigned int cpu = smp_processor_id();
> +    const struct vcpu *v = per_cpu(pgtable_vcpu, cpu);
>      const struct vcpu *curr = current;

Maybe you can reorder the variables here:

const struct vcpu *curr = current;
unsigned int cpu = curr->processor;
const struct vcpu *v = per_cpu(pgtable_vcpu, cpu);

So that we avoid two smp_processor_id() calls (ie: the hidden one in
current).

Thanks, Roger.