All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger@xenproject.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Teddy Astie <teddy.astie@vates.tech>
Subject: Re: [PATCH 1/2] x86: constify local variables in mapcache_current_vcpu()
Date: Tue, 13 Oct 2026 12:08:42 +0200	[thread overview]
Message-ID: <as4DKjCDU5EQUt9Z@Mac.lan> (raw)
In-Reply-To: <d7ddc188-0e92-46c3-a38b-9a628eb60cd2@suse.com>

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@suse.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

> ---
> 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.


  reply	other threads:[~2026-10-13 10:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13  9:04 [PATCH 0/2] x86: mapcache_current_vcpu() adjustments Jan Beulich
2026-10-13  9:05 ` [PATCH 1/2] x86: constify local variables in mapcache_current_vcpu() Jan Beulich
2026-10-13 10:08   ` Roger Pau Monné [this message]
2026-10-13  9:06 ` [PATCH 2/2] x86: replace this_cpu() uses " Jan Beulich
2026-10-13 10:15   ` Roger Pau Monné
2026-10-13 11:52     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=as4DKjCDU5EQUt9Z@Mac.lan \
    --to=roger@xenproject.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=teddy.astie@vates.tech \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.