Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 28 Jan 2021 00:13:16 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Jin Yao <yao.jin@linux.intel.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        Linux-kernel@vger.kernel.org, ak@linux.intel.com,
        kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH 1/2] perf script: Support filtering by hex address
Message-ID: <20210127222905.GD284633@krava>
References: <20210124232750.19170-1-yao.jin@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210124232750.19170-1-yao.jin@linux.intel.com>
Precedence: bulk

On Mon, Jan 25, 2021 at 07:27:49AM +0800, Jin Yao wrote:

SNIP

>  	perf script --symbols=noploop,0x4007a0
>  
> +	Support filtering trace records by symbol name, start address of
> +	symbol, any hexadecimal address and address range.
> +
> +	The comparison order is:
> +	1. symbol name comparison
> +	2. symbol start address comparison.
> +	3. any hexadecimal address comparison.
> +	4. address range comparison (see --addr-range).
> +
> +--addr-range::
> +	Use with -S or --symbols to list traced records within address range.
> +
> +	For example, to list the traced records within the address range
> +	[0x4007a0, 0x0x4007a9]:
> +	perf script -S 0x4007a0 --addr-range 10
> +
>  --call-trace::
>  	Show call stream for intel_pt traces. The CPUs are interleaved, but
>  	can be filtered with -C.
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index edacfa98d073..e0feda33dbb9 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3525,6 +3525,8 @@ int cmd_script(int argc, const char **argv)
>  		    "system-wide collection from all CPUs"),
>  	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
>  		   "only consider these symbols"),
> +	OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
> +		    "Use with -S to list traced records within address range"),
>  	OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
>  			"Decode instructions from itrace", parse_insn_trace),
>  	OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index fbe8578e4c47..525b859cb445 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -645,6 +645,22 @@ struct symbol *thread__find_symbol_fb(struct thread *thread, u8 cpumode,
>  	return al->sym;
>  }
>  
> +static bool check_address_range(struct strlist *sym_list, int addr_range,
> +				struct addr_location *al)
> +{
> +	struct str_node *pos;
> +	char *endptr;
> +	u64 addr, al_addr = al->map->unmap_ip(al->map, al->addr);
> +
> +	strlist__for_each_entry(pos, sym_list) {
> +		addr = strtoull(pos->s, &endptr, 16);
> +		if (al_addr >= addr && al_addr < addr + addr_range)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  /*
>   * Callers need to drop the reference to al->thread, obtained in
>   * machine__findnew_thread()
> @@ -709,6 +725,26 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
>  			ret = strlist__has_entry(symbol_conf.sym_list,
>  						al_addr_str);
>  		}
> +		if (!ret && al->map) {
> +			snprintf(al_addr_str, sz, "0x%"PRIx64,
> +				al->map->unmap_ip(al->map, al->addr));
> +			ret = strlist__has_entry(symbol_conf.sym_list,
> +						 al_addr_str);
> +			if (!ret) {
> +				/* Check for hex without "0x" prefix */
> +				snprintf(al_addr_str, sz, "%"PRIx64,
> +					al->map->unmap_ip(al->map, al->addr));
> +				ret = strlist__has_entry(symbol_conf.sym_list,
> +							 al_addr_str);
> +			}

that seems tricky.. what if user specify more leading zeros,
I think it'd be better to search intlist instead

we could move all 'address' entries from sym_list to
new intlist (in symbol__init) and use it for this search

jirka