Message-ID: <a781481a0705091531x6d4174abmf181371e43bfaffe@mail.gmail.com>
Date:	Thu, 10 May 2007 04:01:12 +0530
From:	"Satyam Sharma" <satyam.sharma@...il.com>
To:	"Jesper Juhl" <jesper.juhl@...il.com>
Cc:	"Jonathan Corbet" <corbet@....net>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, "Randy Dunlap" <randy.dunlap@...cle.com>
Subject: Re: [PATCH] "volatile considered harmful" document

On 5/10/07, Jesper Juhl <jesper.juhl@...il.com> wrote:
> On 09/05/07, Jonathan Corbet <corbet@....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 :-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists