[PATCH 1/4] perf: allow building PMU driver as module

[PATCH 1/4] perf: allow building PMU driver as module

[Yan, Zheng]

This patch adds support for building PMU driver as module. It exports
function perf_pmu_{register,unregister} and adds reference tracking
for PMU driver module. When PMU driver is built as module, each active
event of the PMU holds a reference to the driver module.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index e56b07f..4f0710e 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -172,6 +172,7 @@ struct perf_event;
 struct pmu {
 	struct list_head		entry;
 
+	struct module			*module;
 	struct device			*dev;
 	const struct attribute_group	**attr_groups;
 	const char			*name;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 661951a..1616b7d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -39,6 +39,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/mm_types.h>
 #include <linux/cgroup.h>
+#include <linux/module.h>
 
 #include "internal.h"
 
@@ -3244,6 +3245,9 @@ static void __free_event(struct perf_event *event)
 	if (event->ctx)
 		put_ctx(event->ctx);
 
+	if (event->pmu)
+		module_put(event->pmu->module);
+
 	call_rcu(&event->rcu_head, free_event_rcu);
 }
 static void free_event(struct perf_event *event)
@@ -6566,6 +6570,7 @@ free_pdc:
 	free_percpu(pmu->pmu_disable_count);
 	goto unlock;
 }
+EXPORT_SYMBOL_GPL(perf_pmu_register);
 
 void perf_pmu_unregister(struct pmu *pmu)
 {
@@ -6587,6 +6592,7 @@ void perf_pmu_unregister(struct pmu *pmu)
 	put_device(pmu->dev);
 	free_pmu_context(pmu);
 }
