linux-kernel - Re: [PATCH 3/3] perf tool: give user better message if precise is not supported

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:	Tue, 11 Sep 2012 07:22:32 -0600
From:	David Ahern <dsahern@...il.com>
To:	Robert Richter <robert.richter@....com>
CC:	acme@...stprotocols.net, linux-kernel@...r.kernel.org,
	peterz@...radead.org, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH 3/3] perf tool: give user better message if precise is
 not supported

On 9/11/12 3:20 AM, Robert Richter wrote:
> On 10.09.12 10:40:16, David Ahern wrote:
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -294,6 +294,11 @@ try_again:
>>   					  perf_evsel__name(pos));
>>   				rc = -err;
>>   				goto out;
>> +			} else if ((err == ENOTSUP) && (attr->precise_ip)) {
>
> It is EOPNOTSUPP, did you test this?

I do not post patches without testing them. This particular patch was 
verified in a Virtual Machine (no PEBS) and using :pG modifier.

'egrep -r ENOTSUP tools/perf' shows hits in 3 other files, so I am not 
the only one using the shortcut. I'll change it in the follow up with 
better commit messages to make it consistent with patch 2.

>
>> +				ui__error("\'precise\' request not supported. "
>> +					  "Try removing 'p' modifier\n");
>
> I would better print "... request may not be supported.", since you
> don't know for sure if this is the real cause.

Sure I'll add the 'may not be'.

>
>> +				rc = -err;
>> +				goto out;
>>   			}
>>
>>   			printf("\n");
>> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
>> index 0513aaa..0d3653b 100644
>> --- a/tools/perf/builtin-top.c
>> +++ b/tools/perf/builtin-top.c
>> @@ -975,6 +975,10 @@ try_again:
>>   				ui__error("Too many events are opened.\n"
>>   					    "Try again after reducing the number of events\n");
>>   				goto out_err;
>> +			} else if ((err == ENOTSUP) && (attr->precise_ip)) {
>> +				ui__error("\'precise\' request not supported. "
>> +					  "Try removing 'p' modifier\n");
>
> Same here.
>
>> +				goto out_err;
>
> To avoid adding more duplicate code, maybe we should start to unify
> the code by implementing this in a shared helper function.

Doing that requires additional modifications to not break perl and 
python scripts. Adding it in both commands here is consistent with all 
other open counter failures. Consolidation of those loops into a common 
base is known to do item.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/