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

[SeongJae Park]

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