Messages in this thread | | | From | Joel Fernandes <> | Date | Thu, 5 Oct 2017 16:29:40 -0700 | Subject | Re: [PATCH v7 2/2] tracing: Add support for preempt and irq enable/disable events |
| |
On Thu, Oct 5, 2017 at 4:28 PM, Joel Fernandes <joelaf@google.com> wrote: > Hi Peter, > > On Wed, Oct 4, 2017 at 9:01 AM, Peter Zijlstra <peterz@infradead.org> wrote: >> On Fri, Sep 29, 2017 at 02:22:45PM -0700, Joel Fernandes wrote: [...] >>> EXPORT_SYMBOL(trace_hardirqs_off); >>> >>> __visible void trace_hardirqs_on_caller(unsigned long caller_addr) >>> { >>> + if (!this_cpu_read(tracing_irq_cpu)) >>> + return; >>> + >>> + trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr); >>> tracer_hardirqs_on_caller(caller_addr); >>> + >>> + this_cpu_write(tracing_irq_cpu, 0); >>> } >>> EXPORT_SYMBOL(trace_hardirqs_on_caller); >>> >>> __visible void trace_hardirqs_off_caller(unsigned long caller_addr) >>> { >>> + if (this_cpu_read(tracing_irq_cpu)) >>> + return; >>> + >>> + this_cpu_write(tracing_irq_cpu, 1); >>> + >>> + trace_irq_disable_rcuidle(CALLER_ADDR0, caller_addr); >>> tracer_hardirqs_off_caller(caller_addr); >>> } >>> EXPORT_SYMBOL(trace_hardirqs_off_caller); >> >> 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
Sorry, I got lost with double-negatives here, I meant "I don't think it makes sense to.."
thanks,
- Joel
| |