Messages in this thread | | | From | Joel Fernandes <> | Date | Tue, 1 May 2018 14:15:14 -0700 | Subject | Re: [PATCH] tracing/irqtrace: only call trace_hardirqs_on/off when state changes |
| |
On Tue, May 1, 2018 at 1:00 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 1 May 2018 21:48:38 +0200 > Peter Zijlstra <peterz@infradead.org> wrote: > >> On Tue, May 01, 2018 at 03:38:40PM -0400, Steven Rostedt wrote: >> > On Tue, 1 May 2018 21:19:51 +0200 >> > Peter Zijlstra <peterz@infradead.org> wrote: >> >> > > Now, lockdep only minimally tracks these otherwise redundant operations; >> > > see redundant_hardirqs_{on,off} counters, and loosing that doesn't seen >> > > like a big issue. >> > > >> > > But I'm confused how this helps track superfluous things, it looks like >> > > it explicitly tracks _less_ superfluous transitions. >> > >> > I think it is about triggering on OFF->OFF a warning, as that would >> > only happen if we have: >> > >> > local_irq_save(flags); >> > [..] >> > local_irq_disable(); >> > >> >> Ahh, ok. Yes, that is easier to do with these changes. The alternative >> is to add more information to the tracehooks such that we can do the >> same internally, but whatever. >> >> Yeah, I'm fine with the proposed change, but maybe improve the Changelog >> a little for slow people like me :-) > > Great! > > Nicholas, > > I know this is an old patch (from last November), but want to send it > again with a proper change log and signed off by?
I actually wrote the exact same patch yesterday with changes Matsami suggested. However I decided not to send it, since it didn't have any performance improvement (which was the reason I wrote it).
Also with my recent set, I don't think it will help detect repeated calls to trace_hardirqs_off because we are handling that recursive case by using per-cpu variable and bailing out if there is a recursion, before even calling into lockdep.
I have mixed feelings about this patch, I am Ok with this patch but I suggest its sent with the follow-up patch that shows its use of this. And also appreciate if such a follow-up patch is rebased onto the IRQ tracepoint work: https://patchwork.kernel.org/patch/10373129/
What do you think?
thanks,
- Joel
| |