On 04/18/2014 06:53 PM, Ingo Molnar wrote: > > * Yan, Zheng <zheng.z....@intel.com> wrote: > >> This patch adds support for building Intel uncore driver as module. >> It adds clean-up code and config option for the Intel uncore driver >> >> Signed-off-by: Yan, Zheng <zheng.z....@intel.com> >> --- >> Changes since v1: >> move config option to "General setup/Kernel Performance Events and Counters" >> >> arch/x86/kernel/cpu/Makefile | 3 +- >> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 >> ++++++++++++++++++++++++--- >> init/Kconfig | 10 ++++++ >> 3 files changed, 56 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile >> index 7fd54f0..5d3da70 100644 >> --- a/arch/x86/kernel/cpu/Makefile >> +++ b/arch/x86/kernel/cpu/Makefile >> @@ -36,7 +36,8 @@ obj-$(CONFIG_CPU_SUP_AMD) += >> perf_event_amd_iommu.o >> endif >> obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_knc.o >> perf_event_p4.o >> obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_lbr.o >> perf_event_intel_ds.o perf_event_intel.o >> -obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore.o >> perf_event_intel_rapl.o >> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_rapl.o >> +obj-$(CONFIG_PERF_X86_INTEL_UNCORE) += perf_event_intel_uncore.o >> endif >> >> >> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c >> b/arch/x86/kernel/cpu/perf_event_intel_uncore.c >> index 618d502..fd5e883 100644 >> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c >> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c > > So I'm not willing to apply this patch until the documentation of > perf_event_intel_uncore.c is improved. Right now the file starts > without a single comment (!). Just lines after lines of code, without > any explanation what it does, what its scope is, etc.
there are comments mark the scope of code for different CPU. > > How should even a knowledgable user know about what it's all about? > > That problem percolates to the Kconfig entry as well: > Most of the codes without comments are hardware specific codes. The corresponding contents in Intel uncore documents are big tables/lists, nothing tricky/interesting. I really don't know how to comment these code. Regards Yan, Zheng >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -1502,6 +1502,16 @@ config PERF_EVENTS >> >> Say Y if unsure. >> >> +config PERF_X86_INTEL_UNCORE >> + default y >> + tristate "Intel uncore performance monitoring support" >> + depends on PERF_EVENTS && CPU_SUP_INTEL && PCI >> + help >> + This option allows kernel to access the Uncore performance >> + monitoring units of Intel processors. >> + >> + Say Y if unsure. > > Firstly, it should probably not be 'default y'. > > Secondly, the description is essentially non-existent. What does the > hardware do? Why should users care about uncore PMUs? > > This needs to be fixed before we can make this modular. > > Thanks, > > Ingo > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
