linux-kernel - Re: [PATCH V7 2/2] arm64: perf: add support for percpu pmu interrupt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Dec 2013 13:00:26 +0530
From:	Vinayak Kale <vkale@....com>
To:	Will Deacon <will.deacon@....com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"patches@....com" <patches@....com>,
	"jcm@...hat.com" <jcm@...hat.com>,
	"sboyd@...eaurora.org" <sboyd@...eaurora.org>,
	Marc Zyngier <Marc.Zyngier@....com>, Tuan Phan <tphan@....com>
Subject: Re: [PATCH V7 2/2] arm64: perf: add support for percpu pmu interrupt

Hi Will,


On Mon, Dec 9, 2013 at 10:20 PM, Will Deacon <will.deacon@....com> wrote:
> Hi Vinayak,
>
> On Wed, Dec 04, 2013 at 10:09:51AM +0000, Vinayak Kale wrote:
>> Add support for irq registration when pmu interrupt is percpu.
>
> Getting closer...
>
>> Signed-off-by: Vinayak Kale <vkale@....com>
>> Signed-off-by: Tuan Phan <tphan@....com>
>> ---
>>  arch/arm64/kernel/perf_event.c |  108 +++++++++++++++++++++++++++++-----------
>>  1 file changed, 78 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index cea1594..d8e6667 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -22,6 +22,7 @@
>>
>>  #include <linux/bitmap.h>
>>  #include <linux/interrupt.h>
>> +#include <linux/irq.h>
>>  #include <linux/kernel.h>
>>  #include <linux/export.h>
>>  #include <linux/perf_event.h>
>> @@ -363,26 +364,52 @@ validate_group(struct perf_event *event)
>>  }
>>
>>  static void
>> +armpmu_disable_percpu_irq(void *data)
>> +{
>> +     disable_percpu_irq((long)data);
>> +}
>
> Given that we wait for the CPUs to finish enabling/disabling the IRQ, I
> actually meant pass the pointer to the IRQ, which removes the horrible
> casts in the caller.
>
>> +     if (irq_is_percpu(irq)) {
>> +             cpumask_clear(&armpmu->active_irqs);
>
> Thanks for moving the mask manipulation out. It now makes it obvious that we
> don't care about the mask at all for PPIs, so that can be removed (the code
> you have is racy against hotplug anyway).
>
> I took the liberty of writing a fixup for you (see below). Can you test it
> on your platform please?

Below fixup works fine on APM platform.
Do you want me to send this fixup as part of next revision of the
patch or will you apply it yourself? (For later case, you have my ack)

>
> Cheers,
>
> Will
>
> --->8
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 503c1eeedc1c..5b1cd792274a 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -366,7 +366,8 @@ validate_group(struct perf_event *event)
>  static void
>  armpmu_disable_percpu_irq(void *data)
>  {
> -       disable_percpu_irq((long)data);
> +       unsigned int irq = *(unsigned int *)data;
> +       disable_percpu_irq(irq);
>  }
>
>  static void
> @@ -385,8 +386,7 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>                 return;
>
>         if (irq_is_percpu(irq)) {
> -               cpumask_clear(&armpmu->active_irqs);
> -               on_each_cpu(armpmu_disable_percpu_irq, (void *)(long)irq, 1);
> +               on_each_cpu(armpmu_disable_percpu_irq, &irq, 1);
>                 free_percpu_irq(irq, &cpu_hw_events);
>         } else {
>                 for (i = 0; i < irqs; ++i) {
> @@ -402,7 +402,8 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
>  static void
>  armpmu_enable_percpu_irq(void *data)
>  {
> -       enable_percpu_irq((long)data, IRQ_TYPE_NONE);
> +       unsigned int irq = *(unsigned int *)data;
> +       enable_percpu_irq(irq, IRQ_TYPE_NONE);
>  }
>
>  static int
> @@ -440,8 +441,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>                         return err;
>                 }
>
> -               on_each_cpu(armpmu_enable_percpu_irq, (void *)(long)irq, 1);
> -               cpumask_setall(&armpmu->active_irqs);
> +               on_each_cpu(armpmu_enable_percpu_irq, &irq, 1);
>         } else {
>                 for (i = 0; i < irqs; ++i) {
>                         err = 0;

Acked-by: Vinayak Kale <vkale@....com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/