mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alex Elder <elder@linaro.org>
To: akpm@linux-foundation.org
Cc: bobby.prani@gmail.com, pmladek@suse.cz, jack@suse.cz,
	mcgrof@suse.com, rostedt@goodmis.org, joe@perches.com,
	linux-kernel@vger.kernel.org
Subject: [PATCH] printk: drop logbuf_cpu volatile qualifier
Date: Fri, 14 Nov 2014 13:19:31 -0600	[thread overview]
Message-ID: <1415992771-9071-1-git-send-email-elder@linaro.org> (raw)

Pranith Kumar posted a patch in which removed the "volatile"
qualifier for the "logbuf_cpu" variable in vprintk_emit().
    https://lkml.org/lkml/2014/11/13/894
In his patch, he used ACCESS_ONCE() for all references to
that symbol to provide whatever protection was intended.

There was some discussion that followed, and in the end
Stephen Rostedt concluded that not only was "volatile" not
needed, neither was it required to use ACCESS_ONCE().  I
offered an elaborate description that concluded Stephen
was right, and Pranith asked me to submit an alternative
patch.  And this is it.

The basic reason "volatile" is not needed is that "logbuf_cpu" has
static storage duration, and vprintk_emit() is an exported
interface.  This means that the value of logbuf_cpu must be read
from memory the first time it is used in a particular call of
vprintk_emit().  The variable's value is read only once in that
function, when it's read it'll be the copy from memory (or cache).

In addition, the value of "logbuf_cpu" is only ever written under
protection of a spinlock.  So the value that is read is the "real"
value (and not an out-of-date cached one).  If its value is not
UINT_MAX, it is the current CPU's processor id, and it will have
been last written by the running CPU.

Reported-by: Pranith Kumar <bobby.prani@gmail.com>
Fix-suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Alex Elder <elder@linaro.org>
---
	Note!!!  I am not able to stress test this patch.

 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ced2b84..fefc8d0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1627,7 +1627,7 @@ asmlinkage int vprintk_emit(int facility, int level,
 	int printed_len = 0;
 	bool in_sched = false;
 	/* cpu currently holding logbuf_lock in this function */
-	static volatile unsigned int logbuf_cpu = UINT_MAX;
+	static unsigned int logbuf_cpu = UINT_MAX;
 
 	if (level == SCHED_MESSAGE_LOGLEVEL) {
 		level = -1;
-- 
1.9.1


             reply	other threads:[~2014-11-14 19:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-14 19:19 Alex Elder [this message]
2014-11-14 19:45 ` Steven Rostedt
2014-11-14 19:46   ` Alex Elder
2014-11-14 19:55     ` Steven Rostedt
2014-11-14 20:01       ` Alex Elder
2014-11-18  9:10 ` Jan Kara

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=1415992771-9071-1-git-send-email-elder@linaro.org \
    --to=elder@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=bobby.prani@gmail.com \
    --cc=jack@suse.cz \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@suse.com \
    --cc=pmladek@suse.cz \
    --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

Powered by JetHome