2011/9/25 David Daney <david.s.daney@xxxxxxxxx>: > On 09/23/2011 07:50 PM, Deng-Cheng Zhu wrote: >> >> 2011/9/23 David Daney<david.daney@xxxxxxxxxx> >>> >>> The contents of arch/mips/kernel/perf_event.c and >>> arch/mips/kernel/perf_event_mipsxx.c were divided in a seemingly ad >>> hoc manner, with the first including the second. >>> >>> I moved all the hardware counter support code to perf_event_mipsxx.c >>> and removed the gating #ifdefs to the Kconfig and Makefile. >>> >>> Now perf_event.c contains only the callchain support, everything else >>> is in perf_event_mipsxx.c >>> >> Sorry for my late comment. I personally don't think it's a bad idea to >> use the original gating #ifdefs, because it allows sharing common code >> among different types of MIPS PMUs. Also, using CPU types as compiling >> conditions seems make sense. If you move the common hunk to >> perf_event_mipsxx.c, other CPUs like loognson series will have to >> duplicate >> these stuff. >> > > I disagree, and here is why: > > Almost all the the code is mipsxx specific. > > If Loongson has a PMU that can reuse this code, it can just be added to > perf_event_mipsxx.c along with all the other mips compatible PMUs > > If this is not feasible, then it can have its own file and and common code > can be removed to a common place at that time. > > In any event, I have not seen any Loongson PMU patches, if someone has such > patches I would be happy to consider changes that would make the kernel as a > whole cleaner. But preventing cleanup and removal of a ton of #ifdefery in > hope that Loongson patches may someday want something different is not what > I would call a good way forward. [DC]: Here's a patch set to support Loongson2 sent by Wu Zhangjin based on MIPS Perf-events v2: http://www.linux-mips.org/archives/linux-mips/2010-04/msg00161.html It used the "skeleton code (perf_event.c) + PMU specific code (perf_event_$PMU.c)" model. The current perf_event.c is right the place you mentioned as "a common place" to accommodate common code. The naming convention here came from the current Oprofile code which contains op_model_mipsxx.c, op_model_loongson2.c and op_model_rm9000.c. I think it will confuse people if we put Loongson perf code into perf_event_mipsxx.c. Deng-Cheng
