mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Satyam Sharma" <satyam.sharma@gmail.com>
To: "Jesper Juhl" <jesper.juhl@gmail.com>
Cc: "Jonathan Corbet" <corbet@lwn.net>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	"Randy Dunlap" <randy.dunlap@oracle.com>
Subject: Re: [PATCH] "volatile considered harmful" document
Date: Thu, 10 May 2007 04:01:12 +0530	[thread overview]
Message-ID: <a781481a0705091531x6d4174abmf181371e43bfaffe@mail.gmail.com> (raw)
In-Reply-To: <9a8748490705091445y65ec74bcpbb9251d9184f481a@mail.gmail.com>

On 5/10/07, Jesper Juhl <jesper.juhl@gmail.com> wrote:
> On 09/05/07, Jonathan Corbet <corbet@lwn.net> wrote:
> > OK, here's an updated version of the volatile document - as a plain text
> > file this time.  It drops a new file in Documentation/, but might it be
> > better as an addition to CodingStyle?
> >
> IMHO this is better as a sepperate document rather than an adition to
> CodingStyle. The use of volatile is not a style issue, it's a
> correctness issue, so it doesn't belong in the CodingStyle document.

Yes, definitely.

> > diff -ruNp linux-2.6/Documentation/volatile.txt volatile/Documentation/volatile.txt
>
> Might I suggest a different filename: volatile-considered-harmful.txt

Yes, I feel it's important to have a filename that strongly states the
dim view its contents take on its subject (the "volatile" type
qualifier in this case). Sort of like stable-api-nonsense.txt vs
stable-api.txt

> > --- linux-2.6/Documentation/volatile.txt        1969-12-31 17:00:00.000000000 -0700
> > +++ volatile/Documentation/volatile.txt 2007-05-09 14:56:40.000000000 -0600
> > @@ -0,0 +1,127 @@
> > +Why the "volatile" type class should not be used
> > +------------------------------------------------
> > +
> > +The C Programming Language, Second Edition (copyright 1988, still known as
> > +"the new C book") has the following to say about the volatile keyword:
> > +
> > +       The purpose of volatile is to force an implementation to suppress
> > +       optimization that could otherwise occur.  For example, for a
> > +       machine with memory-mapped input/output, a pointer to a device
> > +       register might be declared as a pointer to volatile, in
> > +       order to prevent the compiler from removing apparently redundant
> > +       references through the pointer.
> > +
> > +C programmers have often taken volatile to mean that the variable could be
> > +changed outside of the current thread of execution; as a result, they are
>
> you write: "... that the variable could be changed outside of the
> current thread of execution ..."
>
> I suggest: "... that the variable could be changed outside of the
> current thread of execution - a sort of simple atomic variable ..."

I'm not so sure here. Why would any C programmer (worth his weight in
salt) think that volatile objects are automatically _atomic_? At
worst, the mistake someone might make would be to _implement_ locking
primitives using volatile. "that the variable could be changed outside
of the current thread of execution" sounds sufficient to me, and after
all, that is exactly what volatile hints to the compiler.

> > +For most code, none of the above justifications for volatile
> > +apply.  As a result, the use of volatile is likely to be seen as a
> > +bug and will bring additional scrutiny to the code.  Developers who are
> > +tempted to use volatile should take a step back and think about
> > +what they are truly trying to accomplish.
> > +
>
> Suggested addition :
>
> Patches that remove volatile from current code (provided there's a
> good explanation of why the volatile can be removed and how the bug it
> was hiding has been dealt with) are a good thing.
>
> Friends don't let friends use volatile.

Yes, this addition would be nice :-)

  reply	other threads:[~2007-05-09 22:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-09 21:05 Jonathan Corbet
2007-05-09 21:45 ` Jesper Juhl
2007-05-09 22:31   ` Satyam Sharma [this message]
2007-05-09 22:47     ` Jesper Juhl
2007-05-09 23:20       ` Satyam Sharma
2007-05-09 22:05 ` Heikki Orsila
2007-05-09 22:19   ` Randy Dunlap
2007-05-09 22:35 ` Scott Preece
2007-05-10 16:14 ` Giacomo A. Catenazzi
2007-05-10 19:35 ` H. Peter Anvin
2007-05-10 22:00   ` Alan Cox
2007-05-10 21:54 ` Bill Davidsen
2007-05-16  9:30 ` Thomas De Schampheleire
     [not found] <fa.UIDGHS72acFv9jKylmdQQwWcXPA@ifi.uio.no>
     [not found] ` <fa.fKNBJtZJWOQthlLjc1TDfY6jCLc@ifi.uio.no>
2007-05-12 18:32   ` Robert Hancock
2007-05-13 16:30     ` Krzysztof Halasa
2007-05-13 23:26       ` Bill Davidsen
2007-05-13 23:53         ` Jeff Garzik
2007-05-14 14:10           ` Bill Davidsen

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=a781481a0705091531x6d4174abmf181371e43bfaffe@mail.gmail.com \
    --to=satyam.sharma@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=jesper.juhl@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=randy.dunlap@oracle.com \
    /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

all inboxes | Powered by JetHome®