linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Nicholas Piggin <npiggin@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] tracing/irqtrace: only call trace_hardirqs_on/off when state changes
Date: Fri, 17 Nov 2017 02:15:06 +1000	[thread overview]
Message-ID: <20171116161506.19691-1-npiggin@gmail.com> (raw)

In local_irq_save and local_irq_restore, only call irq tracing when
the flag state acutally changes. It is not unexpected for the state
to go disable->disable.

This allows the irq tracing code to better track superfluous
enables and disables, and in future could issue warnings. For the
most part they are harmless, but they can indicate that the caller
has lost track of its irq state.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
I wonder what you think of this patch? After a small fix to
init/main.c and some powerpc fixes, I was able to use this patch 
and a change to warn in the lockdep code in case of redundant ops,
to verify there were no local_irq_enable() when enabled, or
local_irq_disable() when disabled (which was implicated in a bug).

Lots of code to verify so we can't do this immediately, but I
think it's cleaner to enforce the rule?

Thanks,
Nick

 include/linux/irqflags.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 46cb57d5eb13..82287e1c6c52 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -110,7 +110,8 @@ do {						\
 #define local_irq_save(flags)				\
 	do {						\
 		raw_local_irq_save(flags);		\
-		trace_hardirqs_off();			\
+		if (!raw_irqs_disabled_flags(flags))	\
+			trace_hardirqs_off();		\
 	} while (0)
 
 
@@ -118,9 +119,11 @@ do {						\
 	do {						\
 		if (raw_irqs_disabled_flags(flags)) {	\
 			raw_local_irq_restore(flags);	\
-			trace_hardirqs_off();		\
+			if (!irqs_disabled())		\
+				trace_hardirqs_off();	\
 		} else {				\
-			trace_hardirqs_on();		\
+			if (irqs_disabled())		\
+				trace_hardirqs_on();	\
 			raw_local_irq_restore(flags);	\
 		}					\
 	} while (0)
-- 
2.15.0

             reply	other threads:[~2017-11-16 16:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16 16:15 Nicholas Piggin [this message]
2018-05-01 18:46 ` [PATCH] tracing/irqtrace: only call trace_hardirqs_on/off when state changes Steven Rostedt
2018-05-01 19:19   ` Peter Zijlstra
2018-05-01 19:38     ` Steven Rostedt
2018-05-01 19:48       ` Peter Zijlstra
2018-05-01 20:00         ` Steven Rostedt
2018-05-01 21:15           ` Joel Fernandes
2018-05-02  0:12             ` Nicholas Piggin
2018-07-11  1:44               ` Steven Rostedt
2018-07-11  5:58                 ` Joel Fernandes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171116161506.19691-1-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).