* [PATCH v6 01/11] perf expr: Add expr_ prefix for parse_ctx and parse_id
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
@ 2020-03-20 12:53 ` Kajol Jain
2020-03-20 12:53 ` [PATCH v6 02/11] perf expr: Add expr_scanner_ctx object Kajol Jain
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_ prefix for parse_ctx and parse_id,
to straighten out the expr* namespace.
There's no functional change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/tests/expr.c | 4 ++--
tools/perf/util/expr.c | 10 +++++-----
tools/perf/util/expr.h | 12 ++++++------
tools/perf/util/expr.y | 6 +++---
tools/perf/util/stat-shadow.c | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 28313e59d6f6..ea10fc4412c4 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,7 +6,7 @@
#include <string.h>
#include <linux/zalloc.h>
-static int test(struct parse_ctx *ctx, const char *e, double val2)
+static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
@@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
const char **other;
double val;
int i, ret;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
int num_other;
expr__ctx_init(&ctx);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index fd192ddf93c1..c8ccc548a585 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -11,7 +11,7 @@ extern int expr_debug;
#endif
/* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
{
int idx;
@@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
ctx->ids[idx].val = val;
}
-void expr__ctx_init(struct parse_ctx *ctx)
+void expr__ctx_init(struct expr_parse_ctx *ctx)
{
ctx->num_ids = 0;
}
static int
-__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
YY_BUFFER_STATE buffer;
@@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
{
return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
}
@@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other)
{
int err, i = 0, j = 0;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9377538f4097..b9e53f2b5844 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -5,19 +5,19 @@
#define EXPR_MAX_OTHER 20
#define MAX_PARSE_ID EXPR_MAX_OTHER
-struct parse_id {
+struct expr_parse_id {
const char *name;
double val;
};
-struct parse_ctx {
+struct expr_parse_ctx {
int num_ids;
- struct parse_id ids[MAX_PARSE_ID];
+ struct expr_parse_id ids[MAX_PARSE_ID];
};
-void expr__ctx_init(struct parse_ctx *ctx);
-void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+void expr__ctx_init(struct expr_parse_ctx *ctx);
+void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other);
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 4720cbe79357..cd17486c1c5d 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -15,7 +15,7 @@
%define api.pure full
%parse-param { double *final_val }
-%parse-param { struct parse_ctx *ctx }
+%parse-param { struct expr_parse_ctx *ctx }
%parse-param {void *scanner}
%lex-param {void* scanner}
@@ -39,14 +39,14 @@
%{
static void expr_error(double *final_val __maybe_unused,
- struct parse_ctx *ctx __maybe_unused,
+ struct expr_parse_ctx *ctx __maybe_unused,
void *scanner,
const char *s)
{
pr_debug("%s\n", s);
}
-static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
{
int i;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 0fd713d3674f..402af3e8d287 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
struct runtime_stat *st)
{
print_metric_t print_metric = out->print_metric;
- struct parse_ctx pctx;
+ struct expr_parse_ctx pctx;
double ratio, scale;
int i;
void *ctxp = out->ctx;
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 02/11] perf expr: Add expr_scanner_ctx object
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
2020-03-20 12:53 ` [PATCH v6 01/11] perf expr: Add expr_ prefix for parse_ctx and parse_id Kajol Jain
@ 2020-03-20 12:53 ` Kajol Jain
2020-03-20 12:53 ` [PATCH v6 03/11] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run Kajol Jain
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_scanner_ctx object to hold user data
for the expr scanner. Currently it holds only
start_token, Kajol Jain will use it to hold 24x7
runtime param.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/expr.c | 6 ++++--
tools/perf/util/expr.h | 4 ++++
tools/perf/util/expr.l | 10 +++++-----
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c8ccc548a585..c3382d58cf40 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -3,7 +3,6 @@
#include <assert.h>
#include "expr.h"
#include "expr-bison.h"
-#define YY_EXTRA_TYPE int
#include "expr-flex.h"
#ifdef PARSER_DEBUG
@@ -30,11 +29,14 @@ static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
+ struct expr_scanner_ctx scanner_ctx = {
+ .start_token = start,
+ };
YY_BUFFER_STATE buffer;
void *scanner;
int ret;
- ret = expr_lex_init_extra(start, &scanner);
+ ret = expr_lex_init_extra(&scanner_ctx, &scanner);
if (ret)
return ret;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b9e53f2b5844..0938ad166ece 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,10 @@ struct expr_parse_ctx {
struct expr_parse_id ids[MAX_PARSE_ID];
};
+struct expr_scanner_ctx {
+ int start_token;
+};
+
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index eaad29243c23..2582c2464938 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -76,13 +76,13 @@ sym [0-9a-zA-Z_\.:@]+
symbol {spec}*{sym}*{spec}*{sym}*
%%
- {
- int start_token;
+ struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
- start_token = expr_get_extra(yyscanner);
+ {
+ int start_token = sctx->start_token;
- if (start_token) {
- expr_set_extra(NULL, yyscanner);
+ if (sctx->start_token) {
+ sctx->start_token = 0;
return start_token;
}
}
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 03/11] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
2020-03-20 12:53 ` [PATCH v6 01/11] perf expr: Add expr_ prefix for parse_ctx and parse_id Kajol Jain
2020-03-20 12:53 ` [PATCH v6 02/11] perf expr: Add expr_scanner_ctx object Kajol Jain
@ 2020-03-20 12:53 ` Kajol Jain
2020-03-20 12:53 ` [PATCH v6 04/11] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details Kajol Jain
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
values")' added to print _change_ in the counter value rather then raw
value for 24x7 counters. Incase of transactions, the event count
is set to 0 at the beginning of the transaction. It also sets
the event's prev_count to the raw value at the time of initialization.
Because of setting event count to 0, we are seeing some weird behaviour,
whenever we run multiple 24x7 events at a time.
For example:
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000121704 120 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000121704 5 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000357733 8 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000357733 10 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000641884 56 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000641884 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
5.000791887 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Getting these large values in case we do -I.
As we are setting event_count to 0, for interval case, overall event_count is not
coming in incremental order. As we may can get new delta lesser then previous count.
Because of which when we print intervals, we are getting negative value which create
these large values.
This patch removes part where we set event_count to 0 in function
'h_24x7_event_read'. There won't be much impact as we do set event->hw.prev_count
to the raw value at the time of initialization to print change value.
With this patch
In power9 platform
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000117685 93 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000117685 1 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000349331 98 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000349331 2 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495900 131 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495900 4 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000645920 204 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000645920 61 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.284169997 22 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Suggested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 573e0b309c0c..48e8f4b17b91 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1400,16 +1400,6 @@ static void h_24x7_event_read(struct perf_event *event)
h24x7hw = &get_cpu_var(hv_24x7_hw);
h24x7hw->events[i] = event;
put_cpu_var(h24x7hw);
- /*
- * Clear the event count so we can compute the _change_
- * in the 24x7 raw counter value at the end of the txn.
- *
- * Note that we could alternatively read the 24x7 value
- * now and save its value in event->hw.prev_count. But
- * that would require issuing a hcall, which would then
- * defeat the purpose of using the txn interface.
- */
- local64_set(&event->count, 0);
}
put_cpu_var(hv_24x7_reqb);
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 04/11] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (2 preceding siblings ...)
2020-03-20 12:53 ` [PATCH v6 03/11] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run Kajol Jain
@ 2020-03-20 12:53 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 05/11] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show " Kajol Jain
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
For hv_24x7 socket/chip level events, specific chip-id to which
the data requested should be added as part of pmu events.
But number of chips/socket in the system details are not exposed.
Patch implements read_sys_info_pseries() to get system
parameter values like number of sockets and chips per socket.
Rtas_call with token "PROCESSOR_MODULE_INFO"
is used to get these values.
Sub-sequent patch exports these values via sysfs.
Patch also make these parameters default to 1.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 72 ++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 3 +
2 files changed, 75 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 48e8f4b17b91..9ae00f29bd21 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -20,6 +20,11 @@
#include <asm/io.h>
#include <linux/byteorder/generic.h>
+#ifdef CONFIG_PPC_RTAS
+#include <asm/rtas.h>
+#include <../../platforms/pseries/pseries.h>
+#endif
+
#include "hv-24x7.h"
#include "hv-24x7-catalog.h"
#include "hv-common.h"
@@ -57,6 +62,69 @@ static bool is_physical_domain(unsigned domain)
}
}
+#ifdef CONFIG_PPC_RTAS
+#define PROCESSOR_MODULE_INFO 43
+#define PROCESSOR_MAX_LENGTH (8 * 1024)
+
+static int strbe16toh(const char *buf, int offset)
+{
+ return (buf[offset] << 8) + buf[offset + 1];
+}
+
+static u32 physsockets; /* Physical sockets */
+static u32 physchips; /* Physical chips */
+
+/*
+ * Function read_sys_info_pseries() make a rtas_call which require
+ * data buffer of size 8K. As standard 'rtas_data_buf' is of size
+ * 4K, we are adding new local buffer 'rtas_local_data_buf'.
+ */
+char rtas_local_data_buf[PROCESSOR_MAX_LENGTH] __cacheline_aligned;
+
+/*
+ * read_sys_info_pseries()
+ * Retrieve the number of sockets and chips per socket details
+ * through the get-system-parameter rtas call.
+ */
+void read_sys_info_pseries(void)
+{
+ int call_status, len, ntypes;
+
+ /*
+ * Making system parameter: chips and sockets default to 1.
+ */
+ physsockets = 1;
+ physchips = 1;
+ memset(rtas_local_data_buf, 0, PROCESSOR_MAX_LENGTH);
+ spin_lock(&rtas_data_buf_lock);
+
+ call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+ NULL,
+ PROCESSOR_MODULE_INFO,
+ __pa(rtas_local_data_buf),
+ PROCESSOR_MAX_LENGTH);
+
+ spin_unlock(&rtas_data_buf_lock);
+
+ if (call_status != 0) {
+ pr_info("%s %s Error calling get-system-parameter (0x%x)\n",
+ __FILE__, __func__, call_status);
+ } else {
+ rtas_local_data_buf[PROCESSOR_MAX_LENGTH - 1] = '\0';
+ len = strbe16toh(rtas_local_data_buf, 0);
+ if (len < 6)
+ return;
+
+ ntypes = strbe16toh(rtas_local_data_buf, 2);
+
+ if (!ntypes)
+ return;
+ physsockets = strbe16toh(rtas_local_data_buf, 4);
+ physchips = strbe16toh(rtas_local_data_buf, 6);
+ }
+}
+#endif /* CONFIG_PPC_RTAS */
+
/* Domains for which more than one result element are returned for each event. */
static bool domain_needs_aggregation(unsigned int domain)
{
@@ -1605,6 +1673,10 @@ static int hv_24x7_init(void)
if (r)
return r;
+#ifdef CONFIG_PPC_RTAS
+ read_sys_info_pseries();
+#endif
+
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 13fa370a87e4..1727559ce304 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -19,6 +19,9 @@ extern void request_event_sources_irqs(struct device_node *np,
struct pt_regs;
extern int pSeries_system_reset_exception(struct pt_regs *regs);
+#ifdef CONFIG_PPC_RTAS
+extern void read_sys_info_pseries(void);
+#endif
extern int pSeries_machine_check_exception(struct pt_regs *regs);
extern long pseries_machine_check_realmode(struct pt_regs *regs);
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 05/11] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show processor details
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (3 preceding siblings ...)
2020-03-20 12:53 ` [PATCH v6 04/11] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 06/11] Documentation/ABI: Add ABI documentation for chips and sockets Kajol Jain
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
To expose the system dependent parameter like total number of
sockets and numbers of chips per socket, patch adds two sysfs files.
"sockets" and "chips" are added to /sys/devices/hv_24x7/interface/
of the "hv_24x7" pmu.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 9ae00f29bd21..a31bd5b88f7a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -454,6 +454,20 @@ static ssize_t device_show_string(struct device *dev,
return sprintf(buf, "%s\n", (char *)d->var);
}
+#ifdef CONFIG_PPC_RTAS
+static ssize_t sockets_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", physsockets);
+}
+
+static ssize_t chips_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", physchips);
+}
+#endif
+
static struct attribute *device_str_attr_create_(char *name, char *str)
{
struct dev_ext_attribute *attr = kzalloc(sizeof(*attr), GFP_KERNEL);
@@ -1100,6 +1114,10 @@ PAGE_0_ATTR(catalog_len, "%lld\n",
(unsigned long long)be32_to_cpu(page_0->length) * 4096);
static BIN_ATTR_RO(catalog, 0/* real length varies */);
static DEVICE_ATTR_RO(domains);
+#ifdef CONFIG_PPC_RTAS
+static DEVICE_ATTR_RO(sockets);
+static DEVICE_ATTR_RO(chips);
+#endif
static struct bin_attribute *if_bin_attrs[] = {
&bin_attr_catalog,
@@ -1110,6 +1128,10 @@ static struct attribute *if_attrs[] = {
&dev_attr_catalog_len.attr,
&dev_attr_catalog_version.attr,
&dev_attr_domains.attr,
+#ifdef CONFIG_PPC_RTAS
+ &dev_attr_sockets.attr,
+ &dev_attr_chips.attr,
+#endif
NULL,
};
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 06/11] Documentation/ABI: Add ABI documentation for chips and sockets
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (4 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 05/11] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show " Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 07/11] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration Kajol Jain
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
Add documentation for the following sysfs files:
/sys/devices/hv_24x7/interface/chips,
/sys/devices/hv_24x7/interface/sockets
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../testing/sysfs-bus-event_source-devices-hv_24x7 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
index ec27c6c9e737..e17e5b444a1c 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
@@ -22,6 +22,20 @@ Description:
Exposes the "version" field of the 24x7 catalog. This is also
extractable from the provided binary "catalog" sysfs entry.
+What: /sys/devices/hv_24x7/interface/sockets
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of sockets present in the
+ system.
+
+What: /sys/devices/hv_24x7/interface/chips
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of chips per socket
+ present in the system.
+
What: /sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
Date: February 2014
Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 07/11] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (5 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 06/11] Documentation/ABI: Add ABI documentation for chips and sockets Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 08/11] perf/tools: Refactoring metricgroup__add_metric function Kajol Jain
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
Function 'read_sys_info_pseries()' is added to get system parameter
values like number of sockets and chips per socket.
and it gets these details via rtas_call with token
"PROCESSOR_MODULE_INFO".
Incase lpar migrate from one system to another, system
parameter details like chips per sockets or number of sockets might
change. So, it needs to be re-initialized otherwise, these values
corresponds to previous system values.
This patch adds a call to 'read_sys_info_pseries()' from
'post-mobility_fixup()' to re-init the physsockets and physchips values.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..226accd6218b 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -371,6 +371,18 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
+ /*
+ * Incase lpar migrate from one system to another, system
+ * parameter details like chips per sockets and number of sockets
+ * might change. So, it needs to be re-initialized otherwise these
+ * values corresponds to previous system.
+ * Here, adding a call to read_sys_info_pseries() declared in
+ * platforms/pseries/pseries.h to re-init the physsockets and
+ * physchips value.
+ */
+ if (IS_ENABLED(CONFIG_HV_PERF_CTRS) && IS_ENABLED(CONFIG_PPC_RTAS))
+ read_sys_info_pseries();
+
return;
}
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 08/11] perf/tools: Refactoring metricgroup__add_metric function
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (6 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 07/11] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?" Kajol Jain
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
This patch refactor metricgroup__add_metric function where
some part of it move to function metricgroup__add_metric_param.
No logic change.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/metricgroup.c | 64 +++++++++++++++++++++--------------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index c3a8c701609a..52fb119d25c8 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,6 +474,42 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+static int metricgroup__add_metric_param(struct strbuf *events,
+ struct list_head *group_list, struct pmu_event *pe)
+{
+
+ const char **ids;
+ int idnum;
+ struct egroup *eg;
+ int ret = -EINVAL;
+
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+ return ret;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ if (metricgroup__has_constraint(pe))
+ metricgroup__add_metric_non_group(events, ids, idnum);
+ else
+ metricgroup__add_metric_weak_group(events, ids, idnum);
+
+ eg = malloc(sizeof(*eg));
+ if (!eg) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = pe->metric_name;
+ eg->metric_expr = pe->metric_expr;
+ eg->metric_unit = pe->unit;
+ list_add_tail(&eg->nd, group_list);
+
+ return 0;
+}
+
static int metricgroup__add_metric(const char *metric, struct strbuf *events,
struct list_head *group_list)
{
@@ -493,35 +529,13 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
continue;
if (match_metric(pe->metric_group, metric) ||
match_metric(pe->metric_name, metric)) {
- const char **ids;
- int idnum;
- struct egroup *eg;
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- if (expr__find_other(pe->metric_expr,
- NULL, &ids, &idnum) < 0)
+ ret = metricgroup__add_metric_param(events,
+ group_list, pe);
+ if (ret == -EINVAL || !ret)
continue;
- if (events->len > 0)
- strbuf_addf(events, ",");
-
- if (metricgroup__has_constraint(pe))
- metricgroup__add_metric_non_group(events, ids, idnum);
- else
- metricgroup__add_metric_weak_group(events, ids, idnum);
-
- eg = malloc(sizeof(struct egroup));
- if (!eg) {
- ret = -ENOMEM;
- break;
- }
- eg->ids = ids;
- eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
- eg->metric_expr = pe->metric_expr;
- eg->metric_unit = pe->unit;
- list_add_tail(&eg->nd, group_list);
- ret = 0;
}
}
return ret;
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (7 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 08/11] perf/tools: Refactoring metricgroup__add_metric function Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-24 13:11 ` Jiri Olsa
2020-03-24 13:11 ` Jiri Olsa
2020-03-20 12:54 ` [PATCH v6 10/11] tools/perf: Enable Hz/hz prinitg for --metric-only option Kajol Jain
2020-03-20 12:54 ` [PATCH v6 11/11] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events Kajol Jain
10 siblings, 2 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
Patch enhances current metric infrastructure to handle "?" in the metric
expression. The "?" can be use for parameters whose value not known while
creating metric events and which can be replace later at runtime to
the proper value. It also add flexibility to create multiple events out
of single metric event added in json file.
Patch adds function 'arch_get_runtimeparam' which is a arch specific
function, returns the count of metric events need to be created.
By default it return 1.
This infrastructure needed for hv_24x7 socket/chip level events.
"hv_24x7" chip level events needs specific chip-id to which the
data is requested. Function 'arch_get_runtimeparam' implemented
in header.c which extract number of sockets from sysfs file
"sockets" under "/sys/devices/hv_24x7/interface/".
With this patch basically we are trying to create as many metric events
as define by runtime_param.
For that one loop is added in function 'metricgroup__add_metric',
which create multiple events at run time depend on return value of
'arch_get_runtimeparam' and merge that event in 'group_list'.
To achieve that we are actually passing this parameter value as part of
`expr__find_other` function and changing "?" present in metric expression
with this value.
As in our json file, there gonna be single metric event, and out of
which we are creating multiple events, I am also merging this value
to the original metric name to specify parameter value.
For example,
command:# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
# time counts unit events
1.000101867 9,356,933 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
1.000101867 9,366,134 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
2.000314878 9,365,868 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
2.000314878 9,366,092 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
So, here _0 and _1 after PowerBUS_Frequency specify parameter value.
As after adding this to group_list, again we call expr__parse
in 'generic_metric' function present in util/stat-display.c.
By this time again we need to pass this parameter value. So, now to get this value
actually I am trying to extract it from metric name itself. Because
otherwise it gonna point to last updated value present in runtime_param.
And gonna match for that value only.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
| 8 ++++++
tools/perf/tests/expr.c | 8 +++---
tools/perf/util/expr.c | 11 ++++----
tools/perf/util/expr.h | 5 ++--
tools/perf/util/expr.l | 27 +++++++++++++-----
tools/perf/util/metricgroup.c | 40 +++++++++++++++++++++++----
tools/perf/util/metricgroup.h | 1 +
tools/perf/util/stat-shadow.c | 12 ++++++--
8 files changed, 86 insertions(+), 26 deletions(-)
--git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 3b4cdfc5efd6..c0a86afe63fb 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -7,6 +7,8 @@
#include <string.h>
#include <linux/stringify.h>
#include "header.h"
+#include "metricgroup.h"
+#include <api/fs/fs.h>
#define mfspr(rn) ({unsigned long rval; \
asm volatile("mfspr %0," __stringify(rn) \
@@ -44,3 +46,9 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
return bufp;
}
+
+int arch_get_runtimeparam(void)
+{
+ int count;
+ return sysfs__read_int("/devices/hv_24x7/interface/sockets", &count) < 0 ? 3 : count;
+}
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index ea10fc4412c4..516504cf0ea5 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -10,7 +10,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
- if (expr__parse(&val, ctx, e))
+ if (expr__parse(&val, ctx, e, 1))
TEST_ASSERT_VAL("parse test failed", 0);
TEST_ASSERT_VAL("unexpected value", val == val2);
return 0;
@@ -44,15 +44,15 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
return ret;
p = "FOO/0";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("missing operand", ret == -1);
TEST_ASSERT_VAL("find other",
- expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other) == 0);
+ expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other, 1) == 0);
TEST_ASSERT_VAL("find other", num_other == 3);
TEST_ASSERT_VAL("find other", !strcmp(other[0], "BAR"));
TEST_ASSERT_VAL("find other", !strcmp(other[1], "BAZ"));
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c3382d58cf40..9d56e5fb3873 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -27,10 +27,11 @@ void expr__ctx_init(struct expr_parse_ctx *ctx)
static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
- int start)
+ int start, int param)
{
struct expr_scanner_ctx scanner_ctx = {
.start_token = start,
+ .runtime_param = param,
};
YY_BUFFER_STATE buffer;
void *scanner;
@@ -54,9 +55,9 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param)
{
- return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
+ return __expr__parse(final_val, ctx, expr, EXPR_PARSE, param) ? -1 : 0;
}
static bool
@@ -74,13 +75,13 @@ already_seen(const char *val, const char *one, const char **other,
}
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other)
+ int *num_other, int param)
{
int err, i = 0, j = 0;
struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
- err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
+ err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER, param);
if (err)
return -1;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 0938ad166ece..0b91597d6512 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -17,12 +17,13 @@ struct expr_parse_ctx {
struct expr_scanner_ctx {
int start_token;
+ int runtime_param;
};
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param);
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other);
+ int *num_other, int param);
#endif
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 2582c2464938..86eb822c51cc 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -35,7 +35,7 @@ static int value(yyscan_t scanner, int base)
* Allow @ instead of / to be able to specify pmu/event/ without
* conflicts with normal division.
*/
-static char *normalize(char *str)
+static char *normalize(char *str, int runtime_param)
{
char *ret = str;
char *dst = str;
@@ -45,6 +45,19 @@ static char *normalize(char *str)
*dst++ = '/';
else if (*str == '\\')
*dst++ = *++str;
+ else if (*str == '?') {
+ char *paramval;
+ int i = 0;
+ int size = asprintf(¶mval, "%d", runtime_param);
+
+ if (size < 0)
+ *dst++ = '0';
+ else {
+ while (i < size)
+ *dst++ = paramval[i++];
+ free(paramval);
+ }
+ }
else
*dst++ = *str;
str++;
@@ -54,16 +67,16 @@ static char *normalize(char *str)
return ret;
}
-static int str(yyscan_t scanner, int token)
+static int str(yyscan_t scanner, int token, int runtime_param)
{
YYSTYPE *yylval = expr_get_lval(scanner);
char *text = expr_get_text(scanner);
- yylval->str = normalize(strdup(text));
+ yylval->str = normalize(strdup(text), runtime_param);
if (!yylval->str)
return EXPR_ERROR;
- yylval->str = normalize(yylval->str);
+ yylval->str = normalize(yylval->str, runtime_param);
return token;
}
%}
@@ -72,8 +85,8 @@ number [0-9]+
sch [-,=]
spec \\{sch}
-sym [0-9a-zA-Z_\.:@]+
-symbol {spec}*{sym}*{spec}*{sym}*
+sym [0-9a-zA-Z_\.:@?]+
+symbol {spec}*{sym}*{spec}*{sym}*{spec}*{sym}
%%
struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
@@ -93,7 +106,7 @@ if { return IF; }
else { return ELSE; }
#smt_on { return SMT_ON; }
{number} { return value(yyscanner, 10); }
-{symbol} { return str(yyscanner, ID); }
+{symbol} { return str(yyscanner, ID, sctx->runtime_param); }
"|" { return '|'; }
"^" { return '^'; }
"&" { return '&'; }
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 52fb119d25c8..b4b91d8ad5be 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,8 +474,13 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+int __weak arch_get_runtimeparam(void)
+{
+ return 1;
+}
+
static int metricgroup__add_metric_param(struct strbuf *events,
- struct list_head *group_list, struct pmu_event *pe)
+ struct list_head *group_list, struct pmu_event *pe, int param)
{
const char **ids;
@@ -483,7 +488,7 @@ static int metricgroup__add_metric_param(struct strbuf *events,
struct egroup *eg;
int ret = -EINVAL;
- if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, param) < 0)
return ret;
if (events->len > 0)
@@ -502,11 +507,21 @@ static int metricgroup__add_metric_param(struct strbuf *events,
eg->ids = ids;
eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
+ if (strstr(pe->metric_expr, "?")) {
+ char value[PATH_MAX];
+
+ sprintf(value, "%s%c%d", pe->metric_name, '_', param);
+ eg->metric_name = strdup(value);
+ if (!eg->metric_name) {
+ ret = -ENOMEM;
+ return ret;
+ }
+ }
+ else
+ eg->metric_name = pe->metric_name;
eg->metric_expr = pe->metric_expr;
eg->metric_unit = pe->unit;
list_add_tail(&eg->nd, group_list);
-
return 0;
}
@@ -532,8 +547,21 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- ret = metricgroup__add_metric_param(events,
- group_list, pe);
+ if (!strstr(pe->metric_expr, "?")) {
+ ret = metricgroup__add_metric_param(events, group_list, pe, 1);
+ } else {
+ int j, count;
+
+ count = arch_get_runtimeparam();
+
+ /* This loop is added to create multiple
+ * events depend on count value and add
+ * those events to group_list.
+ */
+
+ for (j = 0; j < count; j++)
+ ret = metricgroup__add_metric_param(events, group_list, pe, j);
+ }
if (ret == -EINVAL || !ret)
continue;
}
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..81224ba1270d 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -34,4 +34,5 @@ int metricgroup__parse_groups(const struct option *opt,
void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
bool metricgroup__has_metric(const char *metric);
+int arch_get_runtimeparam(void);
#endif
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 402af3e8d287..85ac6d913782 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -336,7 +336,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
metric_events = counter->metric_events;
if (!metric_events) {
if (expr__find_other(counter->metric_expr, counter->name,
- &metric_names, &num_metric_names) < 0)
+ &metric_names, &num_metric_names, 1) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
@@ -777,7 +777,15 @@ static void generic_metric(struct perf_stat_config *config,
}
if (!metric_events[i]) {
- if (expr__parse(&ratio, &pctx, metric_expr) == 0) {
+ int param = 1;
+ if (strstr(metric_expr, "?")) {
+ char *tmp = strrchr(metric_name, '_');
+
+ tmp++;
+ param = strtol(tmp, &tmp, 10);
+ }
+
+ if (expr__parse(&ratio, &pctx, metric_expr, param) == 0) {
char *unit;
char metric_bf[64];
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
2020-03-20 12:54 ` [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?" Kajol Jain
@ 2020-03-24 13:11 ` Jiri Olsa
2020-03-24 13:11 ` Jiri Olsa
1 sibling, 0 replies; 15+ messages in thread
From: Jiri Olsa @ 2020-03-24 13:11 UTC (permalink / raw)
To: Kajol Jain
Cc: acme, linuxppc-dev, mpe, sukadev, linux-kernel, linux-perf-users,
anju, maddy, ravi.bangoria, peterz, yao.jin, ak, jolsa,
kan.liang, jmario, alexander.shishkin, mingo, paulus, namhyung,
mpetlan, gregkh, benh, mamatha4, mark.rutland, tglx
On Fri, Mar 20, 2020 at 06:24:04PM +0530, Kajol Jain wrote:
SNIP
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 52fb119d25c8..b4b91d8ad5be 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -474,8 +474,13 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
> return false;
> }
>
> +int __weak arch_get_runtimeparam(void)
> +{
> + return 1;
> +}
> +
> static int metricgroup__add_metric_param(struct strbuf *events,
> - struct list_head *group_list, struct pmu_event *pe)
> + struct list_head *group_list, struct pmu_event *pe, int param)
> {
>
> const char **ids;
> @@ -483,7 +488,7 @@ static int metricgroup__add_metric_param(struct strbuf *events,
could you please call this function __metricgroup__add_metric instead?
> struct egroup *eg;
> int ret = -EINVAL;
>
> - if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
> + if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, param) < 0)
> return ret;
>
> if (events->len > 0)
> @@ -502,11 +507,21 @@ static int metricgroup__add_metric_param(struct strbuf *events,
>
> eg->ids = ids;
> eg->idnum = idnum;
> - eg->metric_name = pe->metric_name;
> + if (strstr(pe->metric_expr, "?")) {
> + char value[PATH_MAX];
> +
> + sprintf(value, "%s%c%d", pe->metric_name, '_', param);
> + eg->metric_name = strdup(value);
how is eg->metric_name getting released?
> + if (!eg->metric_name) {
> + ret = -ENOMEM;
> + return ret;
return -ENOMEM; ??
> + }
> + }
> + else
> + eg->metric_name = pe->metric_name;
> eg->metric_expr = pe->metric_expr;
> eg->metric_unit = pe->unit;
> list_add_tail(&eg->nd, group_list);
> -
> return 0;
> }
>
SNIP
thanks,
jirka
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
2020-03-20 12:54 ` [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?" Kajol Jain
2020-03-24 13:11 ` Jiri Olsa
@ 2020-03-24 13:11 ` Jiri Olsa
2020-03-27 10:00 ` kajoljain
1 sibling, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2020-03-24 13:11 UTC (permalink / raw)
To: Kajol Jain
Cc: acme, linuxppc-dev, mpe, sukadev, linux-kernel, linux-perf-users,
anju, maddy, ravi.bangoria, peterz, yao.jin, ak, jolsa,
kan.liang, jmario, alexander.shishkin, mingo, paulus, namhyung,
mpetlan, gregkh, benh, mamatha4, mark.rutland, tglx
On Fri, Mar 20, 2020 at 06:24:04PM +0530, Kajol Jain wrote:
> Patch enhances current metric infrastructure to handle "?" in the metric
> expression. The "?" can be use for parameters whose value not known while
> creating metric events and which can be replace later at runtime to
> the proper value. It also add flexibility to create multiple events out
> of single metric event added in json file.
>
> Patch adds function 'arch_get_runtimeparam' which is a arch specific
> function, returns the count of metric events need to be created.
> By default it return 1.
>
> This infrastructure needed for hv_24x7 socket/chip level events.
> "hv_24x7" chip level events needs specific chip-id to which the
> data is requested. Function 'arch_get_runtimeparam' implemented
> in header.c which extract number of sockets from sysfs file
> "sockets" under "/sys/devices/hv_24x7/interface/".
>
>
> With this patch basically we are trying to create as many metric events
> as define by runtime_param.
>
> For that one loop is added in function 'metricgroup__add_metric',
> which create multiple events at run time depend on return value of
> 'arch_get_runtimeparam' and merge that event in 'group_list'.
>
> To achieve that we are actually passing this parameter value as part of
> `expr__find_other` function and changing "?" present in metric expression
> with this value.
>
> As in our json file, there gonna be single metric event, and out of
> which we are creating multiple events, I am also merging this value
> to the original metric name to specify parameter value.
>
> For example,
> command:# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
> # time counts unit events
> 1.000101867 9,356,933 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
> 1.000101867 9,366,134 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
> 2.000314878 9,365,868 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
> 2.000314878 9,366,092 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
>
> So, here _0 and _1 after PowerBUS_Frequency specify parameter value.
>
> As after adding this to group_list, again we call expr__parse
> in 'generic_metric' function present in util/stat-display.c.
> By this time again we need to pass this parameter value. So, now to get this value
> actually I am trying to extract it from metric name itself. Because
> otherwise it gonna point to last updated value present in runtime_param.
> And gonna match for that value only.
so why can't we pass that param as integer value through the metric objects?
it get's created in metricgroup__add_metric_param:
- as struct egroup *eg
- we can add egroup::param and store the param value there
then in metricgroup__setup_events it moves to:
- struct metric_expr *expr
- we can add metric_expr::param to keep the param
then in perf_stat__print_shadow_stats there's:
- struct metric_expr *mexp loop
- calling generic_metric metric - we could call it with mexp::param
- and pass the param to expr__parse
jirka
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
2020-03-24 13:11 ` Jiri Olsa
@ 2020-03-27 10:00 ` kajoljain
0 siblings, 0 replies; 15+ messages in thread
From: kajoljain @ 2020-03-27 10:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, linuxppc-dev, mpe, sukadev, linux-kernel, linux-perf-users,
anju, maddy, ravi.bangoria, peterz, yao.jin, ak, jolsa,
kan.liang, jmario, alexander.shishkin, mingo, paulus, namhyung,
mpetlan, gregkh, benh, mamatha4, mark.rutland, tglx
On 3/24/20 6:41 PM, Jiri Olsa wrote:
> On Fri, Mar 20, 2020 at 06:24:04PM +0530, Kajol Jain wrote:
>> Patch enhances current metric infrastructure to handle "?" in the metric
>> expression. The "?" can be use for parameters whose value not known while
>> creating metric events and which can be replace later at runtime to
>> the proper value. It also add flexibility to create multiple events out
>> of single metric event added in json file.
>>
>> Patch adds function 'arch_get_runtimeparam' which is a arch specific
>> function, returns the count of metric events need to be created.
>> By default it return 1.
>>
>> This infrastructure needed for hv_24x7 socket/chip level events.
>> "hv_24x7" chip level events needs specific chip-id to which the
>> data is requested. Function 'arch_get_runtimeparam' implemented
>> in header.c which extract number of sockets from sysfs file
>> "sockets" under "/sys/devices/hv_24x7/interface/".
>>
>>
>> With this patch basically we are trying to create as many metric events
>> as define by runtime_param.
>>
>> For that one loop is added in function 'metricgroup__add_metric',
>> which create multiple events at run time depend on return value of
>> 'arch_get_runtimeparam' and merge that event in 'group_list'.
>>
>> To achieve that we are actually passing this parameter value as part of
>> `expr__find_other` function and changing "?" present in metric expression
>> with this value.
>>
>> As in our json file, there gonna be single metric event, and out of
>> which we are creating multiple events, I am also merging this value
>> to the original metric name to specify parameter value.
>>
>> For example,
>> command:# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
>> # time counts unit events
>> 1.000101867 9,356,933 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
>> 1.000101867 9,366,134 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
>> 2.000314878 9,365,868 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
>> 2.000314878 9,366,092 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
>>
>> So, here _0 and _1 after PowerBUS_Frequency specify parameter value.
>>
>> As after adding this to group_list, again we call expr__parse
>> in 'generic_metric' function present in util/stat-display.c.
>> By this time again we need to pass this parameter value. So, now to get this value
>> actually I am trying to extract it from metric name itself. Because
>> otherwise it gonna point to last updated value present in runtime_param.
>> And gonna match for that value only.
>
> so why can't we pass that param as integer value through the metric objects?
>
> it get's created in metricgroup__add_metric_param:
> - as struct egroup *eg
> - we can add egroup::param and store the param value there
>
> then in metricgroup__setup_events it moves to:
> - struct metric_expr *expr
> - we can add metric_expr::param to keep the param
>
> then in perf_stat__print_shadow_stats there's:
> - struct metric_expr *mexp loop
> - calling generic_metric metric - we could call it with mexp::param
> - and pass the param to expr__parse
>
Hi jiri,
Thanks for the suggestion, Yes it make more sense to use like that.
Will update.
Thanks,
Kajol
> jirka
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 10/11] tools/perf: Enable Hz/hz prinitg for --metric-only option
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (8 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?" Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
2020-03-20 12:54 ` [PATCH v6 11/11] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events Kajol Jain
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
Commit 54b5091606c18 ("perf stat: Implement --metric-only mode")
added function 'valid_only_metric()' which drops "Hz" or "hz",
if it is part of "ScaleUnit". This patch enable it since hv_24x7
supports couple of frequency events.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/stat-display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 16efdba1973a..ecdebfcdd379 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -237,8 +237,6 @@ static bool valid_only_metric(const char *unit)
if (!unit)
return false;
if (strstr(unit, "/sec") ||
- strstr(unit, "hz") ||
- strstr(unit, "Hz") ||
strstr(unit, "CPUs utilized"))
return false;
return true;
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 11/11] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
2020-03-20 12:53 [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events Kajol Jain
` (9 preceding siblings ...)
2020-03-20 12:54 ` [PATCH v6 10/11] tools/perf: Enable Hz/hz prinitg for --metric-only option Kajol Jain
@ 2020-03-20 12:54 ` Kajol Jain
10 siblings, 0 replies; 15+ messages in thread
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario,
alexander.shishkin, mingo, paulus, namhyung, mpetlan, gregkh,
benh, mamatha4, mark.rutland, tglx, kjain
The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
facility to continuously collect large numbers of hardware performance
metrics efficiently and accurately.
This patch adds hv_24x7 metric file for different Socket/chip
resources.
Result:
power9 platform:
command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0 -I 1000
1.000096188 0.9 0.3
2.000285720 0.5 0.1
3.000424990 0.4 0.1
command:# ./perf stat --metric-only -M PowerBUS_Frequency -C 0 -I 1000
1.000097981 2.3 2.3
2.000291713 2.3 2.3
3.000421719 2.3 2.3
4.000550912 2.3 2.3
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../arch/powerpc/power9/nest_metrics.json | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
diff --git a/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
new file mode 100644
index 000000000000..ac38f5540ac6
--- /dev/null
+++ b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
@@ -0,0 +1,19 @@
+[
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
+ "MetricName": "Memory_RD_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
+ "MetricName": "Memory_WR_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
+ "MetricName": "PowerBUS_Frequency",
+ "ScaleUnit": "2.5e-7GHz"
+ }
+]
--
2.18.1
^ permalink raw reply related [flat|nested] 15+ messages in thread