Messages in this thread | | | Date | Tue, 22 Dec 2009 13:17:52 +0800 | From | Li Zefan <> | Subject | Re: [PATCH] tracing: Fix lockdep warning in global_clock() |
| |
Steven Rostedt wrote: > On Mon, 2009-12-21 at 15:40 +0100, Peter Zijlstra wrote: >> On Mon, 2009-12-21 at 15:36 +0800, Li Zefan wrote: >> >>> +++ b/kernel/sched_clock.c >>> @@ -250,9 +250,9 @@ unsigned long long cpu_clock(int cpu) >>> unsigned long long clock; >>> unsigned long flags; >>> >>> - raw_local_irq_save(flags); >>> + local_irq_save(flags); >>> clock = sched_clock_cpu(cpu); >>> - raw_local_irq_restore(flags); >>> + local_irq_restore(flags); >>> >>> return clock; >>> } >>> =============================================== >>> >>> I guess it's still true that lower level functions can take locks? >> No, I removed the locks from cpu_clock a while back. >> > > The problem isn't with locks, it's with disabling interrupts. Lockdep > keeps track of every time interrupts are disabled. If something disables > interrupts with raw_* but then calls something that disables interrupts > the normal way, lockdep will detect that interrupts are being disabled > but they already are disabled. The internal irq disabled variable in > lockdep wont match reality, and this will blow up (well, disable > lockdep). > > So if cpu_clock disables interrupts with local_irq_* then so must > trace_clock_global. >
Looking into sched_clock.c, seems the only place that can trigger lockdep is the local_irq_* in cpu_clock(), though I'm not sure for architecture- specific sched_clock().
If so, I think we can use raw_local_irq_* in cpu_clock(). Actually I tried it and the warning is gone.
| |