Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v3 1/3] perf: Allow using AUX data in perf samples | Date | Mon, 28 Oct 2019 19:08:18 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> I have the below delta on top of this patch. > > And while I get why we need recursion protection for pmu::snapshot_aux, > I'm a little puzzled on why it is over the padding, that is, why isn't > the whole of aux_in_sampling inside (the newly minted) > perf_pmu_snapshot_aux() ?
No reason. Too long staring at that code by myself.
> --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -6237,7 +6237,7 @@ perf_output_sample_ustack(struct perf_ou > } > } > > -static unsigned long perf_aux_sample_size(struct perf_event *event, > +static unsigned long perf_prepare_sample_aux(struct perf_event *event, > struct perf_sample_data *data, > size_t size) > { > @@ -6275,9 +6275,9 @@ static unsigned long perf_aux_sample_siz > return data->aux_size; > } > > -long perf_pmu_aux_sample_output(struct perf_event *event, > - struct perf_output_handle *handle, > - unsigned long size) > +long perf_pmu_snapshot_aux(struct perf_event *event, > + struct perf_output_handle *handle, > + unsigned long size)
That makes more sense indeed.
> { > unsigned long flags; > long ret; > @@ -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.
Thanks, -- Alex
| |