Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-16 Thread Jiri Olsa 
On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:

SNIP

>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 
> 99
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>usleep-1537  [003] d... 2745538.053737: : 104
> 
> Error case:
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
> usleep 99
>  event syntax error: '..annel.value[10...1000]=104/'
>\___ Index too large
>  Hint:Valid config terms:
>   maps:[].value=[value]
>   maps:[].event=[event]
> 
>   where  is something like [0,3...5] or [all]
>   (add -v to see detail)
>  Run 'perf list' for a list of valid events
> 
>   Usage: perf record [] []
>  or: perf record [] --  []
> 
>  -e, --eventevent selector. use 'perf list' to list available 
> events
> 
> Signed-off-by: Wang Nan 
> Cc: Alexei Starovoitov 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Jiri Olsa 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Zefan Li 
> Cc: pi3or...@163.com
> ---
>  tools/perf/util/parse-events.c |  5 ++-
>  tools/perf/util/parse-events.l | 13 ++-
>  tools/perf/util/parse-events.y | 85 
> ++

Acked-by: Jiri Olsa 

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-16 Thread Jiri Olsa 
On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:

SNIP

>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 
> 99
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>usleep-1537  [003] d... 2745538.053737: : 104
> 
> Error case:
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
> usleep 99
>  event syntax error: '..annel.value[10...1000]=104/'
>\___ Index too large
>  Hint:Valid config terms:
>   maps:[].value=[value]
>   maps:[].event=[event]
> 
>   where  is something like [0,3...5] or [all]
>   (add -v to see detail)
>  Run 'perf list' for a list of valid events
> 
>   Usage: perf record [] []
>  or: perf record [] --  []
> 
>  -e, --eventevent selector. use 'perf list' to list available 
> events
> 
> Signed-off-by: Wang Nan 
> Cc: Alexei Starovoitov 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Jiri Olsa 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Zefan Li 
> Cc: pi3or...@163.com
> ---
>  tools/perf/util/parse-events.c |  5 ++-
>  tools/perf/util/parse-events.l | 13 ++-
>  tools/perf/util/parse-events.y | 85 
> ++

Acked-by: Jiri Olsa 

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Jiri Olsa 
On Wed, Dec 16, 2015 at 10:02:28AM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/12/15 21:42, Jiri Olsa wrote:
> >On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:
> >>This patch introduce a new syntax to perf event parser:
> >>
> >>  # perf record -e 
> >> './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' usleep 2
> >why 3 dots? I'd think the standard is 2 ?
> 
> The standard (actually it is a gcc extension, not C standard) is 3 dots.
> Please have a look at [1] and [2]. Although I also think '..' is better.
> 
> So after you seeing this, do you still think we should follow our
> intuition instead of following GCC? If you still prefer '..' I'll
> change it.

I'm ok with '...'

I think I only thought about '..' as a standard because of the way I use git 
log ;-)

thanks,
jirka

> 
> Thank you.
> 
> [1] https://lkml.org/lkml/2015/11/23/4
> [2] https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html
> 
> Thank you.
> 
> >just curious
> >   [0,1,2,3..5]
> >3 made me think there's something speecial about it ;-)
> >
> >
> >jirka
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Wangnan (F) 



On 2015/12/15 21:42, Jiri Olsa wrote:

On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:

This patch introduce a new syntax to perf event parser:

  # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2

why 3 dots? I'd think the standard is 2 ?


The standard (actually it is a gcc extension, not C standard) is 3 dots.
Please have a look at [1] and [2]. Although I also think '..' is better.

So after you seeing this, do you still think we should follow our
intuition instead of following GCC? If you still prefer '..' I'll
change it.

Thank you.

[1] https://lkml.org/lkml/2015/11/23/4
[2] https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html

Thank you.


just curious
   [0,1,2,3..5]
3 made me think there's something speecial about it ;-)


jirka



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Jiri Olsa 
On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:
> This patch introduce a new syntax to perf event parser:
> 
>  # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
> usleep 2

