linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Documentation/memory-barriers: fix wrong comment in example
@ 2016-02-20  6:01 SeongJae Park
  2016-02-20 19:57 ` Paul E. McKenney
  0 siblings, 1 reply; 10+ messages in thread
From: SeongJae Park @ 2016-02-20  6:01 UTC (permalink / raw)
  To: corbet, dhowells, paulmck; +Cc: linux-doc, linux-kernel, SeongJae Park

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@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-20  6:01 [PATCH] Documentation/memory-barriers: fix wrong comment in example SeongJae Park
@ 2016-02-20 19:57 ` Paul E. McKenney
  2016-02-20 22:50   ` SeongJae Park
  0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2016-02-20 19:57 UTC (permalink / raw)
  To: SeongJae Park; +Cc: corbet, dhowells, linux-doc, linux-kernel

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@gmail.com>

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

Or am I missing your point here?

							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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-20 19:57 ` Paul E. McKenney
@ 2016-02-20 22:50   ` SeongJae Park
  2016-02-21  5:25     ` Paul E. McKenney
  0 siblings, 1 reply; 10+ messages in thread
From: SeongJae Park @ 2016-02-20 22:50 UTC (permalink / raw)
  To: Paul McKenney; +Cc: Jonathan Corbet, dhowells, linux-doc, linux-kernel

On Sun, Feb 21, 2016 at 4:57 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> 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@gmail.com>
>
> 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
>>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-20 22:50   ` SeongJae Park
@ 2016-02-21  5:25     ` Paul E. McKenney
  2016-02-21  6:33       ` SeongJae Park
  2016-02-22 10:01       ` David Howells
  0 siblings, 2 replies; 10+ messages in thread
From: Paul E. McKenney @ 2016-02-21  5:25 UTC (permalink / raw)
  To: SeongJae Park; +Cc: Jonathan Corbet, dhowells, linux-doc, linux-kernel

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
> <paulmck@linux.vnet.ibm.com> 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@gmail.com>
> >
> > 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
> >>
> >
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-21  5:25     ` Paul E. McKenney
@ 2016-02-21  6:33       ` SeongJae Park
  2016-02-22 10:01       ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: SeongJae Park @ 2016-02-21  6:33 UTC (permalink / raw)
  To: Paul McKenney; +Cc: Jonathan Corbet, dhowells, linux-doc, linux-kernel

On Sun, Feb 21, 2016 at 2:25 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> 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
>> <paulmck@linux.vnet.ibm.com> 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@gmail.com>
>> >
>> > 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?

Ah, now I understood the original intent.  Thank you for the kind explanation.
I think your third option will be most helpful for me. How about the
patch below?

BTW, the problem looks trivial rather than critical.
If you think so, feel free to ignore my patch, please.


Thanks,
SeongJae Park


========================== >3 ==========================
>From 77e0b1c77d64c358b329b097cffcdacd2c484867 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj38.park@gmail.com>
Date: Sun, 21 Feb 2016 15:18:16 +0900
Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
 example

Comments of examples about compiler store omit in memory-barriers.txt is
about code that could be possible at that point.  However, someone could
interpret the comment as an explanation about below line.  This commit
exploits the intent more explicitly by adding a function call below the
comment.

Signed-off-by: SeongJae Park <sj38.park@gmail.com>
---
 Documentation/memory-barriers.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/memory-barriers.txt
b/Documentation/memory-barriers.txt
index 904ee42..3a17d66 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1460,6 +1460,7 @@ of optimizations:

  a = 0;
  /* Code that does not store to variable a. */
+ does_not_change_a();
  a = 0;

      The compiler sees that the value of variable 'a' is already zero, so
@@ -1472,6 +1473,7 @@ of optimizations:

  WRITE_ONCE(a, 0);
  /* Code that does not store to variable a. */
+ does_not_change_a();
  WRITE_ONCE(a, 0);

  (*) The compiler is within its rights to reorder memory accesses unless
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-21  5:25     ` Paul E. McKenney
  2016-02-21  6:33       ` SeongJae Park
