Date: Fri, 2 Dec 2005 04:53:36 +0100
From: Pavel Machek <pavel@ucw.cz>
To: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: "Zhang, Yanmin" <yanmin.zhang@intel.com>, linux-kernel@vger.kernel.org,
       "Shah, Rajesh" <rajesh.shah@intel.com>
Subject: Re: [BUG] Variable stopmachine_state should be volatile
Message-ID: <20051202035336.GC1770@elf.ucw.cz>
References: <88056F38E9E48644A0F562A38C64FB60067BE61C@scsmsx403.amr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <88056F38E9E48644A0F562A38C64FB60067BE61C@scsmsx403.amr.corp.intel.com>
User-Agent: Mutt/1.5.9i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1337
Lines: 40

Hi!

> >Thanks. The functions are not performance sensitive, so 
> >atomic_t is ok. Here is the new patch.
> >
> 
> The only reason atomic_t will help in this case is because it uses volatile for internal counter, as it does nothing additional on atomic_read(). I don't get what is the issue with using volatile directly. May be I am missing something. Pavel can you elaborate please.
> 

Look at atomic_t again. It is definitely not "just volatile",
definitely not on non-i386 architectures.

> The code here is doing something like this
> 
> While (variable != specific_value)
> 	cpu_relax();

So either do 

while (variable != value)
	mb();

or better use atomic_t.

> So, making variable atomic or declaring variable as volatile should have the same impact here. Note there is only one CPU setting this variable and rest of the CPUs just read it.
> 
> If volatile is not good, probably we need to find something other than atomic as well.
> 

atomic_t works, is used across the kernel, and works on all
architectures. volatile int does not. Do not use it.

							Pavel
-- 
Thanks, Sharp!
-
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/