why 3 dots? I'd think the standard is 2 ? just curious
  [0,1,2,3..5]
3 made me think there's something speecial about it ;-)


jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Jiri Olsa 
On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:
> This patch introduce a new syntax to perf event parser:
> 
>  # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
> usleep 2

why 3 dots? I'd think the standard is 2 ? just curious
  [0,1,2,3..5]
3 made me think there's something speecial about it ;-)


jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Wangnan (F) 



On 2015/12/15 21:42, Jiri Olsa wrote:

On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:

This patch introduce a new syntax to perf event parser:

  # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2

why 3 dots? I'd think the standard is 2 ?


The standard (actually it is a gcc extension, not C standard) is 3 dots.
Please have a look at [1] and [2]. Although I also think '..' is better.

So after you seeing this, do you still think we should follow our
intuition instead of following GCC? If you still prefer '..' I'll
change it.

Thank you.

[1] https://lkml.org/lkml/2015/11/23/4
[2] https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html

Thank you.


just curious
   [0,1,2,3..5]
3 made me think there's something speecial about it ;-)


jirka



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-15 Thread Jiri Olsa 
On Wed, Dec 16, 2015 at 10:02:28AM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/12/15 21:42, Jiri Olsa wrote:
> >On Mon, Dec 14, 2015 at 10:39:19AM +, Wang Nan wrote:
> >>This patch introduce a new syntax to perf event parser:
> >>
> >>  # perf record -e 
> >> './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' usleep 2
> >why 3 dots? I'd think the standard is 2 ?
> 
> The standard (actually it is a gcc extension, not C standard) is 3 dots.
> Please have a look at [1] and [2]. Although I also think '..' is better.
> 
> So after you seeing this, do you still think we should follow our
> intuition instead of following GCC? If you still prefer '..' I'll
> change it.

I'm ok with '...'

I think I only thought about '..' as a standard because of the way I use git 
log ;-)

thanks,
jirka

> 
> Thank you.
> 
> [1] https://lkml.org/lkml/2015/11/23/4
> [2] https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html
> 
> Thank you.
> 
> >just curious
> >   [0,1,2,3..5]
> >3 made me think there's something speecial about it ;-)
> >
> >
> >jirka
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-14 Thread Wang Nan 
This patch introduce a new syntax to perf event parser:

 # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2

By utilizing the basic facilities in bpf-loader.c which allow setting
different slots in a BPF map separately, the newly introduced syntax
allows perf to control specific elements in a BPF map.

Test result:

 # cat ./test_bpf_map_3.c
 / BEGIN **/
 #include 
 #define SEC(NAME) __attribute__((section(NAME), used))
 struct bpf_map_def {
unsigned int type;
unsigned int key_size;
unsigned int value_size;
unsigned int max_entries;
 };
 static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
(void *)BPF_FUNC_map_lookup_elem;
 static int (*trace_printk)(const char *fmt, int fmt_size, ...) =
(void *)BPF_FUNC_trace_printk;
 struct bpf_map_def SEC("maps") channel = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(int),
.value_size = sizeof(unsigned char),
.max_entries = 100,
 };
 SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
 int func(void *ctx, int err, long nsec)
 {
char fmt[] = "%ld\n";
long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
int key = (int)usec;
unsigned char *pval = map_lookup_elem(, );

if (!pval)
return 0;
trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
return 0;
 }
 char _license[] SEC("license") = "GPL";
 int _version SEC("version") = LINUX_VERSION_CODE;
 /* END ***/

Normal case:
 # echo "" > /sys/kernel/debug/tracing/trace
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 3
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 15
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 99
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
   usleep-1537  [003] d... 2745538.053737: : 104

Error case:
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
usleep 99
 event syntax error: '..annel.value[10...1000]=104/'
   \___ Index too large
 Hint:  Valid config terms:
maps:[].value=[value]
maps:[].event=[event]

