Skip to content

Commit f3fa0e4

Browse files
laoarIngo Molnar
authored andcommitted
sched/clock: Don't define sched_clock_irqtime as static key
The sched_clock_irqtime was defined as a static key in: 8722903 ("sched: Define sched_clock_irqtime as static key") However, this change introduces a 'sleeping in atomic context' warning: arch/x86/kernel/tsc.c:1214 mark_tsc_unstable() warn: sleeping in atomic context As analyzed by Dan, the affected code path is as follows: vcpu_load() <- disables preempt -> kvm_arch_vcpu_load() -> mark_tsc_unstable() <- sleeps virt/kvm/kvm_main.c 166 void vcpu_load(struct kvm_vcpu *vcpu) 167 { 168 int cpu = get_cpu(); ^^^^^^^^^^ This get_cpu() disables preemption. 169 170 __this_cpu_write(kvm_running_vcpu, vcpu); 171 preempt_notifier_register(&vcpu->preempt_notifier); 172 kvm_arch_vcpu_load(vcpu, cpu); 173 put_cpu(); 174 } arch/x86/kvm/x86.c 4979 if (unlikely(vcpu->cpu != cpu) || kvm_check_tsc_unstable()) { 4980 s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 : 4981 rdtsc() - vcpu->arch.last_host_tsc; 4982 if (tsc_delta < 0) 4983 mark_tsc_unstable("KVM discovered backwards TSC"); arch/x86/kernel/tsc.c 1206 void mark_tsc_unstable(char *reason) 1207 { 1208 if (tsc_unstable) 1209 return; 1210 1211 tsc_unstable = 1; 1212 if (using_native_sched_clock()) 1213 clear_sched_clock_stable(); --> 1214 disable_sched_clock_irqtime(); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ kernel/jump_label.c 245 void static_key_disable(struct static_key *key) 246 { 247 cpus_read_lock(); ^^^^^^^^^^^^^^^^ This lock has a might_sleep() in it which triggers the static checker warning. 248 static_key_disable_cpuslocked(key); 249 cpus_read_unlock(); 250 } Let revert this change for now as {disable,enable}_sched_clock_irqtime are used in many places, as pointed out by Sean, including the following: The code path in clocksource_watchdog(): clocksource_watchdog() | -> spin_lock(&watchdog_lock); | -> __clocksource_unstable() | -> clocksource.mark_unstable() == tsc_cs_mark_unstable() | -> disable_sched_clock_irqtime() And the code path in sched_clock_register(): /* Cannot register a sched_clock with interrupts on */ local_irq_save(flags); ... /* Enable IRQ time accounting if we have a fast enough sched_clock() */ if (irqtime > 0 || (irqtime == -1 && rate >= 1000000)) enable_sched_clock_irqtime(); local_irq_restore(flags); [ [email protected]: reported a build error in the prev version ] [ mingo: cherry-picked it over into sched/urgent ] Closes: https://lore.kernel.org/kvm/[email protected]/ Fixes: 8722903 ("sched: Define sched_clock_irqtime as static key") Reported-by: Dan Carpenter <[email protected]> Debugged-by: Dan Carpenter <[email protected]> Debugged-by: Sean Christopherson <[email protected]> Debugged-by: Michal Koutný <[email protected]> Signed-off-by: Yafang Shao <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Reviewed-by: Vincent Guittot <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 80e54e8 commit f3fa0e4

File tree

2 files changed

+6
-6
lines changed

2 files changed

+6
-6
lines changed

kernel/sched/cputime.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
#ifdef CONFIG_IRQ_TIME_ACCOUNTING
1111

12-
DEFINE_STATIC_KEY_FALSE(sched_clock_irqtime);
13-
1412
/*
1513
* There are no locks covering percpu hardirq/softirq time.
1614
* They are only modified in vtime_account, on corresponding CPU
@@ -24,14 +22,16 @@ DEFINE_STATIC_KEY_FALSE(sched_clock_irqtime);
2422
*/
2523
DEFINE_PER_CPU(struct irqtime, cpu_irqtime);
2624

25+
int sched_clock_irqtime;
26+
2727
void enable_sched_clock_irqtime(void)
2828
{
29-
static_branch_enable(&sched_clock_irqtime);
29+
sched_clock_irqtime = 1;
3030
}
3131

3232
void disable_sched_clock_irqtime(void)
3333
{
34-
static_branch_disable(&sched_clock_irqtime);
34+
sched_clock_irqtime = 0;
3535
}
3636

3737
static void irqtime_account_delta(struct irqtime *irqtime, u64 delta,

kernel/sched/sched.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,11 +3259,11 @@ struct irqtime {
32593259
};
32603260

32613261
DECLARE_PER_CPU(struct irqtime, cpu_irqtime);
3262-
DECLARE_STATIC_KEY_FALSE(sched_clock_irqtime);
3262+
extern int sched_clock_irqtime;
32633263

32643264
static inline int irqtime_enabled(void)
32653265
{
3266-
return static_branch_likely(&sched_clock_irqtime);
3266+
return sched_clock_irqtime;
32673267
}
32683268

32693269
/*

0 commit comments

Comments
 (0)