From: William C. <wc...@re...> - 2010-12-01 00:51:36
|
On 11/30/2010 01:29 PM, Maynard Johnson wrote: > William Cohen wrote: >> On 11/29/2010 03:52 PM, Maynard Johnson wrote: >>> William Cohen wrote: >>>> On 11/15/2010 04:57 PM, Maynard Johnson wrote: >>>>> William Cohen wrote: >>>>>> One thought for the Intel Westmere and other processors that support >>>>>> the arch_perfmon events is to just have the kernel module report >>>>>> i386/arch_perfmon as the cpu_type, then have user-space do cpuid or >>>>>> similar operation to identify the specific implementation of processor >>>>>> (for example i386/westmere or i386/nehalem). Having the kernel just >>>>> Hi, Will, >>>>> Your patch looks like the right direction to go. Then oprofile userspace can handle either generic "arch_perfmon" cpu_type strings or the actual processor model. >>>>> >>>>> I agree with your comment below that we should avoid the cpuid checks when using "ophelp -c cpu_type". Are you revising this patch to address that concern (along with the two other issues)? >>>>> >>>>> In John V's original patch posting, he said Andi suggested the kernel "continue to report Westmere as i386/arch_perfmon". Assuming that code is already upstream in the kernel, then your patch becomes a pre-requisite to John's patch to make it functional. >>>>> >>>>> -Maynard >>>> >>>> Hi Maynard, >>>> >>>> I would like to address the "ophelp -c cpu_type" for the patch. It would be nice to return consistent results for "ophelp -c cpu_type". I am not sure how to best differentiate between explicit specification of cpu_type with '-c cpu_type' and the implicit determination with a call to op_get_cpu_type. Original I was thinking just have to lookup when the cpu_type was i386/arch_perfmon. However, that will give varying results for "ophelp -c i386/arch_perfmon" Maybe have a flag for op_event: >>> >>> Will, sorry for the delay getting back to you on this. I don't have an arch_perfmon type of system to test this with, but we might be able to solve this problem if we use your original idea, plus a comparison of the passed cpu type to the actual cpu type. So the first few lines of 'arch_get_filter' and 'arch_num_counters' become . . . >>> >>> if ((op_get_cpu_type()!=cpu_type) && (cpu_type != CPU_ARCH_PERFMON)) >>> return -1U; >>> if (op_cpu_base_type(cpu_type) == CPU_ARCH_PERFMON) { >>> blah . . . >>> >>> Can you give this a try, if it sounds reasonable? >> >> Hi Maynard, >> >> I thought about doing something like that. However, what happens with the following running on a machine that /dev/oprofile/cpu_type is i386/arch_perfmon? >> >> ophelp -c i386/arch_perfmon >> >> The cpuid would be used and the results are going to vary based on the results of the cpuid instruction. This might be a reasonable compromise, but it would be better if the "ophelp -c" results were consistent between machines for "i386/arch_perfmon". > > Earlier this year, Andi Kleen was working on submitting a patch that would only install events files for the current architecture. Not sure why it got dropped when it seemed we were in the final stage of review, but that patch would have made 'ophelp --cpu-type" obsolete. I put out a request for comments, asking the oprofile community whether anyone might be impacted by such a change. You were the only one who responded, and you said you could live without it. If Andi does come back and dot the final "i's" and cross the final "t's" on his patch, this all becomes a moot point. IMHO, the solution above is good enough, since there's little to no chance that anyone would care about the difference in output of 'ophelp -c i386/arch_perfmon'. Do you agree? > > -Maynard > >> >> -Will Hi Maynard, Restricting thing to the same arch won't address the issue mentioned above. Different machines are going to return different results due to results of the cpuid instruction on the machine, e.g. "ophelp -c i386/nehalem" on a i386/core_2 vs "ophelp -c i386/nehalem" on a i386/nehalem. Having consistent results "would be nice", but is not a blocker. I have attached the current version of the patch that includes John Villalovos's westmere event descriptions and changes. Signed-off-by: William Cohen <wc...@re...> 2010-11-30 William Cohen <wc...@re...> * libop/op_cpu_type.c: * libop/op_cpu_type.h: * libop/op_hw_specific.h: User-space identification processors that support Intel Architectural events. 2010-11-10 John Villalovos <joh...@in...> * events/Makefile.am * events/i386/westmere/events * events/i386/westmere/unit_masks * libop/op_cpu_type.c * libop/op_cpu_type.h * libop/op_events.c * utils/ophelp.c: Add data files for support of Intel Westmere micro-architecture processors. Also update so that "ophelp -c i386/westmere" will work. This does NOT add support for Westmere into Oprofile, but helps to enable future work. -Will |