Messages in this thread | | | Date | Wed, 28 May 2014 10:22:37 -0700 | From | John Stultz <> | Subject | Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers |
| |
On 05/28/2014 12:24 AM, Peter Zijlstra wrote: > On Tue, May 27, 2014 at 08:42:14PM -0400, Steven Rostedt wrote: >> On Tue, 2014-05-27 at 17:11 -0700, Stephen Boyd wrote: >> >>> cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds >>> calls seqcount_lockdep_reader_access() which calls local_irq_save() that >> >> seqcount_lockdep_reader_access()?? Ug, I wonder if that should call >> raw_local_irq_save/restore() as it's a lockdep helper to begin with. If >> it's wrong then it's the lockdep infrastructure that broke, not the core >> kernel. >> >> Peter? > > Hurm,.. don't know actually.. so from a lockdep pov it doesn't need to > do that and we can simply remove the local_irq_{save,restore}() from > that function. > > It could be John did it to avoid some IRQ recursion warning, but if so, > he failed to mention it. > > John, remember why you typed those characters?
So.. With seqlocks, we're trying to just make sure reads and writes don't nest under a write. However we don't care if a write nests in a read, because the read will be restarted. An example is someone hitting gettimeofday over and over taking a read, and then an IRQ lands mid-read and we take the write and update the data. This is expected normal behavior. So this was trying to make the read side lockdep aquire/release combo atomic, so we don't create false warnings if an IRQ landed right in-between.
Does that make sense?
thanks -john
| |