+EXPORT_SYMBOL_GPL(perf_pmu_unregister);
 
 struct pmu *perf_init_event(struct perf_event *event)
 {
@@ -6600,6 +6606,10 @@ struct pmu *perf_init_event(struct perf_event *event)
 	pmu = idr_find(&pmu_idr, event->attr.type);
 	rcu_read_unlock();
 	if (pmu) {
+		if (!try_module_get(pmu->module)) {
+			pmu = ERR_PTR(-ENODEV);
+			goto unlock;
+		}
 		event->pmu = pmu;
 		ret = pmu->event_init(event);
 		if (ret)
@@ -6608,6 +6618,10 @@ struct pmu *perf_init_event(struct perf_event *event)
 	}
 
 	list_for_each_entry_rcu(pmu, &pmus, entry) {
+		if (!try_module_get(pmu->module)) {
+			pmu = ERR_PTR(-ENODEV);
+			goto unlock;
+		}
 		event->pmu = pmu;
 		ret = pmu->event_init(event);
 		if (!ret)
@@ -6786,6 +6800,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 err_pmu:
 	if (event->destroy)
 		event->destroy(event);
+	module_put(pmu->module);
 err_ns:
 	if (event->ns)
 		put_pid_ns(event->ns);
-- 
1.8.5.3

[PATCH 2/4] hrtimer: export __hrtimer_start_range_ns

[Yan, Zheng]

export __hrtimer_start_range_ns() to allow building perf Intel uncore
driver as module

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 kernel/hrtimer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 0909436..650bf38 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1030,6 +1030,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns);
 
 /**
  * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
-- 
1.8.5.3

Re: [PATCH 2/4] hrtimer: export __hrtimer_start_range_ns

[Peter Zijlstra]

On Tue, Mar 18, 2014 at 04:56:42PM +0800, Yan, Zheng wrote:
> export __hrtimer_start_range_ns() to allow building perf Intel uncore
> driver as module

Bah; you didn't cc the hrtimer maintainer.

Thomas, ack on this?

> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  kernel/hrtimer.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 0909436..650bf38 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -1030,6 +1030,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns);
>  
>  /**
>   * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
> -- 
> 1.8.5.3
> 

Re: [PATCH 2/4] hrtimer: export __hrtimer_start_range_ns

[Thomas Gleixner]

On Wed, 19 Mar 2014, Peter Zijlstra wrote:

> On Tue, Mar 18, 2014 at 04:56:42PM +0800, Yan, Zheng wrote:
> > export __hrtimer_start_range_ns() to allow building perf Intel uncore
> > driver as module
> 
> Bah; you didn't cc the hrtimer maintainer.
> 
> Thomas, ack on this?

Yup.
 
> > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> > ---
> >  kernel/hrtimer.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> > index 0909436..650bf38 100644
> > --- a/kernel/hrtimer.c
> > +++ b/kernel/hrtimer.c
> > @@ -1030,6 +1030,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
> >  
> >  	return ret;
> >  }
> > +EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns);
> >  
> >  /**
> >   * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
> > -- 
> > 1.8.5.3
> > 
> 

Re: [PATCH 2/4] hrtimer: export __hrtimer_start_range_ns

[Peter Zijlstra]

On Wed, Mar 19, 2014 at 10:53:34AM +0100, Thomas Gleixner wrote:
> On Wed, 19 Mar 2014, Peter Zijlstra wrote:
> 
> > On Tue, Mar 18, 2014 at 04:56:42PM +0800, Yan, Zheng wrote:
> > > export __hrtimer_start_range_ns() to allow building perf Intel uncore
> > > driver as module
> > 
> > Bah; you didn't cc the hrtimer maintainer.
> > 
> > Thomas, ack on this?
> 
> Yup.

Thanks!

[tip:perf/core] hrtimer: Export __hrtimer_start_range_ns()

[tip-bot for Yan, Zheng]

Commit-ID:  8588a2bbddc524325d84d1d6996758e9242e4ffc
Gitweb:     http://git.kernel.org/tip/8588a2bbddc524325d84d1d6996758e9242e4ffc
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Tue, 18 Mar 2014 16:56:42 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 18 Apr 2014 12:54:46 +0200

hrtimer: Export __hrtimer_start_range_ns()

Export __hrtimer_start_range_ns() to allow building perf Intel uncore
driver as a module.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1395133004-23205-2-git-send-email-zheng.z.yan@intel.com
Cc: eranian@google.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/hrtimer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index d55092c..53d2682 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1017,6 +1017,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns);
 
 /**
  * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU

[PATCH 3/4] perf/x86: export perf_assign_events

[Yan, Zheng]

export perf_assign_events to allow building perf Intel uncore driver
as module

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 arch/x86/kernel/cpu/perf_event.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index ae407f7..89f3b7c 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -721,6 +721,7 @@ int perf_assign_events(struct perf_event **events, int n,
 
 	return sched.state.unassigned;
 }
+EXPORT_SYMBOL_GPL(perf_assign_events);
 
 int x86_schedule_events(struct cpu_hw_events *cpuc, int n, int *assign)
 {
-- 
1.8.5.3

[tip:perf/core] perf/x86: Export perf_assign_events()

[tip-bot for Yan, Zheng]

Commit-ID:  4a3dc121d3c370625575247bf714db3f601d83e9
Gitweb:     http://git.kernel.org/tip/4a3dc121d3c370625575247bf714db3f601d83e9
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Tue, 18 Mar 2014 16:56:43 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 18 Apr 2014 12:54:46 +0200

perf/x86: Export perf_assign_events()

export perf_assign_events to allow building perf Intel uncore driver
as module

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1395133004-23205-3-git-send-email-zheng.z.yan@intel.com
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: eranian@google.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index ae407f7..89f3b7c 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -721,6 +721,7 @@ int perf_assign_events(struct perf_event **events, int n,
 
 	return sched.state.unassigned;
 }
+EXPORT_SYMBOL_GPL(perf_assign_events);
 
 int x86_schedule_events(struct cpu_hw_events *cpuc, int n, int *assign)
 {

[PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Yan, Zheng]

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.yan@intel.com>
---
 arch/x86/Kconfig                              |  7 ++++
 arch/x86/configs/i386_defconfig               |  1 +
 arch/x86/configs/x86_64_defconfig             |  1 +
 arch/x86/kernel/cpu/Makefile                  |  3 +-
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 ++++++++++++++++++++++++---
 5 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..8e4e247 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1103,6 +1103,13 @@ config X86_CPUID
 	  with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
 	  /dev/cpu/31/cpuid.
 
+config X86_INTEL_UNCORE
+	tristate "Intel uncore performance monitoring support"
+	depends on CPU_SUP_INTEL
+	---help---
+	  This adds a driver to access the Uncore performance monitoring Units
+	  of Intel processors.
+
 choice
 	prompt "High Memory Support"
 	default HIGHMEM64G if X86_NUMAQ
diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index a7fef26..15ad0c3 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -49,6 +49,7 @@ CONFIG_MICROCODE=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
+CONFIG_X86_INTEL_UNCORE=y
 CONFIG_HIGHPTE=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
 # CONFIG_MTRR_SANITIZER is not set
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index c1119d4..cc55ed8 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -47,6 +47,7 @@ CONFIG_MICROCODE=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
+CONFIG_X86_INTEL_UNCORE=y
 CONFIG_NUMA=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
 # CONFIG_MTRR_SANITIZER is not set
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 7fd54f0..434af30 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_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
@@ -3643,6 +3643,7 @@ static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
 
 	if (!pmu->type->pmu) {
 		pmu->pmu = (struct pmu) {
+			.module		= THIS_MODULE,
 			.attr_groups	= pmu->type->attr_groups,
 			.task_ctx_nr	= perf_invalid_context,
 			.event_init	= uncore_pmu_event_init,
@@ -3654,6 +3655,7 @@ static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
 		};
 	} else {
 		pmu->pmu = *pmu->type->pmu;
+		pmu->pmu.module = THIS_MODULE;
 		pmu->pmu.attr_groups = pmu->type->attr_groups;
 	}
 
@@ -3671,7 +3673,7 @@ static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
 	return ret;
 }
 
-static void __init uncore_type_exit(struct intel_uncore_type *type)
+static void uncore_type_exit(struct intel_uncore_type *type)
 {
 	int i;
 
@@ -3683,7 +3685,7 @@ static void __init uncore_type_exit(struct intel_uncore_type *type)
 	type->events_group = NULL;
 }
 
-static void __init uncore_types_exit(struct intel_uncore_type **types)
+static void uncore_types_exit(struct intel_uncore_type **types)
 {
 	int i;
 	for (i = 0; types[i]; i++)
@@ -3910,7 +3912,7 @@ static int __init uncore_pci_init(void)
 	return ret;
 }
 
-static void __init uncore_pci_exit(void)
+static void uncore_pci_exit(void)
 {
 	if (pcidrv_registered) {
 		pcidrv_registered = false;
@@ -4293,4 +4295,42 @@ static int __init intel_uncore_init(void)
 fail:
 	return ret;
 }
-device_initcall(intel_uncore_init);
+
+static void __exit uncore_pmus_unregister(void)
+{
+	struct intel_uncore_pmu *pmu;
+	struct intel_uncore_type *type;
+	int i, j;
+
+	for (i = 0; msr_uncores[i]; i++) {
+		type = msr_uncores[i];
+		for (j = 0; j < type->num_boxes; j++) {
+			pmu = &type->pmus[j];
+			perf_pmu_unregister(&pmu->pmu);
+		}
+	}
+
+	for (i = 0; pci_uncores[i]; i++) {
+		type = pci_uncores[i];
+		for (j = 0; j < type->num_boxes; j++) {
+			pmu = &type->pmus[j];
+			perf_pmu_unregister(&pmu->pmu);
+		}
+	}
+}
+
+static void __exit intel_uncore_exit(void)
+{
+	uncore_pmus_unregister();
+
+	unregister_cpu_notifier(&uncore_cpu_nb);
+
+	uncore_pci_exit();
+
+	uncore_types_exit(msr_uncores);
+}
+
+module_init(intel_uncore_init);
+module_exit(intel_uncore_exit);
+MODULE_DESCRIPTION("Intel Uncore Performance Monitoring driver");
+MODULE_LICENSE("GPL");
-- 
1.8.5.3

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Peter Zijlstra]

On Tue, Mar 18, 2014 at 04:56:44PM +0800, Yan, Zheng 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.yan@intel.com>
> ---
>  arch/x86/Kconfig                              |  7 ++++
>  arch/x86/configs/i386_defconfig               |  1 +
>  arch/x86/configs/x86_64_defconfig             |  1 +
>  arch/x86/kernel/cpu/Makefile                  |  3 +-
>  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 ++++++++++++++++++++++++---
>  5 files changed, 55 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0af5250..8e4e247 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1103,6 +1103,13 @@ config X86_CPUID
>  	  with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
>  	  /dev/cpu/31/cpuid.
>  
> +config X86_INTEL_UNCORE
> +	tristate "Intel uncore performance monitoring support"
> +	depends on CPU_SUP_INTEL
> +	---help---
> +	  This adds a driver to access the Uncore performance monitoring Units
> +	  of Intel processors.
> +

Ingo do you agree with this config option placement?

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Mar 18, 2014 at 04:56:44PM +0800, Yan, Zheng 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.yan@intel.com>
> > ---
> >  arch/x86/Kconfig                              |  7 ++++
> >  arch/x86/configs/i386_defconfig               |  1 +
> >  arch/x86/configs/x86_64_defconfig             |  1 +
> >  arch/x86/kernel/cpu/Makefile                  |  3 +-
> >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 ++++++++++++++++++++++++---
> >  5 files changed, 55 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 0af5250..8e4e247 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1103,6 +1103,13 @@ config X86_CPUID
> >  	  with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
> >  	  /dev/cpu/31/cpuid.
> >  
> > +config X86_INTEL_UNCORE
> > +	tristate "Intel uncore performance monitoring support"
> > +	depends on CPU_SUP_INTEL
> > +	---help---
> > +	  This adds a driver to access the Uncore performance monitoring Units
> > +	  of Intel processors.
> > +
> 
> Ingo do you agree with this config option placement?

Yeah, the modularization looks mostly good to me, but its name should 
lie within the PERF namespace, i.e. CONFIG_PERF_PMU_X86_INTEL_UNCORE=y.

Thanks,

	Ingo

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Peter Zijlstra]

On Wed, Mar 19, 2014 at 12:32:54PM +0100, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, Mar 18, 2014 at 04:56:44PM +0800, Yan, Zheng 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.yan@intel.com>
> > > ---
> > >  arch/x86/Kconfig                              |  7 ++++
> > >  arch/x86/configs/i386_defconfig               |  1 +
> > >  arch/x86/configs/x86_64_defconfig             |  1 +
> > >  arch/x86/kernel/cpu/Makefile                  |  3 +-
> > >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 ++++++++++++++++++++++++---
> > >  5 files changed, 55 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 0af5250..8e4e247 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -1103,6 +1103,13 @@ config X86_CPUID
> > >  	  with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
> > >  	  /dev/cpu/31/cpuid.
> > >  
> > > +config X86_INTEL_UNCORE
> > > +	tristate "Intel uncore performance monitoring support"
> > > +	depends on CPU_SUP_INTEL
> > > +	---help---
> > > +	  This adds a driver to access the Uncore performance monitoring Units
> > > +	  of Intel processors.
> > > +
> > 
> > Ingo do you agree with this config option placement?
> 
> Yeah, the modularization looks mostly good to me, but its name should 
> lie within the PERF namespace, i.e. CONFIG_PERF_PMU_X86_INTEL_UNCORE=y.

Ah; but do you want this config option hidden in the "Processor type and
features" menu or would they be better in "General setup/Kernel
Performance Events and Counters" ?

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Mar 19, 2014 at 12:32:54PM +0100, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > On Tue, Mar 18, 2014 at 04:56:44PM +0800, Yan, Zheng 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.yan@intel.com>
> > > > ---
> > > >  arch/x86/Kconfig                              |  7 ++++
> > > >  arch/x86/configs/i386_defconfig               |  1 +
> > > >  arch/x86/configs/x86_64_defconfig             |  1 +
> > > >  arch/x86/kernel/cpu/Makefile                  |  3 +-
> > > >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 48 ++++++++++++++++++++++++---
> > > >  5 files changed, 55 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index 0af5250..8e4e247 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -1103,6 +1103,13 @@ config X86_CPUID
> > > >  	  with major 203 and minors 0 to 31 for /dev/cpu/0/cpuid to
> > > >  	  /dev/cpu/31/cpuid.
> > > >  
> > > > +config X86_INTEL_UNCORE
> > > > +	tristate "Intel uncore performance monitoring support"
> > > > +	depends on CPU_SUP_INTEL
> > > > +	---help---
> > > > +	  This adds a driver to access the Uncore performance monitoring Units
> > > > +	  of Intel processors.
> > > > +
> > > 
> > > Ingo do you agree with this config option placement?
> > 
> > Yeah, the modularization looks mostly good to me, but its name 
> > should lie within the PERF namespace, i.e. 
> > CONFIG_PERF_PMU_X86_INTEL_UNCORE=y.
> 
> Ah; but do you want this config option hidden in the "Processor type 
> and features" menu or would they be better in "General setup/Kernel 
> Performance Events and Counters" ?

What's your preference? There's a vali argument for both: the uncore 
driver is part of the CPU, but it's also a perf driver.

Thanks,

	Ingo

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Peter Zijlstra]

On Wed, Mar 19, 2014 at 12:53:23PM +0100, Ingo Molnar wrote:
> > Ah; but do you want this config option hidden in the "Processor type 
> > and features" menu or would they be better in "General setup/Kernel 
> > Performance Events and Counters" ?
> 
> What's your preference? There's a vali argument for both: the uncore 
> driver is part of the CPU, but it's also a perf driver.

My initial thought was having it in the perf menu because that's
relatively easy to find and not big and cluttered (yet) like the arch
menu is.

I actually had to double check I had the right patches applied because I
couldn't find it the first time.

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Mar 19, 2014 at 12:53:23PM +0100, Ingo Molnar wrote:
> > >
> > > Ah; but do you want this config option hidden in the "Processor 
> > > type and features" menu or would they be better in "General 
> > > setup/Kernel Performance Events and Counters" ?
> > 
> > What's your preference? There's a vali argument for both: the 
> > uncore driver is part of the CPU, but it's also a perf driver.
> 
> My initial thought was having it in the perf menu because that's 
> relatively easy to find and not big and cluttered (yet) like the 
> arch menu is.
> 
> I actually had to double check I had the right patches applied 
> because I couldn't find it the first time.

Okay, agreed, lets do it that way. It's also the better choice UI 
wise: arguably people will associate this driver with 'perf', not with 
CPU features.

Thanks,

	Ingo

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Vince Weaver]

On Tue, 18 Mar 2014, Yan, Zheng 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

so just curious, after all this time why are we suddenly making 
modularizable perf pmu drivers?

Will other pmus be modularized, or is the uncore a special case?

I'm not necessarily against the idea, it's just it's tricky enough as it 
is trying to help people with uncore pmu issues and now I will have to add 
"make sure the relevant kernel module is loaded" to the list of things 
that could be going wrong.

Vince

Re: [PATCH 4/4] perf/x86/uncore: modularize Intel uncore driver

[Andi Kleen]

> 
> I'm not necessarily against the idea, it's just it's tricky enough as it 
> is trying to help people with uncore pmu issues and now I will have to add 
> "make sure the relevant kernel module is loaded" to the list of things 
> that could be going wrong.

In theory the module could be always auto loaded (by adding a
exported pci_device_id to the driver). However for 99% of users it would
be wasted memory, as they never monitor anything.

One way to handle it would be just to add a request_module with
appropiate aliases to perf.

-Andi

[tip:perf/core] perf: Allow building PMU drivers as modules

[tip-bot for Yan, Zheng]

Commit-ID:  c464c76eec4be587604ca082e8cded7e6b89f3bf
Gitweb:     http://git.kernel.org/tip/c464c76eec4be587604ca082e8cded7e6b89f3bf
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Tue, 18 Mar 2014 16:56:41 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 18 Apr 2014 12:54:45 +0200

perf: Allow building PMU drivers as modules

This patch adds support for building PMU driver as module. It exports
the functions perf_pmu_{register,unregister}() and adds reference tracking
for the PMU driver module.

When the PMU driver is built as a module, each active event of the PMU
holds a reference to the driver module.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1395133004-23205-1-git-send-email-zheng.z.yan@intel.com
Cc: eranian@google.com
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 3356abc..af6dcf1 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -172,6 +172,7 @@ struct perf_event;
 struct pmu {
 	struct list_head		entry;
 
+	struct module			*module;
 	struct device			*dev;
 	const struct attribute_group	**attr_groups;
 	const char			*name;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f83a71a..5129b12 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -39,6 +39,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/mm_types.h>
 #include <linux/cgroup.h>
+#include <linux/module.h>
 
 #include "internal.h"
 
@@ -3229,6 +3230,9 @@ static void __free_event(struct perf_event *event)
 	if (event->ctx)
 		put_ctx(event->ctx);
 
+	if (event->pmu)
+		module_put(event->pmu->module);
+
 	call_rcu(&event->rcu_head, free_event_rcu);
 }
 static void free_event(struct perf_event *event)
@@ -6551,6 +6555,7 @@ free_pdc:
 	free_percpu(pmu->pmu_disable_count);
 	goto unlock;
 }
+EXPORT_SYMBOL_GPL(perf_pmu_register);
 
 void perf_pmu_unregister(struct pmu *pmu)
 {
@@ -6572,6 +6577,7 @@ void perf_pmu_unregister(struct pmu *pmu)
 	put_device(pmu->dev);
 	free_pmu_context(pmu);
 }
+EXPORT_SYMBOL_GPL(perf_pmu_unregister);
 
 struct pmu *perf_init_event(struct perf_event *event)
 {
@@ -6585,6 +6591,10 @@ struct pmu *perf_init_event(struct perf_event *event)
 	pmu = idr_find(&pmu_idr, event->attr.type);
 	rcu_read_unlock();
 	if (pmu) {
+		if (!try_module_get(pmu->module)) {
+			pmu = ERR_PTR(-ENODEV);
+			goto unlock;
+		}
 		event->pmu = pmu;
 		ret = pmu->event_init(event);
 		if (ret)
@@ -6593,6 +6603,10 @@ struct pmu *perf_init_event(struct perf_event *event)
 	}
 
 	list_for_each_entry_rcu(pmu, &pmus, entry) {
+		if (!try_module_get(pmu->module)) {
+			pmu = ERR_PTR(-ENODEV);
+			goto unlock;
+		}
 		event->pmu = pmu;
 		ret = pmu->event_init(event);
 		if (!ret)
@@ -6771,6 +6785,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 err_pmu:
 	if (event->destroy)
 		event->destroy(event);
+	module_put(pmu->module);
 err_ns:
 	if (event->ns)
 		put_pid_ns(event->ns);