[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3627ceef-fc72-1951-c1b6-4c59525a1afa@linux.intel.com>
Date: Tue, 1 Jun 2021 09:10:39 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
mingo@...hat.com, alexander.shishkin@...ux.intel.com,
Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v2 6/8] perf mem: Fix wrong verbose output for recording
events
Hi Jiri,
On 6/1/2021 6:42 AM, Jiri Olsa wrote:
> On Thu, May 27, 2021 at 08:16:08AM +0800, Jin Yao wrote:
>> Current code:
>>
>> for (j = 0; j < argc; j++, i++)
>> rec_argv[i] = argv[j];
>>
>> if (verbose > 0) {
>> pr_debug("calling: record ");
>>
>> while (rec_argv[j]) {
>> pr_debug("%s ", rec_argv[j]);
>> j++;
>> }
>> pr_debug("\n");
>> }
>>
>> The entries of argv[] are copied to the end of rec_argv[], not
>> copied to the beginning of rec_argv[]. So the index j at
>> rec_argv[] doesn't point to the first event.
>>
>> Now we record the start index and end index for events in rec_argv[],
>> and print them if verbose is enabled.
>>
>> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
>> ---
>> v2:
>> - New in v2.
>>
>> tools/perf/builtin-mem.c | 8 +++++---
>
> hi,
> do we need the same in c2c as well?
>
> jirka
>
perf c2c is a bit different. It sets 'j = 0;' before 'while' loop so it prints all of rec_argv[].
In test,
# perf c2c record -vvv -a -- sleep 1
calling: record -W -d --phys-data --sample-cpu -e
{cpu_core/mem-loads-aux/,cpu_core/mem-loads,ldlat=30/}:P -e cpu_atom/mem-loads,ldlat=30/P -e
cpu_core/mem-stores/P -e cpu_atom/mem-stores/P -a sleep 1
The verbose output looks OK.
Thanks
Jin Yao
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
>> index 6b633df458c2..0fd2a74dbaca 100644
>> --- a/tools/perf/builtin-mem.c
>> +++ b/tools/perf/builtin-mem.c
>> @@ -65,6 +65,7 @@ static const char * const *record_mem_usage = __usage;
>> static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>> {
>> int rec_argc, i = 0, j, tmp_nr = 0;
>> + int start, end;
>> const char **rec_argv;
>> char **rec_tmp;
>> int ret;
>> @@ -144,9 +145,11 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>> if (mem->data_page_size)
>> rec_argv[i++] = "--data-page-size";
>>
>> + start = i;
>> ret = perf_mem_events__record_args(rec_argv, &i, rec_tmp, &tmp_nr);
>> if (ret)
>> goto out;
>> + end = i;
>>
>> if (all_user)
>> rec_argv[i++] = "--all-user";
>> @@ -160,10 +163,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>> if (verbose > 0) {
>> pr_debug("calling: record ");
>>
>> - while (rec_argv[j]) {
>> + for (j = start; j < end; j++)
>> pr_debug("%s ", rec_argv[j]);
>> - j++;
>> - }
>> +
>> pr_debug("\n");
>> }
>>
>> --
>> 2.17.1
>>
>
Powered by blists - more mailing lists