From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752865AbcBVCgd (ORCPT ); Sun, 21 Feb 2016 21:36:33 -0500 Received: from e18.ny.us.ibm.com ([129.33.205.208]:36981 "EHLO e18.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752455AbcBVCgb (ORCPT ); Sun, 21 Feb 2016 21:36:31 -0500 X-IBM-Helo: d01dlp02.pok.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com X-IBM-RcptTo: linux-doc@vger.kernel.org;linux-kernel@vger.kernel.org Date: Sat, 20 Feb 2016 21:25:42 -0800 From: "Paul E. McKenney" To: SeongJae Park Cc: Jonathan Corbet , dhowells@redhat.com, linux-doc , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example Message-ID: <20160221052542.GJ3522@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <1455948068-14221-1-git-send-email-sj38.park@gmail.com> <20160220195722.GG3522@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16022105-0045-0000-0000-00000364B675 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Feb 21, 2016 at 07:50:19AM +0900, SeongJae Park wrote: > On Sun, Feb 21, 2016 at 4:57 AM, Paul E. McKenney > wrote: > > On Sat, Feb 20, 2016 at 03:01:08PM +0900, SeongJae Park wrote: > >> There is wrong comment in example for compiler store omit behavior. It > >> shows example of the problem and than problem solved version code. > >> However, the comment in the solved version is still same with not solved > >> version. Fix the wrong statement with this commit. > >> > >> Signed-off-by: SeongJae Park > > > > Hmmm... The code between the two stores of zero to "a" is intended to > > remain the same in the broken and fixed versions. So the only change > > is from "a = 0" to "WRITE_ONCE(a, 0)". Note that it is some other > > CPU that did the third store to "a". > > Agree, of course. > > > > > Or am I missing your point here? > > My point is about the comment. > I thought the comment in broken version is saying "Below line(a = 0) says > it will store to variable 'a', but it will not in actual because a compiler can > omit it". > However, in fixed version, because the compiler cannot omit the store > now, I thought the comment also should be changed to say the difference > between broken and fixed version. > > If I am understanding anything wrong, please let me know. Hmmm... The intent of the comment is to act as a placeholder for arbitrary code that does not affect the value of "a". The current comment is clearly not doing that for you. Possible changes include: o Adding test to the comment making the intent more clear. o Replacing the comment with a function call, perhaps to does_not_change_a() or some similar name. o Keeping the current comment, but adding a call to something like does_not_change_a() after it. Other thoughts? Thanx, Paul > Thanks, > SeongJae Park > > > > > Thanx, Paul > > > >> --- > >> Documentation/memory-barriers.txt | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt > >> index 061ff29..b4754c7 100644 > >> --- a/Documentation/memory-barriers.txt > >> +++ b/Documentation/memory-barriers.txt > >> @@ -1471,7 +1471,7 @@ of optimizations: > >> wrong guess: > >> > >> WRITE_ONCE(a, 0); > >> - /* Code that does not store to variable a. */ > >> + /* Code that does store to variable a. */ > >> WRITE_ONCE(a, 0); > >> > >> (*) The compiler is within its rights to reorder memory accesses unless > >> -- > >> 1.9.1 > >> > > >