mbox series

[RFC,0/9] local_clock() vs noinstr

Message ID 20230508211951.901961964@infradead.org (mailing list archive)
Headers show
Series local_clock() vs noinstr | expand

Message

Peter Zijlstra May 8, 2023, 9:19 p.m. UTC 
Hi all!

A recent commit of mine marked local_clock() as noinstr.

  776f22913b8e ("sched/clock: Make local_clock() noinstr")

Sadly both me and objtool missed the fact that this is subly broken; but
Sebastian tripped over it [*]:

| vmlinux.o: warning: objtool: native_sched_clock+0x97: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section
| vmlinux.o: warning: objtool: kvm_clock_read+0x22: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section
| vmlinux.o: warning: objtool: local_clock+0xb4: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section

Specifically, local_clock() (and many of the sched_clock() implementation is
relies upon) use preempt_{dis,en}able_notrace() which obviously calls out to
schedule().

Now, noinstr code *should* never trigger this and already run in
non-preemptible code. Specifically entry code should have IRQs disabled while
__cpuidle code should have preemption disabled.

So while it is mostly harmless, I figured it wouldn't be too hard to clean this
up a little -- but that was ~10 patches. Anyway, here goes...

Compile tested only on x86_64/s390/arm64 -- I've just fed it to the
robots.

---
 arch/arm64/include/asm/arch_timer.h    |  8 +----
 arch/arm64/include/asm/io.h            | 12 +++----
 arch/loongarch/include/asm/loongarch.h |  2 +-
 arch/loongarch/kernel/time.c           |  6 ++--
 arch/s390/include/asm/timex.h          | 13 +++++---
 arch/s390/kernel/time.c                | 11 ++++++-
 arch/x86/kernel/kvmclock.c             |  4 +--
 arch/x86/kernel/tsc.c                  | 38 ++++++++++++++++-----
 arch/x86/xen/time.c                    |  3 +-
 drivers/clocksource/arm_arch_timer.c   | 60 ++++++++++++++++++++++++++--------
 drivers/clocksource/hyperv_timer.c     |  4 +--
 drivers/cpuidle/cpuidle.c              |  8 ++---
 drivers/cpuidle/poll_state.c           |  4 +--
 include/clocksource/hyperv_timer.h     |  4 +--
 include/linux/rbtree_latch.h           |  2 +-
 include/linux/sched/clock.h            | 17 +++++++++-
 include/linux/seqlock.h                | 15 +++++----
 kernel/printk/printk.c                 |  2 +-
 kernel/sched/clock.c                   | 19 +++++++----
 kernel/time/sched_clock.c              | 24 ++++++++++----
 kernel/time/timekeeping.c              |  4 +--
 21 files changed, 176 insertions(+), 84 deletions(-)


* https://lkml.kernel.org/r/20230309072724.3F6zRkvw@linutronix.de
  TL;DR: PREEMPT_DYNAMIC=n PREEMPT=y DEBUG_ENTRY=y

Comments

Sebastian Sewior May 10, 2023, 1:43 p.m. UTC | #1 
On 2023-05-08 23:19:51 [+0200], Peter Zijlstra wrote:
> Hi all!
Hi Peter,

> Compile tested only on x86_64/s390/arm64 -- I've just fed it to the
Thanks, this works.

Sebastian