Re: [PATCH 1/2] x86: constify local variables in mapcache_current_vcpu()

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Tue, Oct 13, 2026 at 11:05:44AM +0200, Jan Beulich wrote:
> As of the referenced commit v won't be returned from the function anymore.
> Yet that was the sole reason for it to not be pointer-to-const. curr,
> which was introduced by that change, had no reason to be pointer-to-non-
> const from the start.
> 
> Amends: 622c9a5ba95d ("x86/mm: accurately track which vCPU page-tables are 
> loaded")

I think I recall how this ended the way it is: initially I was
returning v, which was wrong because this_cpu(pgtable_vcpu) will have
changed after the sync_local_execstate().  Later I adjusted the return
value of the function to be this_cpu(pgtable_vcpu), but didn't made v
const.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

> ---
> I was heavily tempted to also replace the smp_processor_id() at this
> occasion (by curr->processor). With gcc16, that leads to the function
> actually being inlined (in debug builds), i.e. a significant change in
> generated code, and a measurable increase in code size. From the function
> being declared "inline", that looks to have been the intention, yet then
> that use of "inline" doesn't quite fit our principles. The main reasoning
> for this as a secondary change would be:
> - "curr" could be dropped (instead of having const added) if code
>   generated from ASSERT() wouldn't matter quality-wise,
> - if generated code quality matters there, then the unnecessary use of
>   smp_processor_id() should matter, too.
> However, an adjustment along these lines may better fit in a subsequent
> change dealing with the repeated this_cpu() here.

We might want to introduce a local cpu variable to hold
curr->processor and then replace the this_cpu() with per_cpu(),
together with removing the usage of smp_processor_id().  Shouldn't
make the code more complicated IMO.

Thanks, Roger.