arch/x86/kernel/tsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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];
}
--
2.20.1
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.
On 8/4/26 3:22 PM, Peter Zijlstra wrote:
> 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.
>
Agreed - the loop is N-1 and deliberately skips this_cpu, which is
initialized by cyc2ns_init_boot_cpu(). That CPU is not my concern.
Sorry, my changelog was unclear. The issue is an off-by-one in the
existing code: seqcount_latch_init(&c2n->seq) runs before c2n is
advanced to the target CPU, so each iteration initializes the seqcount
of the previous c2n rather than the current one:
c2n = this_cpu_ptr(&cyc2ns); /* c2n -> boot cpu */
for_each_possible_cpu(cpu) {
if (cpu != this_cpu) {
seqcount_latch_init(&c2n->seq); /* inits the previous c2n */
c2n = per_cpu_ptr(&cyc2ns, cpu); /* only now advanced */
...
}
}
Concretely, with this_cpu=0 and possible cpus {0,1,2,3}:
cpu=1: inits cpu0 (boot, already done by cyc2ns_init_boot_cpu())
cpu=2: inits cpu1
cpu=3: inits cpu2
=> cpu3, the last secondary, is never initialized, while boot is
redundantly re-initialized.
Moving seqcount_latch_init() after per_cpu_ptr() makes each secondary
initialize its own seqcount.
© 2016 - 2026 Red Hat, Inc.