Hi Jason & Paul,
although this has already been applied, I have a comment.
On 2017/05/11 23:03:41 +0800, Junchang Wang wrote:
> Signed-off-by: Junchang Wang <junchangwang@xxxxxxxxx>
> ---
> CodeSamples/count/count_stat_eventual.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/CodeSamples/count/count_stat_eventual.c b/CodeSamples/count/count_stat_eventual.c
> index 059ab8b..cbde4aa 100644
> --- a/CodeSamples/count/count_stat_eventual.c
> +++ b/CodeSamples/count/count_stat_eventual.c
> @@ -27,12 +27,12 @@ int stopflag;
>
> void inc_count(void)
> {
> - ACCESS_ONCE(__get_thread_var(counter))++;
> + READ_ONCE(__get_thread_var(counter))++;
This is OK because READ_ONCE() is defined as the same as ACCESS_ONCE()
in CodeSamples. However, the definition in the current Linux kernel
would not permit this.
A read-modify-write access would need both READ_ONCE() and WRITE_ONCE().
However, since "counter" is thread local and updated only by its owner,
we don't need READ_ONCE() here. So:
+ WRITE_ONCE(__get_thread_var(counter), __get_thread_var(counter) + 1);
should have been sufficient.
Problem with this change is that the line gets too wide when applied to
the code snippet in 2-column layout.
Hmm...
Thanks, Akira
> }
>
> unsigned long read_count(void)
> {
> - return ACCESS_ONCE(global_count);
> + return READ_ONCE(global_count);
> }
>
> void *eventual(void *arg)
> @@ -43,8 +43,8 @@ void *eventual(void *arg)
> while (stopflag < 3) {
> sum = 0;
> for_each_thread(t)
> - sum += ACCESS_ONCE(per_thread(counter, t));
> - ACCESS_ONCE(global_count) = sum;
> + sum += READ_ONCE(per_thread(counter, t));
> + WRITE_ONCE(global_count, sum);
> poll(NULL, 0, 1);
> if (stopflag) {
> smp_mb();
>
--
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
