lkml.org 
[lkml]   [2022]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH 4/4] perf tools: Support reading PERF_FORMAT_LOST
On Tue, Aug 16, 2022 at 03:17:47PM -0700, Namhyung Kim wrote:

SNIP

> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index a7b0931d5137..7753368d70d6 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -65,7 +65,8 @@ struct stack_dump {
>
> struct sample_read_value {
> u64 value;
> - u64 id;
> + u64 id; /* only if PERF_FORMAT_ID */
> + u64 lost; /* only if PERF_FORMAT_LOST */
> };

I was wondering why not to split this patch into smaller piece,
but once you change this struct you break all the places

SNIP

> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1541,7 +1541,7 @@ static int evsel__read_one(struct evsel *evsel, int cpu_map_idx, int thread)
> }
>
> static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
> - u64 val, u64 ena, u64 run)
> + u64 val, u64 ena, u64 run, u64 lost)
> {
> struct perf_counts_values *count;
>
> @@ -1550,6 +1550,7 @@ static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
> count->val = val;
> count->ena = ena;
> count->run = run;
> + count->lost = lost;
>
> perf_counts__set_loaded(counter->counts, cpu_map_idx, thread, true);
> }
> @@ -1558,7 +1559,7 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
> {
> u64 read_format = leader->core.attr.read_format;
> struct sample_read_value *v;
> - u64 nr, ena = 0, run = 0, i;
> + u64 nr, ena = 0, run = 0, lost = 0, i;
>
> nr = *data++;
>
> @@ -1573,16 +1574,25 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
>
> v = (struct sample_read_value *) data;
>
> - evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run);
> + if (read_format & PERF_FORMAT_LOST)
> + lost = v->lost;
> +
> + evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run, lost);
> +
> + v = next_sample_read_value(v, read_format);

oneway of making this simpler here and share with other places
could be adding something like:

for_each_group_data(v, i, nr, read_format) {
}

but not sure how would that turn out, thoughts?

>
> for (i = 1; i < nr; i++) {
> struct evsel *counter;
>
> - counter = evlist__id2evsel(leader->evlist, v[i].id);
> + counter = evlist__id2evsel(leader->evlist, v->id);
> if (!counter)
> return -EINVAL;
>
> - evsel__set_count(counter, cpu_map_idx, thread, v[i].value, ena, run);
> + if (read_format & PERF_FORMAT_LOST)
> + lost = v->lost;
> +
> + evsel__set_count(counter, cpu_map_idx, thread, v->value, ena, run, lost);
> + v = next_sample_read_value(v, read_format);
> }
>
> return 0;
> @@ -2475,16 +2485,21 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
>
> if (data->read.group.nr > max_group_nr)
> return -EFAULT;
> - sz = data->read.group.nr *
> - sizeof(struct sample_read_value);
> +
> + sz = data->read.group.nr * sample_read_value_size(read_format);
> OVERFLOW_CHECK(array, sz, max_size);
> - data->read.group.values =
> - (struct sample_read_value *)array;
> + data->read.group.values = (void *)array;

nit, is this void casting needed?

thanks,
jirka

> array = (void *)array + sz;
> } else {
> OVERFLOW_CHECK_u64(array);
> data->read.one.id = *array;
> array++;
> +
> + if (read_format & PERF_FORMAT_LOST) {
> + OVERFLOW_CHECK_u64(array);
> + data->read.one.lost = *array;
> + array++;
> + }
> }
> }
>

SNIP

\
 
 \ /
  Last update: 2022-08-18 14:07    [W:0.110 / U:2.852 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site