Re: [PATCH 2/3] perf tools: allow user to specify hardware breakpoint bp_len

[Frederic Weisbecker <fweisbec@gmail.com>]

On Wed, Oct 02, 2013 at 11:11:07AM -0500, suravee.suthikulpanit@amd.com wrote:
> From: Jacob Shin <jacob.w.shin@gmail.com>
> 
> Currently bp_len is given a default value of 4. Allow user to override it:
> 
>   $ perf stat -e mem:0x1000/8
>                             ^
>                             bp_len
> 
> If no value is given, it will default to 4 as it did before.
> 
> Signed-off-by: Jacob Shin <jacob.w.shin@gmail.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  tools/perf/Documentation/perf-record.txt |  7 +++++--
>  tools/perf/util/parse-events.c           | 17 +++++++----------
>  tools/perf/util/parse-events.h           |  2 +-
>  tools/perf/util/parse-events.l           |  1 +
>  tools/perf/util/parse-events.y           | 26 ++++++++++++++++++++++++--
>  5 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index e297b74..e07491f 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -33,12 +33,15 @@ OPTIONS
>          - a raw PMU event (eventsel+umask) in the form of rNNN where NNN is a
>  	  hexadecimal event descriptor.
>  
> -        - a hardware breakpoint event in the form of '\mem:addr[:access]'
> +        - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
>            where addr is the address in memory you want to break in.
>            Access is the memory access type (read, write, execute) it can
> -          be passed as follows: '\mem:addr[:[r][w][x]]'.
> +          be passed as follows: '\mem:addr[:[r][w][x]]'. len is the range,
> +          number of bytes from specified addr, which the breakpoint will cover.
>            If you want to profile read-write accesses in 0x1000, just set
>            'mem:0x1000:rw'.
> +          If you want to profile write accesses in [0x1000~1008), just set
> +          'mem:0x1000/8:w'.
>  
>  --filter=<filter>::
>          Event filter.
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 9812531..65d81ae 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -515,7 +515,7 @@ do {					\
>  }
>  
>  int parse_events_add_breakpoint(struct list_head *list, int *idx,
> -				void *ptr, char *type)
> +				void *ptr, char *type, u64 len)
>  {
>  	struct perf_event_attr attr;
>  
> @@ -525,14 +525,11 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
>  	if (parse_breakpoint_type(type, &attr))
>  		return -EINVAL;
>  
> -	/*
> -	 * We should find a nice way to override the access length
> -	 * Provide some defaults for now
> -	 */
> -	if (attr.bp_type == HW_BREAKPOINT_X)
> -		attr.bp_len = sizeof(long);
> -	else
> -		attr.bp_len = HW_BREAKPOINT_LEN_4;
> +	/* Provide some defaults if len is not specified */
> +	if (!len)
> +		len = attr.bp_type == HW_BREAKPOINT_X ? sizeof(long) :
> +							HW_BREAKPOINT_LEN_4;

Why is LEN_4 affected in the type?

> +	attr.bp_len = len;

Thanks.