Re: [PATCH 4/7] perf jevents: Modify match field

From: Ian Rogers
Date: Wed May 11 2022 - 13:27:16 EST

Next message: David Hildenbrand: "Re: [PATCH v4 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration"
Previous message: Mark Brown: "Re: [PATCH] kselftest/arm64: bti: force static linking"
In reply to: Xing Zhengjun: "Re: [PATCH 4/7] perf jevents: Modify match field"
Next in thread: Ian Rogers: "[PATCH 5/7] perf jevents: Add python converter script"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, May 11, 2022 at 12:43 AM Xing Zhengjun
<zhengjun.xing@xxxxxxxxxxxxxxx> wrote:
>
>
>
> On 5/11/2022 3:01 PM, Ian Rogers wrote:
> > The match_field function looks for json values to append to the event
> > string. As the C code processes these in order the output order matches
> > that in the json dictionary. Python json readers read the entire
> > dictionary and lose the ordering. To make the python and C output
> > comparable makee the C code first read the extra fields then append them
>
> The "makee" should be a typo.

Good catch, fixed in v2. Thanks,
Ian

> > to the event in an order not determined by their order in the file.
> >
> > Modify the pmu-events test so that test expectations match the new
> > order.
> >
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > ---
> > tools/perf/pmu-events/jevents.c | 82 ++++++++++++++++++++-------------
> > tools/perf/tests/pmu-events.c | 24 +++++-----
> > 2 files changed, 62 insertions(+), 44 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> > index e1f7c7afd435..cee61c4ed59e 100644
> > --- a/tools/perf/pmu-events/jevents.c
> > +++ b/tools/perf/pmu-events/jevents.c
> > @@ -207,21 +207,6 @@ static struct msrmap {
> > { NULL, NULL }
> > };
> >
> > -static struct field {
> > - const char *field;
> > - const char *kernel;
> > -} fields[] = {
> > - { "UMask", "umask=" },
> > - { "CounterMask", "cmask=" },
> > - { "Invert", "inv=" },
> > - { "AnyThread", "any=" },
> > - { "EdgeDetect", "edge=" },
> > - { "SampleAfterValue", "period=" },
> > - { "FCMask", "fc_mask=" },
> > - { "PortMask", "ch_mask=" },
> > - { NULL, NULL }
> > -};
> > -
> > static void cut_comma(char *map, jsmntok_t *newval)
> > {
> > int i;
> > @@ -233,21 +218,6 @@ static void cut_comma(char *map, jsmntok_t *newval)
> > }
> > }
> >
> > -static int match_field(char *map, jsmntok_t *field, int nz,
> > - char **event, jsmntok_t *val)
> > -{
> > - struct field *f;
> > - jsmntok_t newval = *val;
> > -
> > - for (f = fields; f->field; f++)
> > - if (json_streq(map, field, f->field) && nz) {
> > - cut_comma(map, &newval);
> > - addfield(map, event, ",", f->kernel, &newval);
> > - return 1;
> > - }
> > - return 0;
> > -}
> > -
> > static struct msrmap *lookup_msr(char *map, jsmntok_t *val)
> > {
> > jsmntok_t newval = *val;
> > @@ -581,6 +551,14 @@ static int json_events(const char *fn,
> > jsmntok_t *precise = NULL;
> > jsmntok_t *obj = tok++;
> > bool configcode_present = false;
> > + char *umask = NULL;
> > + char *cmask = NULL;
> > + char *inv = NULL;
> > + char *any = NULL;
> > + char *edge = NULL;
> > + char *period = NULL;
> > + char *fc_mask = NULL;
> > + char *ch_mask = NULL;
> >
> > EXPECT(obj->type == JSMN_OBJECT, obj, "expected object");
> > for (j = 0; j < obj->size; j += 2) {
> > @@ -596,8 +574,23 @@ static int json_events(const char *fn,
> > "Expected string value");
> >
> > nz = !json_streq(map, val, "0");
> > - if (match_field(map, field, nz, &event, val)) {
> > - /* ok */
> > + /* match_field */
> > + if (json_streq(map, field, "UMask") && nz) {
> > + addfield(map, &umask, "", "umask=", val);
> > + } else if (json_streq(map, field, "CounterMask") && nz) {
> > + addfield(map, &cmask, "", "cmask=", val);
> > + } else if (json_streq(map, field, "Invert") && nz) {
> > + addfield(map, &inv, "", "inv=", val);
> > + } else if (json_streq(map, field, "AnyThread") && nz) {
> > + addfield(map, &any, "", "any=", val);
> > + } else if (json_streq(map, field, "EdgeDetect") && nz) {
> > + addfield(map, &edge, "", "edge=", val);
> > + } else if (json_streq(map, field, "SampleAfterValue") && nz) {
> > + addfield(map, &period, "", "period=", val);
> > + } else if (json_streq(map, field, "FCMask") && nz) {
> > + addfield(map, &fc_mask, "", "fc_mask=", val);
> > + } else if (json_streq(map, field, "PortMask") && nz) {
> > + addfield(map, &ch_mask, "", "ch_mask=", val);
> > } else if (json_streq(map, field, "EventCode")) {
> > char *code = NULL;
> > addfield(map, &code, "", "", val);
> > @@ -690,6 +683,23 @@ static int json_events(const char *fn,
> > else
> > snprintf(buf, sizeof buf, "event=%#llx", eventcode);
> > addfield(map, &event, ",", buf, NULL);
> > + if (any)
> > + addfield(map, &event, ",", any, NULL);
> > + if (ch_mask)
> > + addfield(map, &event, ",", ch_mask, NULL);
> > + if (cmask)
> > + addfield(map, &event, ",", cmask, NULL);
> > + if (edge)
> > + addfield(map, &event, ",", edge, NULL);
> > + if (fc_mask)
> > + addfield(map, &event, ",", fc_mask, NULL);
> > + if (inv)
> > + addfield(map, &event, ",", inv, NULL);
> > + if (period)
> > + addfield(map, &event, ",", period, NULL);
> > + if (umask)
> > + addfield(map, &event, ",", umask, NULL);
> > +
> > if (je.desc && extra_desc)
> > addfield(map, &je.desc, " ", extra_desc, NULL);
> > if (je.long_desc && extra_desc)
> > @@ -718,6 +728,14 @@ static int json_events(const char *fn,
> > je.event = real_event(je.name, event);
> > err = func(data, &je);
> > free_strings:
> > + free(umask);
> > + free(cmask);
> > + free(inv);
> > + free(any);
> > + free(edge);
> > + free(period);
> > + free(fc_mask);
> > + free(ch_mask);
> > free(event);
> > free(je.desc);
> > free(je.name);
> > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> > index b74c6ef59e51..f13368569d8b 100644
> > --- a/tools/perf/tests/pmu-events.c
> > +++ b/tools/perf/tests/pmu-events.c
> > @@ -63,33 +63,33 @@ static const struct perf_pmu_test_event bp_l2_btb_correct = {
> > static const struct perf_pmu_test_event segment_reg_loads_any = {
> > .event = {
> > .name = "segment_reg_loads.any",
> > - .event = "umask=0x80,period=200000,event=0x6",
> > + .event = "event=0x6,period=200000,umask=0x80",
> > .desc = "Number of segment register loads",
> > .topic = "other",
> > },
> > - .alias_str = "umask=0x80,period=0x30d40,event=0x6",
> > + .alias_str = "event=0x6,period=0x30d40,umask=0x80",
> > .alias_long_desc = "Number of segment register loads",
> > };
> >
> > static const struct perf_pmu_test_event dispatch_blocked_any = {
> > .event = {
> > .name = "dispatch_blocked.any",
> > - .event = "umask=0x20,period=200000,event=0x9",
> > + .event = "event=0x9,period=200000,umask=0x20",
> > .desc = "Memory cluster signals to block micro-op dispatch for any reason",
> > .topic = "other",
> > },
> > - .alias_str = "umask=0x20,period=0x30d40,event=0x9",
> > + .alias_str = "event=0x9,period=0x30d40,umask=0x20",
> > .alias_long_desc = "Memory cluster signals to block micro-op dispatch for any reason",
> > };
> >
> > static const struct perf_pmu_test_event eist_trans = {
> > .event = {
> > .name = "eist_trans",
> > - .event = "umask=0x0,period=200000,event=0x3a",
> > + .event = "event=0x3a,period=200000,umask=0x0",
> > .desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions",
> > .topic = "other",
> > },
> > - .alias_str = "umask=0,period=0x30d40,event=0x3a",
> > + .alias_str = "event=0x3a,period=0x30d40,umask=0",
> > .alias_long_desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions",
> > };
> >
> > @@ -132,13 +132,13 @@ static const struct perf_pmu_test_event uncore_hisi_ddrc_flux_wcmd = {
> > static const struct perf_pmu_test_event unc_cbo_xsnp_response_miss_eviction = {
> > .event = {
> > .name = "unc_cbo_xsnp_response.miss_eviction",
> > - .event = "umask=0x81,event=0x22",
> > + .event = "event=0x22,umask=0x81",
> > .desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core. Unit: uncore_cbox ",
> > .topic = "uncore",
> > .long_desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core",
> > .pmu = "uncore_cbox",
> > },
> > - .alias_str = "umask=0x81,event=0x22",
> > + .alias_str = "event=0x22,umask=0x81",
> > .alias_long_desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core",
> > .matching_pmu = "uncore_cbox_0",
> > };
> > @@ -146,13 +146,13 @@ static const struct perf_pmu_test_event unc_cbo_xsnp_response_miss_eviction = {
> > static const struct perf_pmu_test_event uncore_hyphen = {
> > .event = {
> > .name = "event-hyphen",
> > - .event = "umask=0x00,event=0xe0",
> > + .event = "event=0xe0,umask=0x00",
> > .desc = "UNC_CBO_HYPHEN. Unit: uncore_cbox ",
> > .topic = "uncore",
> > .long_desc = "UNC_CBO_HYPHEN",
> > .pmu = "uncore_cbox",
> > },
> > - .alias_str = "umask=0,event=0xe0",
> > + .alias_str = "event=0xe0,umask=0",
> > .alias_long_desc = "UNC_CBO_HYPHEN",
> > .matching_pmu = "uncore_cbox_0",
> > };
> > @@ -160,13 +160,13 @@ static const struct perf_pmu_test_event uncore_hyphen = {
> > static const struct perf_pmu_test_event uncore_two_hyph = {
> > .event = {
> > .name = "event-two-hyph",
> > - .event = "umask=0x00,event=0xc0",
> > + .event = "event=0xc0,umask=0x00",
> > .desc = "UNC_CBO_TWO_HYPH. Unit: uncore_cbox ",
> > .topic = "uncore",
> > .long_desc = "UNC_CBO_TWO_HYPH",
> > .pmu = "uncore_cbox",
> > },
> > - .alias_str = "umask=0,event=0xc0",
> > + .alias_str = "event=0xc0,umask=0",
> > .alias_long_desc = "UNC_CBO_TWO_HYPH",
> > .matching_pmu = "uncore_cbox_0",
> > };
>
> --
> Zhengjun Xing

Next message: David Hildenbrand: "Re: [PATCH v4 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration"
Previous message: Mark Brown: "Re: [PATCH] kselftest/arm64: bti: force static linking"
In reply to: Xing Zhengjun: "Re: [PATCH 4/7] perf jevents: Modify match field"
Next in thread: Ian Rogers: "[PATCH 5/7] perf jevents: Add python converter script"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]