Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example

From: SeongJae Park
Date: Sat Feb 20 2016 - 17:51:21 EST


On Sun, Feb 21, 2016 at 4:57 AM, Paul E. McKenney
<paulmck@xxxxxxxxxxxxxxxxxx> 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 <sj38.park@xxxxxxxxx>
>
> 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.


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
>>
>