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

[David Ahern <dsahern@gmail.com>]

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