linux-kernel - Re: [PATCH 21/57] perf c2c report: Add offset dimension key

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Oct 2016 07:57:07 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Don Zickus <dzickus@...hat.com>, Joe Mario <jmario@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Namhyung Kim <namhyung@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 21/57] perf c2c report: Add offset dimension key

Em Thu, Sep 22, 2016 at 05:36:49PM +0200, Jiri Olsa escreveu:
> Adding cacheline offset dimension key support.
> It displays cacheline offset as hex number.
> 
> Link: http://lkml.kernel.org/n/tip-m0424ye98lqveg5nopto8qww@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  tools/perf/builtin-c2c.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 335c0fd30757..7c52481ec36b 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -300,6 +300,32 @@ static int dcacheline_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  	return snprintf(hpp->buf, hpp->size, "%*s", width, hex_str(addr));
>  }
>  
> +static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> +			struct hist_entry *he)
> +{
> +	uint64_t addr = 0;
> +	int width = c2c_width(fmt, hpp, he->hists);
> +
> +	if (he->mem_info)
> +		addr = cl_offset(he->mem_info->daddr.al_addr);
> +
> +	return snprintf(hpp->buf, hpp->size, "%*s", width, hex_str(addr));

Please don't use snprintf when you actually use the return from this
function, see this part of its man page:

"If the output was trun‐ cated due to this limit, then the return value
is the number of characters (excluding the terminating null byte) which
would have been written  to the  final  string  if enough space had been
available.  Thus, a return value of size or more means that the output
was truncated.  (See also below under NOTES.)"

That is why we're using scnprintf where we need to use its return value.

I'm checking how you use the return value and changing all to scnprintf
instead, ok?

- Arnaldo


> +}
> +
> +static int64_t
> +offset_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> +	   struct hist_entry *left, struct hist_entry *right)
> +{
> +	uint64_t l = 0, r = 0;
> +
> +	if (left->mem_info)
> +		l = cl_offset(left->mem_info->daddr.addr);
> +	if (right->mem_info)
> +		r = cl_offset(right->mem_info->daddr.addr);
> +
> +	return (int64_t)(r - l);
> +}
> +
>  #define HEADER_LOW(__h)			\
>  	{				\
>  		.line[1] = {		\
> @@ -343,8 +369,17 @@ static struct c2c_dimension dim_dcacheline = {
>  	.width		= 18,
>  };
>  
> +static struct c2c_dimension dim_offset = {
> +	.header		= HEADER_BOTH("Data address", "Offset"),
> +	.name		= "offset",
> +	.cmp		= offset_cmp,
> +	.entry		= offset_entry,
> +	.width		= 18,
> +};
> +
>  static struct c2c_dimension *dimensions[] = {
>  	&dim_dcacheline,
> +	&dim_offset,
>  	NULL,
>  };
>  
> -- 
> 2.7.4