Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers

[John Stultz <john.stultz@linaro.org>]

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