Re: [PATCH] oprofile: Only install events files for the current architecture

[Andi K. <an...@fi...>]

Maynard Johnson <may...@us...> writes:

> Andi Kleen wrote:
>> On Fri, Jul 02, 2010 at 11:35:15AM -0500, Maynard Johnson wrote:
>>> Andi Kleen wrote:
>>>> On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote:
>>>>> On 06/24/2010 9:43 AM, Andi Kleen wrote:
>>>>>>> Well, as far as I can tell, the only problem we'd have in this
>>>>>>> regard is due to the aggregation of i386 and x86_64 in the
>>>>>>> events/Makefile.am.   The ppc and ppc64 event files are separated in
>>>>>>
>>>>>> s390 / s390x, mips, mips64, ... can come out of uname.
>>>>>
>>>>> I don't see what s390 has to do with this since only timer-based
>>>>> profiling is supported on that architecture.  And there are no
>>>>> mips64 oprofile events.  So I don't grok what it is you're trying to
>>>>> say here.
>>>>
>>>> uname returns mips64, ppc64 etc., the event files are mips without 64 etc. 
>>>> so it would not print the message.
>>>
>>> Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message:
>> 
>> My original patch worked on all architectures.
>
> No, your patch does *NOT* work.   In your testing below, you're passing in '--cpu-type=i386-pii'.  That's not a valid cpu type. It should be "i386/pii".

That was your patch, not mine. But yes I used the wrong command line.

Here is a updated test with your patch:
 
./utils/ophelp --cpu-type=i386/pii
Unable to open cpu_type file for reading
Make sure you have done opcontrol --init
--cpu-type for non native architectures is not supported

So it seems to work for this case.

> And what about mips?  You stated above "uname returns mips64, ppc64 etc., the event files are mips without 64 etc. so it would not print the message."  What exactly do you mean by that?  I took it to mean there would be some kind of breakage with your patch on mips.  


I'm just saying that there's the same situation as on x86 because the
same userland runs on different 'uname -m's and we could probably spend
50-100 lines of code just for that single usage() line.

For very small values of  "breakage" as in a extremly obscure usage() line for 
a rarely to never used option may not be printed.

-Andi

-- 
ak...@li... -- Speaking for myself only.