lkml.org 
[lkml]   [2019]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v3 1/3] perf: Allow using AUX data in perf samples
On Fri, Oct 25, 2019 at 05:08:33PM +0300, Alexander Shishkin wrote:
> +static void perf_aux_sample_output(struct perf_event *event,
> + struct perf_output_handle *handle,
> + struct perf_sample_data *data)
> +{
> + struct perf_event *sampler = event->aux_event;
> + unsigned long pad;
> + struct ring_buffer *rb;
> + long size;
> +
> + if (WARN_ON_ONCE(!sampler || !data->aux_size))
> + return;
> +
> + rb = ring_buffer_get(sampler->parent ? sampler->parent : sampler);
> + if (!rb)
> + return;
> +
> + /*
> + * Guard against NMI hits inside the critical section;
> + * see also perf_aux_sample_size().
> + */
> + WRITE_ONCE(rb->aux_in_sampling, 1);
> +
> + size = perf_pmu_aux_sample_output(sampler, handle, data->aux_size);
> +
> + /*
> + * An error here means that perf_output_copy() failed (returned a
> + * non-zero surplus that it didn't copy), which in its current
> + * enlightened implementation is not possible. If that changes, we'd
> + * like to know.
> + */
> + if (WARN_ON_ONCE(size < 0))
> + goto out_clear;
> +
> + /*
> + * The pad comes from ALIGN()ing data->aux_size up to u64 in
> + * perf_aux_sample_size(), so should not be more than that.
> + */
> + pad = data->aux_size - size;
> + if (WARN_ON_ONCE(pad >= sizeof(u64)))
> + pad = 8;
> +
> + if (pad) {
> + u64 zero = 0;
> + perf_output_copy(handle, &zero, pad);
> + }
> +
> +out_clear:
> + WRITE_ONCE(rb->aux_in_sampling, 0);
> +
> + ring_buffer_put(rb);
> +}

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() ?

---
--- 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)
{
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();

- size = perf_pmu_aux_sample_output(sampler, handle, data->aux_size);
+ size = perf_pmu_snapshot_aux(sampler, handle, data->aux_size);

/*
* An error here means that perf_output_copy() failed (returned a
@@ -6335,7 +6336,7 @@ static void perf_aux_sample_output(struc

/*
* The pad comes from ALIGN()ing data->aux_size up to u64 in
- * perf_aux_sample_size(), so should not be more than that.
+ * perf_prepare_sample_aux(), so should not be more than that.
*/
pad = data->aux_size - size;
if (WARN_ON_ONCE(pad >= sizeof(u64)))
@@ -6347,6 +6348,7 @@ static void perf_aux_sample_output(struc
}

out_clear:
+ barrier();
WRITE_ONCE(rb->aux_in_sampling, 0);

ring_buffer_put(rb);
@@ -6881,7 +6883,7 @@ void perf_prepare_sample(struct perf_eve
size = min_t(size_t, U16_MAX - header->size,
event->attr.aux_sample_size);
size = rounddown(size, 8);
- size = perf_aux_sample_size(event, data, size);
+ size = perf_prepare_sample_aux(event, data, size);

WARN_ON_ONCE(size + header->size > U16_MAX);
header->size += size;
\
 
 \ /
  Last update: 2019-10-28 17:28    [W:0.157 / U:26.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site