On Sun, Apr 26, 2015 at 8:43 PM, Liang, Kan <kan.li...@intel.com> wrote:
>
>>
>> > This leads me to believe that this patch:
>> >
>> > commit c05199e5a57a579fea1e8fa65e2b511ceb524ffc
>> > Author: Kan Liang <kan.li...@intel.com>
>> > Date: Tue Jan 20 04:54:25 2015 +0000
>> >
>> > perf/x86/intel/uncore: Move uncore_box_init() out of driver
>> initialization
>> >
>> > If I revert it, I bet things will work again.
>>
>> Yes the initialization needs to be moved out of the IPI context.
>>
>
> Maybe we can move them to event init, which is not in IPI context.
>
> What do you think of this patch?
>
> ---
>
> From 8a61c48144921e9d1c841656829c3bae9bfb4408 Mon Sep 17 00:00:00 2001
> From: Kan Liang <kan.li...@intel.com>
> Date: Sun, 26 Apr 2015 16:24:59 -0400
> Subject: [PATCH 1/1] perf/x86/intel/uncore: move uncore_box_init to uncore
> event init
>
> commit c05199e5a57a("perf/x86/intel/uncore: Move uncore_box_init() out
> of driver initialization") moves uncore_box_init into uncore_enable_box
> to prevent potential boot failures. However, uncore_enable_box is not
> called on some client platforms (SNB/IVB/HSW) for counting IMC event.
> When it is not called, the box is not initialized, which hard locks the
> system.
>
> Additionally, uncore_enable_box along with the initialization code in it
> is always called in uncore event start functions, which are in IPI
> context. But the initizlization code should not be in IPI context. This
> is because, for example, the IMC box initialization codes for client
> platforms include ioremap, which is not allowed to be called in IPI
> context.
>
> This patch moves uncore_box_init out of IPI context, to uncore event
> init. The box is initialized only when it has not yet been initialized.
>
> Signed-off-by: Kan Liang <kan.li...@intel.com>
Ok this works for me now. Thanks.
Tested-by: Stephane Eranian <eran...@google.com>
> ---
> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 4 ++++
> arch/x86/kernel/cpu/perf_event_intel_uncore.h | 2 --
> arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c | 3 +++
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index c635b8b..cbc1a93 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -623,6 +623,10 @@ static int uncore_pmu_event_init(struct perf_event
> *event)
> box = uncore_pmu_to_box(pmu, event->cpu);
> if (!box || box->cpu < 0)
> return -EINVAL;
> +
> + /* Init box if it's not initialized yet */
> + uncore_box_init(box);
> +
> event->cpu = box->cpu;
>
> event->hw.idx = -1;
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> index 6c8c1e7..1fb2905 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> @@ -273,8 +273,6 @@ static inline void uncore_disable_box(struct
> intel_uncore_box *box)
>
> static inline void uncore_enable_box(struct intel_uncore_box *box)
> {
> - uncore_box_init(box);
> -
> if (box->pmu->type->ops->enable_box)
> box->pmu->type->ops->enable_box(box);
> }
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> index 4562e9e..ead70a6 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> @@ -279,6 +279,9 @@ static int snb_uncore_imc_event_init(struct perf_event
> *event)
> if (!box || box->cpu < 0)
> return -EINVAL;
>
> + /* Init box if it's not initialized yet */
> + uncore_box_init(box);
> +
> event->cpu = box->cpu;
>
> event->hw.idx = -1;
>
> Thanks,
> Kan
>
>> -Andi
>>
>>
>> --
>> a...@linux.intel.com -- Speaking for myself only
--
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/
