perf mem: add priv level filtering support
This patch adds the -u -and -k options to perf to allow
filtering of load/store sampling based on priv levels.
This may not be supported by all HW platforms.
By default, loads/stores are sampled at both user and
kernel privilege levels.
To sample only at the user level:
$ perf mem -u -t load rec ......
To sample only at the kernel level:
$ perf mem -k -t load rec ......
Man page updated accordingly.
Signed-off-by: Stephane Eranian <[email protected]>
---
diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
index 888d511..4c4e405 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -43,6 +43,12 @@ OPTIONS
option can be passed in record mode. It will be interpreted the same way as perf
record.
+-k::
+ Only sample loads/stores at the user level (default: user + kernel)
+
+-u::
+ Only sample loads/stores at the kernel level (default: user + kernel)
+
SEE ALSO
--------
linkperf:perf-record[1], linkperf:perf-report[1]
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 706a1fa..8ac9d1e 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -9,13 +9,18 @@
#define MEM_OPERATION_LOAD "load"
#define MEM_OPERATION_STORE "store"
-static const char *mem_operation = MEM_OPERATION_LOAD;
+#define OP_LOAD 0x1
+#define OP_STORE 0x2
+
struct perf_mem {
struct perf_tool tool;
char const *input_name;
bool hide_unresolved;
+ const char *mem_op;
bool dump_raw;
+ bool user;
+ bool kernel;
const char *cpu_list;
DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
};
@@ -25,35 +30,88 @@ static const char * const mem_usage[] = {
NULL
};
-static int __cmd_record(int argc, const char **argv)
+static inline const char *get_plm(struct perf_mem *mem)
+{
+ const char *plm = "";
+
+ if (mem->user && !mem->kernel) {
+ plm = "u";
+ } else if (!mem->user && mem->kernel) {
+ plm = "k";
+ }
+ return plm;
+}
+
+static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
{
int rec_argc, i = 0, j;
const char **rec_argv;
- char event[64];
- int ret;
+ char *str;
+ int mode = 0;
+ int ki, ret;
+
+
+ if (!strcmp(mem->mem_op, MEM_OPERATION_STORE))
+ mode |= OP_STORE;
+ else if (!strcmp(mem->mem_op, MEM_OPERATION_LOAD))
+ mode |= OP_LOAD;
+ else {
+ fprintf(stderr, "unknown sampling mode: %s\n", mem->mem_op);
+ return -1;
+ }
- rec_argc = argc + 4;
+ rec_argc = argc + 6;
rec_argv = calloc(rec_argc + 1, sizeof(char *));
if (!rec_argv)
return -1;
rec_argv[i++] = strdup("record");
- if (!strcmp(mem_operation, MEM_OPERATION_LOAD))
- rec_argv[i++] = strdup("-W");
+
rec_argv[i++] = strdup("-d");
- rec_argv[i++] = strdup("-e");
- if (strcmp(mem_operation, MEM_OPERATION_LOAD))
- sprintf(event, "cpu/mem-stores/pp");
- else
- sprintf(event, "cpu/mem-loads/pp");
+ if (mode & OP_LOAD) {
+ rec_argv[i++] = strdup("-W");
- rec_argv[i++] = strdup(event);
- for (j = 1; j < argc; j++, i++)
- rec_argv[i] = argv[j];
+ rec_argv[i++] = strdup("-e");
+
+ str = malloc(strlen("cpu/mem-loads/pp") + 1 + 1);
+ if (!str) {
+ ki = i;
+ ret = -1;
+ goto end;
+ }
+ sprintf(str, "cpu/mem-loads/%spp", get_plm(mem));
+ rec_argv[i++] = str;
+ }
+
+ if (mode & OP_STORE) {
+ rec_argv[i++] = strdup("-e");
+
+ str = malloc(strlen("cpu/mem-stores/pp") + 1 + 1);
+ if (!str) {
+ ki = i;
+ ret = -1;
+ goto end;
+ }
+ sprintf(str, "cpu/mem-stores/%spp", get_plm(mem));
+ rec_argv[i++] = str;
+ }
+
+ /* arguments after i are not malloc'd */
+ ki = i;
- ret = cmd_record(i, rec_argv, NULL);
+ for (j = 1; j < argc; j++, ki++)
+ rec_argv[ki] = argv[j];
+
+ ret = cmd_record(ki, rec_argv, NULL);
+
+end:
+ /*
+ * XXX: free rec_argv[] entries, difficult because
+ * cmd_record() drops some of them...
+ */
free(rec_argv);
+
return ret;
}
@@ -171,7 +229,7 @@ static int report_events(int argc, const char **argv, struct perf_mem *mem)
* there is no weight (cost) associated with stores, so don't print
* the column
*/
- if (strcmp(mem_operation, MEM_OPERATION_LOAD))
+ if (strcmp(mem->mem_op, MEM_OPERATION_LOAD))
rep_argv[i++] = strdup("--sort=mem,sym,dso,symbol_daddr,"
"dso_daddr,tlb,locked");
@@ -199,7 +257,7 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
.input_name = "perf.data",
};
const struct option mem_options[] = {
- OPT_STRING('t', "type", &mem_operation,
+ OPT_STRING('t', "type", &mem.mem_op,
"type", "memory operations(load/store)"),
OPT_BOOLEAN('D', "dump-raw-samples", &mem.dump_raw,
"dump raw samples in ASCII"),
@@ -213,13 +271,18 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
"separator",
"separator for columns, no spaces will be added"
" between columns '.' is reserved."),
+ OPT_BOOLEAN('u', "user-level", &mem.user,
+ "include user-level accesses"),
+ OPT_BOOLEAN('k', "kernel-level", &mem.kernel,
+ "include kernel-level accesses"),
OPT_END()
};
argc = parse_options(argc, argv, mem_options, mem_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
- if (!argc || !(strncmp(argv[0], "rec", 3) || mem_operation))
+ if (!argc || !(strncmp(argv[0], "rec", 3)
+ || strncmp(argv[0], "rep", 3)))
usage_with_options(mem_usage, mem_options);
if (!mem.input_name || !strlen(mem.input_name)) {
@@ -228,9 +291,12 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
else
mem.input_name = "perf.data";
}
+ /* default to load only, some processors only support loads */
+ if (!mem.mem_op)
+ mem.mem_op = MEM_OPERATION_LOAD;
if (!strncmp(argv[0], "rec", 3))
- return __cmd_record(argc, argv);
+ return __cmd_record(argc, argv, &mem);
else if (!strncmp(argv[0], "rep", 3))
return report_events(argc, argv, &mem);
else
On 8/26/13 7:11 AM, Stephane Eranian wrote:
> perf mem: add priv level filtering support
>
> This patch adds the -u -and -k options to perf to allow
> filtering of load/store sampling based on priv levels.
> This may not be supported by all HW platforms.
>
> By default, loads/stores are sampled at both user and
> kernel privilege levels.
>
> To sample only at the user level:
> $ perf mem -u -t load rec ......
>
> To sample only at the kernel level:
> $ perf mem -k -t load rec ......
>
> Man page updated accordingly.
>
> Signed-off-by: Stephane Eranian <[email protected]>
> ---
>
> diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
> index 888d511..4c4e405 100644
> --- a/tools/perf/Documentation/perf-mem.txt
> +++ b/tools/perf/Documentation/perf-mem.txt
> @@ -43,6 +43,12 @@ OPTIONS
> option can be passed in record mode. It will be interpreted the same way as perf
> record.
>
> +-k::
> + Only sample loads/stores at the user level (default: user + kernel)
> +
> +-u::
> + Only sample loads/stores at the kernel level (default: user + kernel)
Are the descriptions backwards? In the commit message yuo have -u means
user level and -k means kernel level; the help message here seems backwards.
David
> +
> SEE ALSO
> --------
> linkperf:perf-record[1], linkperf:perf-report[1]
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index 706a1fa..8ac9d1e 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -9,13 +9,18 @@
> #define MEM_OPERATION_LOAD "load"
> #define MEM_OPERATION_STORE "store"
>
> -static const char *mem_operation = MEM_OPERATION_LOAD;
> +#define OP_LOAD 0x1
> +#define OP_STORE 0x2
> +
>
> struct perf_mem {
> struct perf_tool tool;
> char const *input_name;
> bool hide_unresolved;
> + const char *mem_op;
> bool dump_raw;
> + bool user;
> + bool kernel;
> const char *cpu_list;
> DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
> };
> @@ -25,35 +30,88 @@ static const char * const mem_usage[] = {
> NULL
> };
>
> -static int __cmd_record(int argc, const char **argv)
> +static inline const char *get_plm(struct perf_mem *mem)
> +{
> + const char *plm = "";
> +
> + if (mem->user && !mem->kernel) {
> + plm = "u";
> + } else if (!mem->user && mem->kernel) {
> + plm = "k";
> + }
> + return plm;
> +}
> +
> +static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> {
> int rec_argc, i = 0, j;
> const char **rec_argv;
> - char event[64];
> - int ret;
> + char *str;
> + int mode = 0;
> + int ki, ret;
> +
> +
> + if (!strcmp(mem->mem_op, MEM_OPERATION_STORE))
> + mode |= OP_STORE;
> + else if (!strcmp(mem->mem_op, MEM_OPERATION_LOAD))
> + mode |= OP_LOAD;
> + else {
> + fprintf(stderr, "unknown sampling mode: %s\n", mem->mem_op);
> + return -1;
> + }
>
> - rec_argc = argc + 4;
> + rec_argc = argc + 6;
> rec_argv = calloc(rec_argc + 1, sizeof(char *));
> if (!rec_argv)
> return -1;
>
> rec_argv[i++] = strdup("record");
> - if (!strcmp(mem_operation, MEM_OPERATION_LOAD))
> - rec_argv[i++] = strdup("-W");
> +
> rec_argv[i++] = strdup("-d");
> - rec_argv[i++] = strdup("-e");
>
> - if (strcmp(mem_operation, MEM_OPERATION_LOAD))
> - sprintf(event, "cpu/mem-stores/pp");
> - else
> - sprintf(event, "cpu/mem-loads/pp");
> + if (mode & OP_LOAD) {
> + rec_argv[i++] = strdup("-W");
>
> - rec_argv[i++] = strdup(event);
> - for (j = 1; j < argc; j++, i++)
> - rec_argv[i] = argv[j];
> + rec_argv[i++] = strdup("-e");
> +
> + str = malloc(strlen("cpu/mem-loads/pp") + 1 + 1);
> + if (!str) {
> + ki = i;
> + ret = -1;
> + goto end;
> + }
> + sprintf(str, "cpu/mem-loads/%spp", get_plm(mem));
> + rec_argv[i++] = str;
> + }
> +
> + if (mode & OP_STORE) {
> + rec_argv[i++] = strdup("-e");
> +
> + str = malloc(strlen("cpu/mem-stores/pp") + 1 + 1);
> + if (!str) {
> + ki = i;
> + ret = -1;
> + goto end;
> + }
> + sprintf(str, "cpu/mem-stores/%spp", get_plm(mem));
> + rec_argv[i++] = str;
> + }
> +
> + /* arguments after i are not malloc'd */
> + ki = i;
>
> - ret = cmd_record(i, rec_argv, NULL);
> + for (j = 1; j < argc; j++, ki++)
> + rec_argv[ki] = argv[j];
> +
> + ret = cmd_record(ki, rec_argv, NULL);
> +
> +end:
> + /*
> + * XXX: free rec_argv[] entries, difficult because
> + * cmd_record() drops some of them...
> + */
> free(rec_argv);
> +
> return ret;
> }
>
> @@ -171,7 +229,7 @@ static int report_events(int argc, const char **argv, struct perf_mem *mem)
> * there is no weight (cost) associated with stores, so don't print
> * the column
> */
> - if (strcmp(mem_operation, MEM_OPERATION_LOAD))
> + if (strcmp(mem->mem_op, MEM_OPERATION_LOAD))
> rep_argv[i++] = strdup("--sort=mem,sym,dso,symbol_daddr,"
> "dso_daddr,tlb,locked");
>
> @@ -199,7 +257,7 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
> .input_name = "perf.data",
> };
> const struct option mem_options[] = {
> - OPT_STRING('t', "type", &mem_operation,
> + OPT_STRING('t', "type", &mem.mem_op,
> "type", "memory operations(load/store)"),
> OPT_BOOLEAN('D', "dump-raw-samples", &mem.dump_raw,
> "dump raw samples in ASCII"),
> @@ -213,13 +271,18 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
> "separator",
> "separator for columns, no spaces will be added"
> " between columns '.' is reserved."),
> + OPT_BOOLEAN('u', "user-level", &mem.user,
> + "include user-level accesses"),
> + OPT_BOOLEAN('k', "kernel-level", &mem.kernel,
> + "include kernel-level accesses"),
> OPT_END()
> };
>
> argc = parse_options(argc, argv, mem_options, mem_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
>
> - if (!argc || !(strncmp(argv[0], "rec", 3) || mem_operation))
> + if (!argc || !(strncmp(argv[0], "rec", 3)
> + || strncmp(argv[0], "rep", 3)))
> usage_with_options(mem_usage, mem_options);
>
> if (!mem.input_name || !strlen(mem.input_name)) {
> @@ -228,9 +291,12 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
> else
> mem.input_name = "perf.data";
> }
> + /* default to load only, some processors only support loads */
> + if (!mem.mem_op)
> + mem.mem_op = MEM_OPERATION_LOAD;
>
> if (!strncmp(argv[0], "rec", 3))
> - return __cmd_record(argc, argv);
> + return __cmd_record(argc, argv, &mem);
> else if (!strncmp(argv[0], "rep", 3))
> return report_events(argc, argv, &mem);
> else
>
On Tue, Aug 27, 2013 at 12:21 AM, David Ahern <[email protected]> wrote:
>
> On 8/26/13 7:11 AM, Stephane Eranian wrote:
>>
>> perf mem: add priv level filtering support
>>
>> This patch adds the -u -and -k options to perf to allow
>> filtering of load/store sampling based on priv levels.
>> This may not be supported by all HW platforms.
>>
>> By default, loads/stores are sampled at both user and
>> kernel privilege levels.
>>
>> To sample only at the user level:
>> $ perf mem -u -t load rec ......
>>
>> To sample only at the kernel level:
>> $ perf mem -k -t load rec ......
>>
>> Man page updated accordingly.
>>
>> Signed-off-by: Stephane Eranian <[email protected]>
>> ---
>>
>> diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
>> index 888d511..4c4e405 100644
>> --- a/tools/perf/Documentation/perf-mem.txt
>> +++ b/tools/perf/Documentation/perf-mem.txt
>> @@ -43,6 +43,12 @@ OPTIONS
>> option can be passed in record mode. It will be interpreted the same way as perf
>> record.
>>
>> +-k::
>> + Only sample loads/stores at the user level (default: user + kernel)
>> +
>> +-u::
>> + Only sample loads/stores at the kernel level (default: user + kernel)
>
>
> Are the descriptions backwards? In the commit message yuo have -u means user level and -k means kernel level; the help message here seems backwards.
>
You are right. Let me resubmit....
Thanks.
>
> David
>
>
>
>> +
>> SEE ALSO
>> --------
>> linkperf:perf-record[1], linkperf:perf-report[1]
>> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
>> index 706a1fa..8ac9d1e 100644
>> --- a/tools/perf/builtin-mem.c
>> +++ b/tools/perf/builtin-mem.c
>> @@ -9,13 +9,18 @@
>> #define MEM_OPERATION_LOAD "load"
>> #define MEM_OPERATION_STORE "store"
>>
>> -static const char *mem_operation = MEM_OPERATION_LOAD;
>> +#define OP_LOAD 0x1
>> +#define OP_STORE 0x2
>> +
>>
>> struct perf_mem {
>> struct perf_tool tool;
>> char const *input_name;
>> bool hide_unresolved;
>> + const char *mem_op;
>> bool dump_raw;
>> + bool user;
>> + bool kernel;
>> const char *cpu_list;
>> DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>> };
>> @@ -25,35 +30,88 @@ static const char * const mem_usage[] = {
>> NULL
>> };
>>
>> -static int __cmd_record(int argc, const char **argv)
>> +static inline const char *get_plm(struct perf_mem *mem)
>> +{
>> + const char *plm = "";
>> +
>> + if (mem->user && !mem->kernel) {
>> + plm = "u";
>> + } else if (!mem->user && mem->kernel) {
>> + plm = "k";
>> + }
>> + return plm;
>> +}
>> +
>> +static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>> {
>> int rec_argc, i = 0, j;
>> const char **rec_argv;
>> - char event[64];
>> - int ret;
>> + char *str;
>> + int mode = 0;
>> + int ki, ret;
>> +
>> +
>> + if (!strcmp(mem->mem_op, MEM_OPERATION_STORE))
>> + mode |= OP_STORE;
>> + else if (!strcmp(mem->mem_op, MEM_OPERATION_LOAD))
>> + mode |= OP_LOAD;
>> + else {
>> + fprintf(stderr, "unknown sampling mode: %s\n", mem->mem_op);
>> + return -1;
>> + }
>>
>> - rec_argc = argc + 4;
>> + rec_argc = argc + 6;
>> rec_argv = calloc(rec_argc + 1, sizeof(char *));
>> if (!rec_argv)
>> return -1;
>>
>> rec_argv[i++] = strdup("record");
>> - if (!strcmp(mem_operation, MEM_OPERATION_LOAD))
>> - rec_argv[i++] = strdup("-W");
>> +
>> rec_argv[i++] = strdup("-d");
>> - rec_argv[i++] = strdup("-e");
>>
>> - if (strcmp(mem_operation, MEM_OPERATION_LOAD))
>> - sprintf(event, "cpu/mem-stores/pp");
>> - else
>> - sprintf(event, "cpu/mem-loads/pp");
>> + if (mode & OP_LOAD) {
>> + rec_argv[i++] = strdup("-W");
>>
>> - rec_argv[i++] = strdup(event);
>> - for (j = 1; j < argc; j++, i++)
>> - rec_argv[i] = argv[j];
>> + rec_argv[i++] = strdup("-e");
>> +
>> + str = malloc(strlen("cpu/mem-loads/pp") + 1 + 1);
>> + if (!str) {
>> + ki = i;
>> + ret = -1;
>> + goto end;
>> + }
>> + sprintf(str, "cpu/mem-loads/%spp", get_plm(mem));
>> + rec_argv[i++] = str;
>> + }
>> +
>> + if (mode & OP_STORE) {
>> + rec_argv[i++] = strdup("-e");
>> +
>> + str = malloc(strlen("cpu/mem-stores/pp") + 1 + 1);
>> + if (!str) {
>> + ki = i;
>> + ret = -1;
>> + goto end;
>> + }
>> + sprintf(str, "cpu/mem-stores/%spp", get_plm(mem));
>> + rec_argv[i++] = str;
>> + }
>> +
>> + /* arguments after i are not malloc'd */
>> + ki = i;
>>
>> - ret = cmd_record(i, rec_argv, NULL);
>> + for (j = 1; j < argc; j++, ki++)
>> + rec_argv[ki] = argv[j];
>> +
>> + ret = cmd_record(ki, rec_argv, NULL);
>> +
>> +end:
>> + /*
>> + * XXX: free rec_argv[] entries, difficult because
>> + * cmd_record() drops some of them...
>> + */
>> free(rec_argv);
>> +
>> return ret;
>> }
>>
>> @@ -171,7 +229,7 @@ static int report_events(int argc, const char **argv, struct perf_mem *mem)
>> * there is no weight (cost) associated with stores, so don't print
>> * the column
>> */
>> - if (strcmp(mem_operation, MEM_OPERATION_LOAD))
>> + if (strcmp(mem->mem_op, MEM_OPERATION_LOAD))
>> rep_argv[i++] = strdup("--sort=mem,sym,dso,symbol_daddr,"
>> "dso_daddr,tlb,locked");
>>
>> @@ -199,7 +257,7 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
>> .input_name = "perf.data",
>> };
>> const struct option mem_options[] = {
>> - OPT_STRING('t', "type", &mem_operation,
>> + OPT_STRING('t', "type", &mem.mem_op,
>> "type", "memory operations(load/store)"),
>> OPT_BOOLEAN('D', "dump-raw-samples", &mem.dump_raw,
>> "dump raw samples in ASCII"),
>> @@ -213,13 +271,18 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
>> "separator",
>> "separator for columns, no spaces will be added"
>> " between columns '.' is reserved."),
>> + OPT_BOOLEAN('u', "user-level", &mem.user,
>> + "include user-level accesses"),
>> + OPT_BOOLEAN('k', "kernel-level", &mem.kernel,
>> + "include kernel-level accesses"),
>> OPT_END()
>> };
>>
>> argc = parse_options(argc, argv, mem_options, mem_usage,
>> PARSE_OPT_STOP_AT_NON_OPTION);
>>
>> - if (!argc || !(strncmp(argv[0], "rec", 3) || mem_operation))
>> + if (!argc || !(strncmp(argv[0], "rec", 3)
>> + || strncmp(argv[0], "rep", 3)))
>> usage_with_options(mem_usage, mem_options);
>>
>> if (!mem.input_name || !strlen(mem.input_name)) {
>> @@ -228,9 +291,12 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
>> else
>> mem.input_name = "perf.data";
>> }
>> + /* default to load only, some processors only support loads */
>> + if (!mem.mem_op)
>> + mem.mem_op = MEM_OPERATION_LOAD;
>>
>> if (!strncmp(argv[0], "rec", 3))
>> - return __cmd_record(argc, argv);
>> + return __cmd_record(argc, argv, &mem);
>> else if (!strncmp(argv[0], "rep", 3))
>> return report_events(argc, argv, &mem);
>> else
>>
>
Em Mon, Aug 26, 2013 at 04:21:44PM -0600, David Ahern escreveu:
> On 8/26/13 7:11 AM, Stephane Eranian wrote:
> >This patch adds the -u -and -k options to perf to allow
> >filtering of load/store sampling based on priv levels.
> >This may not be supported by all HW platforms.
> >+++ b/tools/perf/Documentation/perf-mem.txt
> >+-k::
> >+ Only sample loads/stores at the user level (default: user + kernel)
> >+-u::
> >+ Only sample loads/stores at the kernel level (default: user + kernel)
> Are the descriptions backwards? In the commit message yuo have -u
> means user level and -k means kernel level; the help message here
> seems backwards.
Looks like it is reversed, yes.
> >+ OPT_BOOLEAN('u', "user-level", &mem.user, "include user-level accesses"),
> >+ OPT_BOOLEAN('k', "kernel-level", &mem.kernel, "include kernel-level accesses"),
And its not clear to say that using -u will _exclude_ kernel samples,
just that it includes user samples, perf top has:
-K, --hide_kernel_symbols hide kernel symbols
-U, --hide_user_symbols hide user symbols
So perhaps we should change both to (and add this to 'report' as well):
-U, --hide_kernel_symbols hide kernel symbols
-K, --hide_user_symbols hide user symbols
To state that:
perf top -K
and
perf top --hide_user_symbols
Are equivalent and asks for 'kernel only' samples, like it seems its the
intent (filtering) of Stephane here, and seems to clarify things?
- Arnaldo
On Wed, Aug 28, 2013 at 3:27 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Mon, Aug 26, 2013 at 04:21:44PM -0600, David Ahern escreveu:
>> On 8/26/13 7:11 AM, Stephane Eranian wrote:
>
>> >This patch adds the -u -and -k options to perf to allow
>> >filtering of load/store sampling based on priv levels.
>> >This may not be supported by all HW platforms.
>
>
>> >+++ b/tools/perf/Documentation/perf-mem.txt
>> >+-k::
>> >+ Only sample loads/stores at the user level (default: user + kernel)
>> >+-u::
>> >+ Only sample loads/stores at the kernel level (default: user + kernel)
>
>> Are the descriptions backwards? In the commit message yuo have -u
>> means user level and -k means kernel level; the help message here
>> seems backwards.
>
> Looks like it is reversed, yes.
>
>> >+ OPT_BOOLEAN('u', "user-level", &mem.user, "include user-level accesses"),
>> >+ OPT_BOOLEAN('k', "kernel-level", &mem.kernel, "include kernel-level accesses"),
>
> And its not clear to say that using -u will _exclude_ kernel samples,
> just that it includes user samples, perf top has:
>
> -K, --hide_kernel_symbols hide kernel symbols
> -U, --hide_user_symbols hide user symbols
>
> So perhaps we should change both to (and add this to 'report' as well):
>
> -U, --hide_kernel_symbols hide kernel symbols
> -K, --hide_user_symbols hide user symbols
>
Well, I don't know what perf top does here but I don't want to hide
the samples. I simply don't want to collect them (do not appear
in the perf.data file). If that's what is happening in perf top, then
I'll be glad to use the same options.
> To state that:
>
> perf top -K
> and
> perf top --hide_user_symbols
>
> Are equivalent and asks for 'kernel only' samples, like it seems its the
> intent (filtering) of Stephane here, and seems to clarify things?
>
> - Arnaldo
Arnaldo,
So I just checked perf top and those 2 options are
doing user level filtering of the samples. This is different
from what I want which is hardware level filtering if avail.
On Wed, Aug 28, 2013 at 3:38 PM, Stephane Eranian <[email protected]> wrote:
> On Wed, Aug 28, 2013 at 3:27 PM, Arnaldo Carvalho de Melo
> <[email protected]> wrote:
>> Em Mon, Aug 26, 2013 at 04:21:44PM -0600, David Ahern escreveu:
>>> On 8/26/13 7:11 AM, Stephane Eranian wrote:
>>
>>> >This patch adds the -u -and -k options to perf to allow
>>> >filtering of load/store sampling based on priv levels.
>>> >This may not be supported by all HW platforms.
>>
>>
>>> >+++ b/tools/perf/Documentation/perf-mem.txt
>>> >+-k::
>>> >+ Only sample loads/stores at the user level (default: user + kernel)
>>> >+-u::
>>> >+ Only sample loads/stores at the kernel level (default: user + kernel)
>>
>>> Are the descriptions backwards? In the commit message yuo have -u
>>> means user level and -k means kernel level; the help message here
>>> seems backwards.
>>
>> Looks like it is reversed, yes.
>>
>>> >+ OPT_BOOLEAN('u', "user-level", &mem.user, "include user-level accesses"),
>>> >+ OPT_BOOLEAN('k', "kernel-level", &mem.kernel, "include kernel-level accesses"),
>>
>> And its not clear to say that using -u will _exclude_ kernel samples,
>> just that it includes user samples, perf top has:
>>
>> -K, --hide_kernel_symbols hide kernel symbols
>> -U, --hide_user_symbols hide user symbols
>>
>> So perhaps we should change both to (and add this to 'report' as well):
>>
>> -U, --hide_kernel_symbols hide kernel symbols
>> -K, --hide_user_symbols hide user symbols
>>
>
> Well, I don't know what perf top does here but I don't want to hide
> the samples. I simply don't want to collect them (do not appear
> in the perf.data file). If that's what is happening in perf top, then
> I'll be glad to use the same options.
>
>
>> To state that:
>>
>> perf top -K
>> and
>> perf top --hide_user_symbols
>>
>> Are equivalent and asks for 'kernel only' samples, like it seems its the
>> intent (filtering) of Stephane here, and seems to clarify things?
>>
>> - Arnaldo
Em Wed, Aug 28, 2013 at 03:38:28PM +0200, Stephane Eranian escreveu:
> On Wed, Aug 28, 2013 at 3:27 PM, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > So perhaps we should change both to (and add this to 'report' as well):
> >
> > -U, --hide_kernel_symbols hide kernel symbols
> > -K, --hide_user_symbols hide user symbols
> >
>
> Well, I don't know what perf top does here but I don't want to hide
> the samples. I simply don't want to collect them (do not appear
> in the perf.data file). If that's what is happening in perf top, then
> I'll be glad to use the same options.
Indeed, its for different purposes, 'perf top' when used with one of
those options will still collect samples for all priv levels and will
just toggle a flag to not zap the ones asked not to show when decaying
the samples.
When the user presses 'U' or 'K' on the UI, the flags gets toggled and
samples start being considered/zapped.
But my worry here is about consistency accross tools for the single
letter options, so perhaps if you could use:
-U collect only user level samples
-K collect only kernel level samples
I think it would stay consistent and clear, what do you think?
- Arnaldo
Arnaldo Carvalho de Melo [[email protected]] wrote:
| Em Wed, Aug 28, 2013 at 03:38:28PM +0200, Stephane Eranian escreveu:
| > On Wed, Aug 28, 2013 at 3:27 PM, Arnaldo Carvalho de Melo <[email protected]> wrote:
| > > So perhaps we should change both to (and add this to 'report' as well):
| > >
| > > -U, --hide_kernel_symbols hide kernel symbols
| > > -K, --hide_user_symbols hide user symbols
| > >
| >
| > Well, I don't know what perf top does here but I don't want to hide
| > the samples. I simply don't want to collect them (do not appear
| > in the perf.data file). If that's what is happening in perf top, then
| > I'll be glad to use the same options.
|
| Indeed, its for different purposes, 'perf top' when used with one of
| those options will still collect samples for all priv levels and will
| just toggle a flag to not zap the ones asked not to show when decaying
| the samples.
|
| When the user presses 'U' or 'K' on the UI, the flags gets toggled and
| samples start being considered/zapped.
|
| But my worry here is about consistency accross tools for the single
| letter options, so perhaps if you could use:
|
| -U collect only user level samples
| -K collect only kernel level samples
|
| I think it would stay consistent and clear, what do you think?
But, we use lower case qualifiers :u, :k to select user or kernel mode
monitoring.
perf record -e cycles # both kernel and user
perf record -e cycles:u ... # just user
(tools/perf/util/parse-events.c:
struct event_modifier {
int eu;
int ek;
int eh;
int eH;
int eG;
int precise;
int exclude_GH;
};
Will we ever need hypervisor and host monitoring for 'perf mem' ?
Or can we add a '-e' option to 'perf mem' so user can specify the events
and qualfiers same as they do for 'perf record' ?
perf mem -e mem-loads:u record .....
(this would of course expose the mem-loads and mem-stores events to
the user)
Sukadev
> But my worry here is about consistency accross tools for the single
> letter options, so perhaps if you could use:
>
> -U collect only user level samples
> -K collect only kernel level samples
Support for this would be nice for perf stat too, to use with
the implicit events (using by -d, soon -T etc.)
-Andi
--
[email protected] -- Speaking for myself only