linux-kernel - Re: [PATCH v3] sched/clock: Avoid false sharing for sched_clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKfTPtD7sDKL5hB-xBW6VJex3zWJkuBKoiVc9Fox14rwCtqWFQ@mail.gmail.com>
Date: Thu, 22 Jan 2026 11:36:34 +0100
From: Vincent Guittot <vincent.guittot@...aro.org>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: Wangyang Guo <wangyang.guo@...el.com>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>, 
	Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>, 
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, 
	Valentin Schneider <vschneid@...hat.com>, linux-kernel@...r.kernel.org, 
	Shrikanth Hegde <sshegde@...ux.ibm.com>, Benjamin Lei <benjamin.lei@...el.com>, 
	Tim Chen <tim.c.chen@...ux.intel.com>, Tianyou Li <tianyou.li@...el.com>
Subject: Re: [PATCH v3] sched/clock: Avoid false sharing for sched_clock_irqtime

On Mon, 19 Jan 2026 at 05:13, K Prateek Nayak <kprateek.nayak@....com> wrote:
>
> Hello Vincent,
>
> On 1/16/2026 10:11 PM, Vincent Guittot wrote:
> >>> I think that disabling irq time accounting if it was enabled in
> >>> __sched_clock_work() should be good
> >>
> >> I though about this and you can have this particular case as a result
> >> of when TSC vs sched_clock is marked unstable:
> >>
> >>   tsc_init()
> >>     enable_sched_clock_irqtime() # irqtime accounting is enabled here
> >>     ...
> >>     if (unsynchronized_tsc()) # true
> >>       mark_tsc_unstable()
> >>         clear_sched_clock_stable()
> >>           __sched_clock_stable_early = 0;
> >>           ...
> >>           if (static_key_count(&sched_clock_running.key) == 2) # Only happens at sched_clock_init_late()
> >>             __clear_sched_clock_stable(); # Never executed
> >>   ...
> >>
> >>   # late_initcall() phase
> >>   sched_clock_init_late()
> >>     if (__sched_clock_stable_early) # Already false
> >>       __set_sched_clock_stable(); # sched_clock is never marked stable
> >>
> >>   # TSC unstable; irqtime_enabled() is true
> >>
> >>
> >> The current approach from Wangyang covers this case so the v3 should be
> >> good as is.
> >
> > The need of a workqueue is only for CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
> > and I'd like to keep it there instead of making the use of a workqueue
> > the default behavior for disabling irq time accounting whereas it's
> > not needed
> >
> > So either we can use a single workqueue in unstable clock to handle
> > the disabling of one or both __sched_clock_stable and
> > sched_clock_irqtime
> >
> > or  unstable clock need another workqueue for disabling sched_clock_irqtime
>
> Ack! We'll likely need a second workqueue and call it early in
> __clear_sched_clock_stable().
>
> AFAICT, PA-RISC already suffers from this since it marks
> sched_clock_unstable() on SMP but since it uses the generic sched_clock,
> on a >= 1MHz processor, it enabled irqtime anyways and nothing disables
> it later (unlike TSC, the clocksource doesn't have a "mark_unstable"
> callback).

>
> If we can delay enabling the irqtime accounting until we do
> __set_sched_clock_stable() for CONFIG_HAVE_UNSTABLE_SCHED_CLOCK (I have
> no clue if this is acceptable of not), then we can move the enabling /
> disabling of irqtime to the same spots as when we flip
> __sched_clock_stable.

Do you mean to delay the enabling/disabling of irqtime accounting in
sched_clock_init_late for CONFIG_HAVE_UNSTABLE_SCHED_CLOCK case ?

I don't have system with unstable clock so I'm not the best to answer
but if irqtime accounting is not compatible with unstable clock it
should be enabled at the same time as the clock is marked stable

>
> --
> Thanks and Regards,
> Prateek
>