Re: [PATCH V3 2/2] arm64: perf: add support for percpu pmu interrupt

[Marc Zyngier]

On 2013-11-18 14:18, Vinayak Kale wrote:
> On Mon, Nov 18, 2013 at 7:16 PM, Marc Zyngier <marc.zyngier@xxxxxxx> 
> wrote:
> > Vinayak,
> >
> >
> > On 2013-11-18 13:22, Vinayak Kale wrote:
> > > Add support for irq registration when pmu interrupt is percpu.
> > >
> > > Signed-off-by: Vinayak Kale <vkale@xxxxxxx>
> > > Signed-off-by: Tuan Phan <tphan@xxxxxxx>
> > > ---
> > >  arch/arm64/kernel/perf_event.c |  102
> > > +++++++++++++++++++++++++++++-----------
> > >  1 file changed, 74 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/perf_event.c
> > > b/arch/arm64/kernel/perf_event.c
> > > index cea1594..23475f6 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,22 +364,51 @@ validate_group(struct perf_event *event)
> > >  }
> > >
> > >  static void
> > > +armpmu_disable_percpu_irq(void *data)
> > > +{
> > > +       struct arm_pmu *armpmu = data;
> > > +       struct platform_device *pmu_device = armpmu->plat_device;
> > > +       int irq = platform_get_irq(pmu_device, 0);
> > > +
> > > +       cpumask_test_and_clear_cpu(smp_processor_id(),
> > > &armpmu->active_irqs);
> > > +       disable_percpu_irq(irq);
> > > +}
> > > +
> > > +static void
> > >  armpmu_release_hardware(struct arm_pmu *armpmu)
> > >  {
> > >         int i, irq, irqs;
> > >         struct platform_device *pmu_device = armpmu->plat_device;
> > >
> > > -       irqs = min(pmu_device->num_resources, num_possible_cpus());
> > > +       irq = platform_get_irq(pmu_device, 0);
> > >
> > > -       for (i = 0; i < irqs; ++i) {
> > > -               if (!cpumask_test_and_clear_cpu(i, 
> > > &armpmu->active_irqs))
> > > -                       continue;
> > > -               irq = platform_get_irq(pmu_device, i);
> > > -               if (irq >= 0)
> > > -                       free_irq(irq, armpmu);
> > > +       if (irq_to_desc(irq) && irq_is_percpu(irq)) {
> >
> >
> > Why do you need to check the irq_desc here? It really looks like a 
> > misuse of
> > the API.
> I don't think it's being misused. In case of invalid irq number, the
> API would return null.

And feeding an error code to irq_to_desc() doesn't disturb you?
Do you call that a normal use of the API? Humfff....

        M.
--
Fast, cheap, reliable. Pick two.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/