From: Peter Zijlstra <peterz@infradead.org>
To: Bo Li <libo.gcs85@bytedance.com>
Cc: tglx@kernel.org, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
sshegde@linux.ibm.com, jgross@suse.com, wangyang.guo@intel.com,
seanjc@google.com, pasha.tatashin@oracle.com,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] x86/tsc: Fix misplaced seqcount_latch_init() in cyc2ns_init_secondary_cpus()
Date: Tue, 4 Aug 2026 09:22:40 +0200 [thread overview]
Message-ID: <20260804072240.GK687043@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20260804071128.31022-1-libo.gcs85@bytedance.com>
On Tue, Aug 04, 2026 at 03:11:28PM +0800, Bo Li wrote:
> In cyc2ns_init_secondary_cpus(), seqcount_latch_init(&c2n->seq) is
> called _before_ c2n is advanced to the next CPU via per_cpu_ptr().
> As a result:
>
> 1. On the first iteration, c2n still points at the BSP's struct, so
> the BSP's seqcount_latch is re-initialized. cyc2ns_init_boot_cpu()
> already did this correctly, so it happens to work by accident
> because no concurrent readers are live at __init time.
>
> 2. On subsequent iterations, seqcount_latch_init() initializes the
> previous CPU's seqcount (because c2n was advanced by the prior
> per_cpu_ptr()), so all CPUs except the last one in the for_each
> loop happen to get initialized. The last secondary CPU's seqcount
> is left uninitialized.
>
> Move seqcount_latch_init() _after_ c2n is pointed at the target CPU's
> struct, so each secondary CPU's seqcount is correctly initialized.
>
> Fixes: e2a9ca29b5ed ("x86/tsc: Initialize cyc2ns when tsc frequency is determined")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bo Li <libo.gcs85@bytedance.com>
> ---
> arch/x86/kernel/tsc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index ce10ae4b298b..84fb80492b01 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -223,8 +223,8 @@ static void __init cyc2ns_init_secondary_cpus(void)
>
> for_each_possible_cpu(cpu) {
> if (cpu != this_cpu) {
> - seqcount_latch_init(&c2n->seq);
> c2n = per_cpu_ptr(&cyc2ns, cpu);
> + seqcount_latch_init(&c2n->seq);
> c2n->data[0] = data[0];
> c2n->data[1] = data[1];
> }
No, this looks wrong. Note that per the 'cpu != this_cpu' there are only
N-1 invocations. One CPU will not be initialized. Further note the name
of this function, and the name of the function above it.
next prev parent reply other threads:[~2026-08-04 7:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 7:11 Bo Li
2026-08-04 7:22 ` Peter Zijlstra [this message]
2026-08-04 12:54 ` Bo Li
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=20260804072240.GK687043@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=libo.gcs85@bytedance.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pasha.tatashin@oracle.com \
--cc=seanjc@google.com \
--cc=sshegde@linux.ibm.com \
--cc=stable@vger.kernel.org \
--cc=tglx@kernel.org \
--cc=wangyang.guo@intel.com \
--cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®