lkml.org 
[lkml]   [2022]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 4/4] perf tools: Support reading PERF_FORMAT_LOST
Hi Jiri,

On Thu, Aug 18, 2022 at 5:04 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> 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

Right.. I'd like to do so but couldn't.. :)

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

Looks good. Let me try. :)


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

Well.. the array is a pointer to u64 so the casting is needed.
But it's an unrelated change, can be dropped.

Thanks,
Namhyung


>
> > 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 19:00    [W:0.065 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site