lkml.org 
[lkml]   [2006]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] remove volatile from nmi.c


On Fri, 14 Jul 2006, Chase Venters wrote:
> >
> > static int __init check_nmi_watchdog(void)
> > {
> > - volatile int endflag = 0;
> > + static int endflag = 0;
>
> Now that this is static, isn't this a candidate for __initdata?

Yes, that would be good.

Somebody want to test that it actually still _works_, and go through all
the logic?

On a similar vein: Steven, looking at the cmos version of the patch, I
have a hard time knowing whether the added barriers are needed, because I
didn't spend any time on looking at the context of the patch. But I
suspect that generally you do _not_ want to add barriers when you remove
volatiles.

Basically, "volatile" is not a sign that a barrier is needed per se. In
many cases, the _only_ thing that "volatile" implies is that the original
programmer was confused and/or lazy.

So replacing volatiles with accesses with barriers is usually the _wrong_
thing to do. The right thing to do is generally to just _remove_ the
volatile entirely, and then think hard about whether there was some _real_
reason why it existed in the first place.

Note that the only thing a volatile can do is a _compiler_ barrier, so if
you add a real memory barrier or make it use a "set_wmb()" or similar,
you're literally changing code that has been tested to work, and you're
in the process also removing the hint that the code may actually have
fundamental problems.

So I'd argue that it's actually _worse_ to do a "mindless" conversion away
from volatile, than it is to just remove them outright. Removing them
outright may show a bug that the volatile hid (and at that point, people
may see what the _deeper_ problem was), but at least it won't add a memory
barrier that isn't necessary and will potentially just confuse people.

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-11-18 23:46    [W:0.046 / U:0.812 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site