Re: [PATCH] advsync: Fix store-buffering sequence table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux