lkml.org 
[lkml]   [2009]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: cpu_clock() in NMIs
From
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon, 14 Dec 2009 10:02:39 +0100

> I'm not sure, traditionally sched_clock() was always called with IRQs
> disabled, and on eg. x86 that is needed because we read the TSC and then
> scale that value depending on CPUfreq state, which can be changed by a
> CPUfreq interrupt. Allowing NMIs in between isn't really a problem,
> allowing IRQs in is.

Ok, that looks fine then.

But, speaking more generally, any local_irq_{disable,save}() done in
an NMI handler has a high probability of being a bug. And it is a
bug if the IRQ disabling is there to prevent re-entry of the code
on the local cpu.

> Now, the SPARC implementation might be good without IRQs disabled, but
> we should at least look at all other arches before we do what you
> propose below. As it removes the IRQ disable from the callsites whereas
> it previously always had that.

Here's a quick audit:

1) Generic kernel/sched_clock.c implementation does a subtraction
of jiffies with a constant, then does some constant math on it.
Should be OK.

2) sparc64 is fine, just reads a register and multiplies with a
boot time calculated value, then shifts down by a constant.
Should be OK.

3) ARM mach-map, same situation as sparc64

4) ARM mach-pxa, same situation as sparc64

5) ARM mach-realview, multiplies counter a constant then divides by
one, should be OK.

6) ARM mach-sa1100, same as mach-realview

7) ARM mach-u300, uses boot time computed multiplier and shift,
should be OK.

8) ARM mach-versatile, same as mach-realview

9) ARM plat-IOP, same as mach-u300

10) ARM plat-omap, same as mach-u300

11) ARM plat-orion, same as sparc64

12) Blackfin, TS version is same as sparc64

non-TS version is unnecessary duplication of generic weak
function version in kernel/sched_clock.c and could be deleted

13) CRIS, another dup of kernel/sched_clock.c weak function

14) FRV, another dup of kernel/sched_clock.c weak function

15) IA64, same as sparc64 for native version.
Paravirt version uses preemption disable, but also relies on
IA64 always setting HAVE_UNSTABLE_SCHED_CLOCK which it does,
and therefore IA64 would still disable interrupts with my change

16) m68knommu coldfire, multiplies by a constant then shifts down by
one, should be OK

17) mn10300, same as sparc64

18) powerpc, non-__USE_RTC() case is same as sparc64

__USE_RTC() case also looks fine

19) s390, depends upon preemption being disabled so that
stop_machine does not interrupt the sched_clock() call

should be OK

20) x86 sets HAVE_UNSTABLE_SCHED_CLOCK

It should be safe.


\
 
 \ /
  Last update: 2009-12-14 20:11    [W:0.064 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site