Re: [PATCH v9 07/22] tracing: Add lock-free tracing_map

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jul 16, 2015 at 04:41:45PM -0500, Tom Zanussi wrote:
> On Thu, 2015-07-16 at 19:49 +0200, Peter Zijlstra wrote:
> > On Thu, Jul 16, 2015 at 12:22:40PM -0500, Tom Zanussi wrote:
> > > +	for (i = 0; i < elt->map->n_fields; i++) {
> > > +		atomic64_set(&dup_elt->fields[i].sum,
> > > +			     atomic64_read(&elt->fields[i].sum));
> > > +		dup_elt->fields[i].cmp_fn = elt->fields[i].cmp_fn;
> > > +	}
> > > +
> > > +	return dup_elt;
> > > +}
> > 
> > So there is a lot of atomic64_{set,read}() in this patch set, what kind
> > of magic properties do you assume they have?
> > 
> > Note that atomic*_{set,read}() are weaker than {WRITE,READ}_ONCE(), so
> > if you're assuming they do that, you're mistaken -- although it is on a
> > TODO list someplace to go fix that.
> 
> Not assuming any magic properties - I just need an atomic 64-bit counter
> for the sums and that's the API for setting/reading those.  When reading
> a live trace the exact sum you get is kind of arbitrary..

OK, so atomic64_read() really should provide load consistency (there are
a few archs that lack the READ_ONCE() there).

But the atomic64_set() does not provide store consistency, and in the
above case it looks like the value you're writing is not exposed yet to
concurrency so it doesn't matter how it issues the store.

So as long as you never atomic64_set() a value that is subject to
concurrent modification you should be good.