linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: John Keeping <john@metanate.com>
Cc: Ben Herrenschmidt <benh@amazon.com>,
	Marc Zyngier <maz@kernel.org>,
	x86@kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Ali Saidi <alisaidi@amazon.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V2] genirq/affinity: Handle affinity setting on inactive interrupts correctly
Date: Fri, 24 Jul 2020 22:03:50 +0200	[thread overview]
Message-ID: <87h7twu1cp.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20200724182422.27ddced6.john@metanate.com>

John,

John Keeping <john@metanate.com> writes:
> On Fri, 17 Jul 2020 18:00:02 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
> It seems that this patch breaks perf events on RK3288 because the PMU
> interrupts that should be per-cpu are now all on CPU0 so no events are
> collected from CPUs 1-3 and those interrupts are killed as spurious
> after a few seconds.
>
> I'm seeing this on 4.19.134 and 5.4.53 but as far as I can tell the
> relevant code hasn't changed through to next-20200723.  Reverting the
> backport of this change fixes the problem.

Bah.

> It looks like what happens is that because the interrupts are not
> per-CPU in the hardware, armpmu_request_irq() calls irq_force_affinity()
> while the interrupt is deactivated and then request_irq() with
> IRQF_PERCPU | IRQF_NOBALANCING.
>
> Now when irq_startup() runs with IRQ_STARTUP_NORMAL, it calls
> irq_setup_affinity() which returns early because IRQF_PERCPU and
> IRQF_NOBALANCING are set, leaving the interrupt on its original CPU.

Right. My brain tricked me to believe that we made activation mandatory,
but that's not.

I have some ideas for a trivial generic way to solve this without
undoing the commit in question and without going through all the irq
chip drivers. So far everything I came up with is butt ugly. Maybe Marc
has some brilliant idea.

Sorry for the wreckage and thanks for the excellent problem
description. I'll come back to you in the next days.

Thanks,

        tglx



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-07-24 20:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-17 13:14 [PATCH] genirq/affinity: Handle affinity setting on inactive interrupts correctly Thomas Gleixner
2020-07-17 15:25 ` kernel test robot
2020-07-17 16:00 ` [PATCH V2] " Thomas Gleixner
2020-07-24 17:24   ` John Keeping
2020-07-24 20:03     ` Thomas Gleixner [this message]
2020-07-24 20:44       ` Thomas Gleixner
2020-07-25 12:30         ` Marc Zyngier
2020-07-25 12:08       ` Marc Zyngier
2020-07-27 13:35         ` Thomas Gleixner
2020-07-17 18:04 ` [PATCH] " kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h7twu1cp.fsf@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=alisaidi@amazon.com \
    --cc=benh@amazon.com \
    --cc=john@metanate.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).