@ 2016-02-22 10:01       ` David Howells
  2016-02-22 11:08         ` SeongJae Park
  2016-02-22 11:16         ` David Howells
  1 sibling, 2 replies; 10+ messages in thread
From: David Howells @ 2016-02-22 10:01 UTC (permalink / raw)
  To: SeongJae Park
  Cc: dhowells, Paul McKenney, Jonathan Corbet, linux-doc, linux-kernel

SeongJae Park <sj38.park@gmail.com> wrote:

>   a = 0;
>   /* Code that does not store to variable a. */
> + does_not_change_a();
>   a = 0;

Since it's not actually code that's meant to be executed, you could make it:

	a = 0;
	... code that does not store to variable a ...
	a = 0;

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-22 10:01       ` David Howells
@ 2016-02-22 11:08         ` SeongJae Park
  2016-02-22 11:16         ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: SeongJae Park @ 2016-02-22 11:08 UTC (permalink / raw)
  To: David Howells
  Cc: SeongJae Park, Paul McKenney, Jonathan Corbet, linux-doc, linux-kernel



On Mon, 22 Feb 2016, David Howells wrote:

> SeongJae Park <sj38.park@gmail.com> wrote:
>
>>   a = 0;
>>   /* Code that does not store to variable a. */
>> + does_not_change_a();
>>   a = 0;
>
> Since it's not actually code that's meant to be executed, you could make it:
>
> 	a = 0;
> 	... code that does not store to variable a ...
> 	a = 0;

I selected Paul's third option because the function could be noop (In this
case, it doesn't break the original meaning) and it makes the code looks
complete.
However, your suggestion looks much better than the comment, too.

So, I am attaching a patch that applying your suggestion below.


=============================== >3 ====================================
>From f7b5677790771599f418f1d95536935be971ae86 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj38.park@gmail.com>
Date: Mon, 22 Feb 2016 19:26:18 +0900
Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
  example

Comments of examples about compiler store omit in memory-barriers.txt is
about code that could be possible at that point.  However, someone could
interpret the comment as an explanation about below line.  This commit
exploits the intent more explicitly by changing the comment to be seems
like a possible code rather than explanation about below line.

Signed-off-by: SeongJae Park <sj38.park@gmail.com>
---
  Documentation/memory-barriers.txt | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/memory-barriers.txt 
b/Documentation/memory-barriers.txt
index 904ee42..dc66351 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1459,7 +1459,7 @@ of optimizations:
       the following:

         a = 0;
-       /* Code that does not store to variable a. */
+       ... Code that does not store to variable a ...
         a = 0;

       The compiler sees that the value of variable 'a' is already zero, so
@@ -1471,7 +1471,7 @@ of optimizations:
       wrong guess:

         WRITE_ONCE(a, 0);
-       /* Code that does not store to variable a. */
+       ... Code that does not store to variable a ...
         WRITE_ONCE(a, 0);

   (*) The compiler is within its rights to reorder memory accesses unless
--
1.9.1



>
> David
>

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-22 10:01       ` David Howells
  2016-02-22 11:08         ` SeongJae Park
@ 2016-02-22 11:16         ` David Howells
  2016-02-22 16:33           ` Paul E. McKenney
  1 sibling, 1 reply; 10+ messages in thread
From: David Howells @ 2016-02-22 11:16 UTC (permalink / raw)
  To: SeongJae Park
  Cc: dhowells, Paul McKenney, Jonathan Corbet, linux-doc, linux-kernel

SeongJae Park <sj38.park@gmail.com> wrote:

> From f7b5677790771599f418f1d95536935be971ae86 Mon Sep 17 00:00:00 2001
> From: SeongJae Park <sj38.park@gmail.com>
> Date: Mon, 22 Feb 2016 19:26:18 +0900
> Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
>  example
> 
> Comments of examples about compiler store omit in memory-barriers.txt is
> about code that could be possible at that point.  However, someone could
> interpret the comment as an explanation about below line.  This commit
> exploits the intent more explicitly by changing the comment to be seems
> like a possible code rather than explanation about below line.
> 
> Signed-off-by: SeongJae Park <sj38.park@gmail.com>
> ---
>  Documentation/memory-barriers.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/memory-barriers.txt
> b/Documentation/memory-barriers.txt
> index 904ee42..dc66351 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1459,7 +1459,7 @@ of optimizations:
>       the following:
> 
>         a = 0;
> -       /* Code that does not store to variable a. */
> +       ... Code that does not store to variable a ...
>         a = 0;
> 
>       The compiler sees that the value of variable 'a' is already zero, so
> @@ -1471,7 +1471,7 @@ of optimizations:
>       wrong guess:
> 
>         WRITE_ONCE(a, 0);
> -       /* Code that does not store to variable a. */
> +       ... Code that does not store to variable a ...
>         WRITE_ONCE(a, 0);
> 
>   (*) The compiler is within its rights to reorder memory accesses unless

Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-22 11:16         ` David Howells
@ 2016-02-22 16:33           ` Paul E. McKenney
  2016-02-22 21:45             ` SeongJae Park
  0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2016-02-22 16:33 UTC (permalink / raw)
  To: David Howells; +Cc: SeongJae Park, Jonathan Corbet, linux-doc, linux-kernel