where  is something like [0,3...5] or [all]
(add -v to see detail)
 Run 'perf list' for a list of valid events

  Usage: perf record [] []
 or: perf record [] --  []

 -e, --eventevent selector. use 'perf list' to list available 
events

Signed-off-by: Wang Nan 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Zefan Li 
Cc: pi3or...@163.com
---
 tools/perf/util/parse-events.c |  5 ++-
 tools/perf/util/parse-events.l | 13 ++-
 tools/perf/util/parse-events.y | 85 ++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index af3d657..abdf551 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 sizeof(errbuf));
data->error->help = strdup(
 "Hint:\tValid config terms:\n"
-" \tmaps:[].value=[value]\n"
-" \tmaps:[].event=[event]\n"
+" \tmaps:[].value=[value]\n"
+" \tmaps:[].event=[event]\n"
 "\n"
+" \twhere  is something like [0,3...5] or [all]\n"
 " \t(add -v to see detail)");
data->error->str = strdup(errbuf);
if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4387728..8bb3437 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -9,8 +9,8 @@
 %{
 #include 
 #include "../perf.h"
-#include

[PATCH v5 10/14] perf tools: Enable indices setting syntax for BPF maps

2015-12-14 Thread Wang Nan 
This patch introduce a new syntax to perf event parser:

 # perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2

By utilizing the basic facilities in bpf-loader.c which allow setting
different slots in a BPF map separately, the newly introduced syntax
allows perf to control specific elements in a BPF map.

Test result:

 # cat ./test_bpf_map_3.c
 / BEGIN **/
 #include 
 #define SEC(NAME) __attribute__((section(NAME), used))
 struct bpf_map_def {
unsigned int type;
unsigned int key_size;
unsigned int value_size;
unsigned int max_entries;
 };
 static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
(void *)BPF_FUNC_map_lookup_elem;
 static int (*trace_printk)(const char *fmt, int fmt_size, ...) =
(void *)BPF_FUNC_trace_printk;
 struct bpf_map_def SEC("maps") channel = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(int),
.value_size = sizeof(unsigned char),
.max_entries = 100,
 };
 SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
 int func(void *ctx, int err, long nsec)
 {
char fmt[] = "%ld\n";
long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
int key = (int)usec;
unsigned char *pval = map_lookup_elem(, );

if (!pval)
return 0;
trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
return 0;
 }
 char _license[] SEC("license") = "GPL";
 int _version SEC("version") = LINUX_VERSION_CODE;
 /* END ***/

Normal case:
 # echo "" > /sys/kernel/debug/tracing/trace
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 3
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 15
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 99
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
   usleep-1537  [003] d... 2745538.053737: : 104

Error case:
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
usleep 99
 event syntax error: '..annel.value[10...1000]=104/'
   \___ Index too large
 Hint:  Valid config terms:
maps:[].value=[value]
maps:[].event=[event]

where  is something like [0,3...5] or [all]
(add -v to see detail)
 Run 'perf list' for a list of valid events

  Usage: perf record [] []
 or: perf record [] --  []

 -e, --eventevent selector. use 'perf list' to list available 
events

Signed-off-by: Wang Nan 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Zefan Li 
Cc: pi3or...@163.com
---
 tools/perf/util/parse-events.c |  5 ++-
 tools/perf/util/parse-events.l | 13 ++-
 tools/perf/util/parse-events.y | 85 ++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index af3d657..abdf551 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 sizeof(errbuf));
data->error->help = strdup(
 "Hint:\tValid config terms:\n"
-" \tmaps:[].value=[value]\n"
-" \tmaps:[].event=[event]\n"
+" \tmaps:[].value=[value]\n"
+" \tmaps:[].event=[event]\n"
 "\n"
+" \twhere  is something like [0,3...5] or [all]\n"
 " \t(add -v to see detail)");
data->error->str = strdup(errbuf);
if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4387728..8bb3437 100644
---