Re: [RFC PATCH 4/4] CodeSamples/defer: Add compiler barriers in gettimestampmp.c

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, May 30, 2017 at 07:18:51AM +0900, Akira Yokosawa wrote:
> >From fd5fb9afb6b99b15370a9823b42fa2a92342b92a Mon Sep 17 00:00:00 2001
> From: Akira Yokosawa <akiyks@gmail.com>
> Date: Sun, 28 May 2017 15:42:45 +0900
> Subject: [RFC PATCH 4/4] CodeSamples/defer: Add compiler barriers in gettimestampmp.c
> 
> They ensure curtimestamp is read and written once in every
> iteration.
> 
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>

I was actually more or less OK with this one, but it does not apply
cleanly without 3/4.

Nevertheless, given that it needs change anyway, would it be better to
replace the ++ with a WRITE_ONCE and capture the value in the "while"
expression?

							Thanx, Paul

> ---
>  CodeSamples/defer/gettimestampmp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/CodeSamples/defer/gettimestampmp.c b/CodeSamples/defer/gettimestampmp.c
> index e794e82..7012caa 100644
> --- a/CodeSamples/defer/gettimestampmp.c
> +++ b/CodeSamples/defer/gettimestampmp.c
> @@ -32,7 +32,7 @@ void *collect_timestamps(void *mask_in)
>  	intptr_t mask = (intptr_t)mask_in;
> 
>  	while (curtimestamp < MAX_TIMESTAMPS) {
> -		while ((curtimestamp & CURTIMESTAMP_MASK) != mask)
> +		while ((READ_ONCE(curtimestamp) & CURTIMESTAMP_MASK) != mask)
>  			continue;
>  		if (curtimestamp >= MAX_TIMESTAMPS)
>  			break;
> @@ -40,6 +40,7 @@ void *collect_timestamps(void *mask_in)
>  		/* Don't need memory barrier -- no other shared vars!!! */
> 
>  		ts[curtimestamp++] = get_timestamp();
> +		barrier();
>  	}
>  	smp_mb();
>  	return (NULL);
> -- 
> 2.7.4
> 
>