On Mon, Feb 22, 2016 at 11:16:39AM +0000, David Howells wrote:
> SeongJae Park <sj38.park@gmail.com> wrote:
> 
> > From f7b5677790771599f418f1d95536935be971ae86 Mon Sep 17 00:00:00 2001
> > From: SeongJae Park <sj38.park@gmail.com>
> > Date: Mon, 22 Feb 2016 19:26:18 +0900
> > Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
> >  example
> > 
> > Comments of examples about compiler store omit in memory-barriers.txt is
> > about code that could be possible at that point.  However, someone could
> > interpret the comment as an explanation about below line.  This commit
> > exploits the intent more explicitly by changing the comment to be seems
> > like a possible code rather than explanation about below line.
> > 
> > Signed-off-by: SeongJae Park <sj38.park@gmail.com>
> > ---
> >  Documentation/memory-barriers.txt | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/memory-barriers.txt
> > b/Documentation/memory-barriers.txt
> > index 904ee42..dc66351 100644
> > --- a/Documentation/memory-barriers.txt
> > +++ b/Documentation/memory-barriers.txt
> > @@ -1459,7 +1459,7 @@ of optimizations:
> >       the following:
> > 
> >         a = 0;
> > -       /* Code that does not store to variable a. */
> > +       ... Code that does not store to variable a ...
> >         a = 0;
> > 
> >       The compiler sees that the value of variable 'a' is already zero, so
> > @@ -1471,7 +1471,7 @@ of optimizations:
> >       wrong guess:
> > 
> >         WRITE_ONCE(a, 0);
> > -       /* Code that does not store to variable a. */
> > +       ... Code that does not store to variable a ...
> >         WRITE_ONCE(a, 0);
> > 
> >   (*) The compiler is within its rights to reorder memory accesses unless
> 
> Acked-by: David Howells <dhowells@redhat.com>

Thank you both!  Patch with updated commit log below, please let me know
if you have any objections to the changes.

							Thanx, Paul

------------------------------------------------------------------------

commit 0a41feb6ab4da3218192e2cde1a54fcc5d8f5658
Author: SeongJae Park <sj38.park@gmail.com>
Date:   Mon Feb 22 08:28:29 2016 -0800

    documentation: Clarify compiler store-fusion example
    
    The compiler store-fusion example in memory-barriers.txt uses a C
    comment to represent arbitrary code that does not update a given
    variable.  Unfortunately, someone could reasonably interpret the
    comment as instead referring to the following line of code.  This
    commit therefore replaces the comment with a string that more
    clearly represents the arbitrary code.
    
    Signed-off-by: SeongJae Park <sj38.park@gmail.com>
    Acked-by: David Howells <dhowells@redhat.com>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 8367d393cba2..3729cbe60e41 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1550,7 +1550,7 @@ of optimizations:
      the following:
 
 	a = 0;
-	/* Code that does not store to variable a. */
+	... Code that does not store to variable a ...
 	a = 0;
 
      The compiler sees that the value of variable 'a' is already zero, so
@@ -1562,7 +1562,7 @@ of optimizations:
      wrong guess:
 
 	WRITE_ONCE(a, 0);
-	/* Code that does not store to variable a. */
+	... Code that does not store to variable a ...
 	WRITE_ONCE(a, 0);
 
  (*) The compiler is within its rights to reorder memory accesses unless

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example
  2016-02-22 16:33           ` Paul E. McKenney
@ 2016-02-22 21:45             ` SeongJae Park
  0 siblings, 0 replies; 10+ messages in thread
