Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v3 1/3] perf: Allow using AUX data in perf samples | Date | Mon, 04 Nov 2019 12:40:05 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Mon, Oct 28, 2019 at 07:08:18PM +0200, Alexander Shishkin wrote: > >> > @@ -6318,11 +6318,12 @@ static void perf_aux_sample_output(struc >> > >> > /* >> > * Guard against NMI hits inside the critical section; >> > - * see also perf_aux_sample_size(). >> > + * see also perf_prepare_sample_aux(). >> > */ >> > WRITE_ONCE(rb->aux_in_sampling, 1); >> > + barrier(); >> >> Isn't WRITE_ONCE() barrier enough on its own? My thinking was that we >> only need a compiler barrier here, hence the WRITE_ONCE. > > WRITE_ONCE() is a volatile store and (IIRC) the compiler ensures order > against other volatile things, but not in general. > > barrier() OTOH clobbers all of memory and thereby ensures nothing can > get hoised over it. > > Now, the only thing we do inside this region is an indirect call, which > on its own already implies a sync point for as long as the compiler > cannot inline it, so it might be a bit paranoid on my end (I don't think > even LTO can reduce this indirection and cause inlining).
I see what you mean. I was only thinking about not having to order the AUX STOREs vs the rb->aux_in_sampling. Ordering the call itself makes sense.
Thanks, -- Alex
| |