mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: zhangmengting <zhangmengting@huawei.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: <linux-kernel@vger.kernel.org>,
	<linux-perf-users@vger.kernel.org>, <peterz@infradead.org>,
	<mingo@redhat.com>, <acme@kernel.org>,
	<alexander.shishkin@linux.intel.com>, <yao.jin@linux.intel.com>,
	<ak@linux.intel.com>, <jolsa@kernel.org>, <namhyung@kernel.org>,
	<kjlx@templeofstupid.com>, <milian.wolff@kdab.com>,
	<wangnan0@huawei.com>, <huawei.libin@huawei.com>
Subject: Re: [PATCH] perf report: Fix debug messages with --call-graph option
Date: Sat, 23 Sep 2017 16:19:38 +0800	[thread overview]
Message-ID: <526e703d-8d14-f43a-e09a-cba6bdb31f65@huawei.com> (raw)
In-Reply-To: <20170922091841.GJ15856@krava>

Hi Jiri,

Thanks for the review. Agreed, the patch seems confused.

I'll add more details in the next version patch and resend it later.

Thanks,

Mengting Zhang


On 2017/9/22 17:18, Jiri Olsa wrote:
> On Fri, Sep 22, 2017 at 10:06:53AM +0800, zhangmengting wrote:
>> With --call-graph option, perf report can display call chains using
>> type, min percent threshold, optional print limit and order. And the
>> default call-graph parameter is 'graph,0.5,caller,function,percent'.
>>
>> Before this patch, 'perf report --call-graph' shows incorrect debug
>> messages as below:
>> [root@localhost perf]# ./perf report --call-graph
>> Invalid callchain mode: 0.5
>> Invalid callchain order: 0.5
>> Invalid callchain sort key: 0.5
>> Invalid callchain config key: 0.5
>> Invalid callchain mode: caller
>> Invalid callchain mode: function
>> Invalid callchain order: function
>> Invalid callchain mode: percent
>> Invalid callchain order: percent
>> Invalid callchain sort key: percent
> hum, could you please mention what was wrong with
> the code and why the change fixes them?
>
> looks like you just moved the warning out of the
> way of the report
>
> thanks,
> jirka
>
>> The patch fixes this issue.
>>
>> Signed-off-by: zhangmengting <zhangmengting@huawei.com>
>> ---
>>   tools/perf/util/callchain.c | 35 +++++++++++++++++++++--------------
>>   1 file changed, 21 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
>> index 510b513..be09d77 100644
>> --- a/tools/perf/util/callchain.c
>> +++ b/tools/perf/util/callchain.c
>> @@ -65,8 +65,6 @@ static int parse_callchain_mode(const char *value)
>>   		callchain_param.mode = CHAIN_FOLDED;
>>   		return 0;
>>   	}
>> -
>> -	pr_err("Invalid callchain mode: %s\n", value);
>>   	return -1;
>>   }
>>   
>> @@ -82,8 +80,6 @@ static int parse_callchain_order(const char *value)
>>   		callchain_param.order_set = true;
>>   		return 0;
>>   	}
>> -
>> -	pr_err("Invalid callchain order: %s\n", value);
>>   	return -1;
>>   }
>>   
>> @@ -105,8 +101,6 @@ static int parse_callchain_sort_key(const char *value)
>>   		callchain_param.branch_callstack = 1;
>>   		return 0;
>>   	}
>> -
>> -	pr_err("Invalid callchain sort key: %s\n", value);
>>   	return -1;
>>   }
>>   
>> @@ -124,8 +118,6 @@ static int parse_callchain_value(const char *value)
>>   		callchain_param.value = CCVAL_COUNT;
>>   		return 0;
>>   	}
>> -
>> -	pr_err("Invalid callchain config key: %s\n", value);
>>   	return -1;
>>   }
>>   
>> @@ -319,12 +311,27 @@ int perf_callchain_config(const char *var, const char *value)
>>   
>>   		return ret;
>>   	}
>> -	if (!strcmp(var, "print-type"))
>> -		return parse_callchain_mode(value);
>> -	if (!strcmp(var, "order"))
>> -		return parse_callchain_order(value);
>> -	if (!strcmp(var, "sort-key"))
>> -		return parse_callchain_sort_key(value);
>> +	if (!strcmp(var, "print-type")){
>> +		int ret;
>> +		ret = parse_callchain_mode(value);
>> +		if (ret == -1)
>> +			pr_err("Invalid callchain mode: %s\n", value);
>> +		return ret;
>> +	}
>> +	if (!strcmp(var, "order")){
>> +		int ret;
>> +		ret = parse_callchain_order(value);
>> +		if (ret == -1)
>> +			pr_err("Invalid callchain order: %s\n", value);
>> +		return ret;
>> +	}
>> +	if (!strcmp(var, "sort-key")){
>> +		int ret;
>> +		ret = parse_callchain_sort_key(value);
>> +		if (ret == -1)
>> +			pr_err("Invalid callchain sort key: %s\n", value);
>> +		return ret;
>> +	}
>>   	if (!strcmp(var, "threshold")) {
>>   		callchain_param.min_percent = strtod(value, &endptr);
>>   		if (value == endptr) {
>> -- 
>> 1.7.12.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> .
>

      reply	other threads:[~2017-09-23  8:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-22  2:06 zhangmengting
2017-09-22  9:18 ` Jiri Olsa
2017-09-23  8:19   ` zhangmengting [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=526e703d-8d14-f43a-e09a-cba6bdb31f65@huawei.com \
    --to=zhangmengting@huawei.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=huawei.libin@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=kjlx@templeofstupid.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=milian.wolff@kdab.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=wangnan0@huawei.com \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®