On Thu, Jul 06, 2017 at 09:09:05PM +0900, Akira Yokosawa wrote:
> On 2017/07/06 7:40 +0900, Akira Yokosawa wrote:
> > On 2017/07/06 7:32, Paul E. McKenney wrote:
> >> On Thu, Jul 06, 2017 at 07:15:24AM +0900, Akira Yokosawa wrote:
> >>> On 2017/07/05 10:32:49 -0700, Paul E. McKenney wrote:
> >>
> >> [ . . . ]
> >>
> >>>>>> So I cannot go with "volatile", but let's see if I can do something
> >>>>>> better than the gcc intrinsics.
> >>>>>
> >>>>> And if the litmus7 guys don't have anything for me, I can always write a
> >>>>> script to do the conversions. So either way, we will have kernel-style
> >>>>> notation at some point. ;-)
> >>>>
> >>>> And it turns out that the contents of a second "{}" in a litmus7 test
> >>>> is pulled directly into the output code, so an easy change. ;-)
> >>>
> >>> Good news!
> >>>
> >>> So __atomic_thread_fence() will also be replaced with smp_mb() in
> >>> the litmus tests? That will reduce the width of the tables and eliminate
> >>> the need for the hspece adjustments in one-column layout.
> >>
> >> Oops, I forgot to push! Done now.
> >>
> >> And yes, there is now smp_mb() in the litmus tests and the tables.
> >
> > I'll take a look later.
> > As I mentioned earlier, __atomic_load_n() and __atomic_store_n() seem
> > to have fairly large overheads (on x86). You might want to see the changes
> > in the resulting statistics and reflect them in the text.
>
> Hi Paul,
>
> The definition of READ_ONCE() and WRITE_ONCE() given in the updated litmus
> tests is inconsistent with kernel API. They are accepting pointers such
> as x0 and x1.
>
> In the article at LWN (https://lwn.net/Articles/720550/), litmus tests use the
> same definition as kernel API. They take the value expression such as *x0 and *x1.
>
> My preference is to accept pointers as arguments as is the way in C functions,
> but the kernel community have chosen otherwise. We should be consist here.
>
> So the definition should be:
>
> ---
> #define READ_ONCE(x) __atomic_load_n(&(x), __ATOMIC_RELAXED)
> #define WRITE_ONCE(x, v) __atomic_store_n(&(x), (v), __ATOMIC_RELAXED)
> #define smp_mb() __atomic_thread_fence(__ATOMIC_SEQ_CST)
> ---
>
> Or am I misreading?
No, you are correct! I added a commit to fix this, and also added
a disclaimer as you suggested in your earlier email.
Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe perfbook" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
