Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1495805 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 21/57] perf c2c report: Add offset dimension key Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-05 13:00 +0200
    Re: [PATCH 21/57] perf c2c report: Add offset dimension key Jiri Olsa <jolsa@redhat.com> - 2016-10-05 14:20 +0200

#1495805 — 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@kernel.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

[toc] | [next] | [standalone]

#1495846

On Wed, Oct 05, 2016 at 07:57:07AM -0300, Arnaldo Carvalho de Melo wrote:
> 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@kernel.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?

ok, always forget this one ;-)

thanks,
jirka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web