On 2017/07/05 10:32:49 -0700, Paul E. McKenney wrote: > On Wed, Jul 05, 2017 at 08:50:04AM -0700, Paul E. McKenney wrote: >> On Wed, Jul 05, 2017 at 08:40:24AM -0700, Paul E. McKenney wrote: >>> On Wed, Jul 05, 2017 at 11:22:52PM +0900, Akira Yokosawa wrote: >>>> On 2017/07/04 15:21:38 -0700, Paul E. McKenney wrote: >>>>> On Wed, Jul 05, 2017 at 12:23:09AM +0900, Akira Yokosawa wrote: >>>>>> >From 2845eb208a6e63493997de47293a47ef774a9d49 Mon Sep 17 00:00:00 2001 >>>>>> From: Akira Yokosawa <akiyks@xxxxxxxxx> >>>>>> Date: Tue, 4 Jul 2017 23:18:30 +0900 >>>>>> Subject: [PATCH] advsync: Fix store-buffering sequence table >>>>>> >>>>>> Row 6 of the table added in commit 2d5bf8d25a71 ("advsync: Add >>>>>> memory-barriered store-buffering example") needs some context >>>>>> adjustment. >>>>>> >>>>>> Also tweak horizontal spacing of wide tables for one-column layout. >>>>>> Also add a few words to the footnote giving definition of >>>>>> __atomic_thread_fence(). >>>>>> >>>>>> Signed-off-by: Akira Yokosawa <akiyks@xxxxxxxxx> >>>>> >>>>> Good catches! Queued and pushed. I reworded the footnote a bit, so >>>>> please let me know if I overdid it. >>>> >>>> After your changes in commit 036372ac2573 ("advsync: Use gcc's C11-like >>>> intrinsics to avoid data races"), this footnote seems verbose, doesn't it? >>>> >>>> But, I'm not so much a fan of the changes of your commit. >>>> It becomes hard to see the relation of lines in litmus tests and rows >>>> in the tables. Also, those intrinsics have fairly large overheads. >>> >>> They certainly are ugly, no two ways about that! ;-) >>> >>>> How about using "volatile" in thread arg declaration such as the following? >>>> >>>> --- >>>> C C-SB+o-o+o-o >>>> { >>>> } >>>> >>>> P0(volatile int *x0, volatile int *x1) >>>> { >>>> int r2; >>>> >>>> *x0 = 2; >>>> r2 = *x1; >>>> } >>>> >>>> >>>> P1(volatile int *x0, volatile int *x1) >>>> { >>>> int r2; >>>> >>>> *x1 = 2; >>>> r2 = *x0; >>>> } >>>> >>>> exists (1:r2=0 /\ 0:r2=0) >>>> --- >>>> >>>> If all you need is to prevent memory accesses from being optimized away, >>>> they should suffice. But they might be unpopular among kernel community. >>> >>> To say nothing of their unpopularity among the C11 and C++11 >>> communities! Ah, I kind of anticipated it... >>> >>>> I checked the generated C code in cross-compiling mode of litmus7, and >>>> the volatile-ness is reflected there. >>> >>> And it also works just fine without the volatile -- the litmus7 tool >>> does the translation so as to avoid destructive compiler optimizations. >>> >>> I am checking with the litmus7 people to see if there is any way to >>> map identifiers. Some of the other tools support a "-macros" >>> command-line argument, which would allow mapping from "smp_mb()" to >>> "__atomic_thread_fence(__ATOMIC_SEQ_CST)", but not litmus7. >>> >>> 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. Thanks, Akira > > 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