Hi Peter, On Fri, Oct 6, 2017 at 12:07 AM, Peter Zijlstra <pet...@infradead.org> wrote: > On Thu, Oct 05, 2017 at 04:28:10PM -0700, Joel Fernandes wrote: >> > lockdep implements the trace_hardirq_*() in terms of *_caller(). Would >> > that make sense here? >> >> In lockdep code, when trace_hardirqs_off is called, >> trace_hardirqs_off_caller would pass CALLER_ADDR0 as >> trace_hardirqs_off. >> >> Because of this, the first argument passed to time_hardirqs_off would >> always be an offset within trace_hardirqs_off: >> time_hardirqs_off(CALLER_ADDR0, ip); >> >> Is that intended? Seems to me that in the lockdep implementation of >> trace_hardirqs_* in terms of *_caller(), we would completely miss the >> second-last return address (CALLER_ADDR1) of trace_hardirqs_off(). >> Also for the above reasons, I don't think it doesn't make sense to use >> this reuse logic for the tracer. Atleast I feel it might change the >> current behavior of the preempt/irqsoff tracer which I don't intend to >> change with my current patch set. > > Hurm.. so I've no clue. I never looked at any of this. I think Ingo > wrote that before I came joined the fun. He might (although unlikely, > because its been _many_ years) still have some memories. > > But given that lockdep enabled kernels never got those arguments right, > how useful are they? I mean, nobody seems to have noticed much.
Oh ok. So I traced this down to the original patch that added time_hardirqs_off to lockdep. I *think* it was added just to keep the irqsoff tracer working while lockdep is enabled, so that both lockdep and the tracer would work at the same time. So I guess nobody noticed any issues because nobody noticed or cared to check if irqsoff tracer showed the same results with lockdep enabled vs disabled. I think the correct way this should be rewritten is lockdep should use register_trace_* to register callbacks onto the new tracepoints I'm adding. I am working on a patch that does something like this and gets rid of the time_* functions. I will try to have an RFC out by the weekend if I hopefully don't hit any roadblocks. thanks! - Joel