From: SeongJae Park @ 2016-02-22 21:45 UTC (permalink / raw)
  To: Paul McKenney; +Cc: David Howells, Jonathan Corbet, linux-doc, linux-kernel

On Tue, Feb 23, 2016 at 1:33 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Mon, Feb 22, 2016 at 11:16:39AM +0000, David Howells wrote:
>> SeongJae Park <sj38.park@gmail.com> wrote:
>>
>> > From f7b5677790771599f418f1d95536935be971ae86 Mon Sep 17 00:00:00 2001
>> > From: SeongJae Park <sj38.park@gmail.com>
>> > Date: Mon, 22 Feb 2016 19:26:18 +0900
>> > Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
>> >  example
>> >
>> > Comments of examples about compiler store omit in memory-barriers.txt is
>> > about code that could be possible at that point.  However, someone could
>> > interpret the comment as an explanation about below line.  This commit
>> > exploits the intent more explicitly by changing the comment to be seems
>> > like a possible code rather than explanation about below line.
>> >
>> > Signed-off-by: SeongJae Park <sj38.park@gmail.com>
>> > ---
>> >  Documentation/memory-barriers.txt | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/Documentation/memory-barriers.txt
>> > b/Documentation/memory-barriers.txt
>> > index 904ee42..dc66351 100644
>> > --- a/Documentation/memory-barriers.txt
>> > +++ b/Documentation/memory-barriers.txt
>> > @@ -1459,7 +1459,7 @@ of optimizations:
>> >       the following:
>> >
>> >         a = 0;
>> > -       /* Code that does not store to variable a. */
>> > +       ... Code that does not store to variable a ...
>> >         a = 0;
>> >
>> >       The compiler sees that the value of variable 'a' is already zero, so
>> > @@ -1471,7 +1471,7 @@ of optimizations:
>> >       wrong guess:
>> >
>> >         WRITE_ONCE(a, 0);
>> > -       /* Code that does not store to variable a. */
>> > +       ... Code that does not store to variable a ...
>> >         WRITE_ONCE(a, 0);
>> >
>> >   (*) The compiler is within its rights to reorder memory accesses unless
>>
>> Acked-by: David Howells <dhowells@redhat.com>
>
> Thank you both!  Patch with updated commit log below, please let me know
> if you have any objections to the changes.

Looks good to me :)


Thanks,
SeongJae Park

>
>                                                         Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 0a41feb6ab4da3218192e2cde1a54fcc5d8f5658
> Author: SeongJae Park <sj38.park@gmail.com>
> Date:   Mon Feb 22 08:28:29 2016 -0800
>
>     documentation: Clarify compiler store-fusion example
>
>     The compiler store-fusion example in memory-barriers.txt uses a C
>     comment to represent arbitrary code that does not update a given
>     variable.  Unfortunately, someone could reasonably interpret the
>     comment as instead referring to the following line of code.  This
>     commit therefore replaces the comment with a string that more
>     clearly represents the arbitrary code.
>
>     Signed-off-by: SeongJae Park <sj38.park@gmail.com>
>     Acked-by: David Howells <dhowells@redhat.com>
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 8367d393cba2..3729cbe60e41 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1550,7 +1550,7 @@ of optimizations:
>       the following:
>
>         a = 0;
> -       /* Code that does not store to variable a. */
> +       ... Code that does not store to variable a ...
>         a = 0;
>
>       The compiler sees that the value of variable 'a' is already zero, so
> @@ -1562,7 +1562,7 @@ of optimizations:
>       wrong guess:
>
>         WRITE_ONCE(a, 0);
> -       /* Code that does not store to variable a. */
> +       ... Code that does not store to variable a ...
>         WRITE_ONCE(a, 0);
>
>   (*) The compiler is within its rights to reorder memory accesses unless
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-02-22 21:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-20  6:01 [PATCH] Documentation/memory-barriers: fix wrong comment in example SeongJae Park
2016-02-20 19:57 ` Paul E. McKenney
2016-02-20 22:50   ` SeongJae Park
2016-02-21  5:25     ` Paul E. McKenney
2016-02-21  6:33       ` SeongJae Park
2016-02-22 10:01       ` David Howells
2016-02-22 11:08         ` SeongJae Park
2016-02-22 11:16         ` David Howells
2016-02-22 16:33           ` Paul E. McKenney
2016-02-22 21:45             ` SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).