From: Stefan Agner <stefan@agner.ch>
To: Lucas Stach <l.stach@pengutronix.de>,
will.deacon@arm.com, mark.rutland@arm.com
Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
max.krummenacher@toradex.com, marcel.ziswiler@toradex.com,
linux-kernel@vger.kernel.org, linux-imx@nxp.com,
kernel@pengutronix.de, fabio.estevam@nxp.com, dev@pschenker.ch,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] ARM: dts: imx6q: add pmu interrupt-affinity
Date: Mon, 21 Jan 2019 10:39:58 +0100 [thread overview]
Message-ID: <5f35af2bc4b5c97d69ff9ab1c71da1f7@agner.ch> (raw)
In-Reply-To: <6e4fe68ec80926b4292ce786c6737182@agner.ch>
[adding Will/Mark]
On 18.01.2019 16:41, Stefan Agner wrote:
> On 18.01.2019 15:12, Lucas Stach wrote:
>> Am Freitag, den 18.01.2019, 14:59 +0100 schrieb Stefan Agner:
>>> Explicitly specify interrupt affinity to avoid HW perfevents
>>> need to guess. This avoids the following error upon boot:
>>> hw perfevents: no interrupt-affinity property for /pmu, guessing.
>>>
>> But then it isn't correct either AFAICS. On i.MX6 all the PMU IRQs are
>> ORed together into a single SPI, instead of each core dealing with its
>> own PPI. So pretending that there are more IRQs with affinity to each
>> core isn't the right thing to do, no?
>>
>
> Oh I see, we only have a single interrupt in the i.MX 6 case.
>
> I agree, this patches are wrong.
>
> Hm, but why does hw perf then think it needs to guess? Doesn't seem hard
> to guess right if there is only one choice...
>
> We probably need to do something like this?
>
> --- a/drivers/perf/arm_pmu_platform.c
> +++ b/drivers/perf/arm_pmu_platform.c
> @@ -122,7 +122,8 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)
> return pmu_parse_percpu_irq(pmu, irq);
> }
>
> - if (nr_cpu_ids != 1 && !pmu_has_irq_affinity(pdev->dev.of_node))
> {
> + if ((nr_cpu_ids != 1 || num_irqs != 1) &&
> + !pmu_has_irq_affinity(pdev->dev.of_node)) {
> pr_warn("no interrupt-affinity property for %pOF,
> guessing.\n",
> pdev->dev.of_node);
> }
Yeah, I see, it was on Friday... if anything, it should be:
- if (nr_cpu_ids != 1 && !pmu_has_irq_affinity(pdev->dev.of_node))
{
+ if (nr_cpu_ids != 1 && num_irqs != 1 &&
+ !pmu_has_irq_affinity(pdev->dev.of_node)) {
However, I realized that arm_pmu_platform actually currently only
assigns the one IRQ to the first CPU. This leads to perf not working if
a process is scheduled on another CPU then the first.
I could reproduce this. PID 763 is a long running task.
root@colibri-imx6-05051054:~# taskset -p 0x1 763
pid 763's current affinity mask: 3
pid 763's new affinity mask: 1
root@colibri-imx6-05051054:~# perf stat -e cpu-cycles -p 763
^C
Performance counter stats for process id '763':
7581021 cpu-cycles
1.222248490 seconds time elapsed
root@colibri-imx6-05051054:~# taskset -p 0x2 763
pid 763's current affinity mask: 1
pid 763's new affinity mask: 2
root@colibri-imx6-05051054:~# perf stat -e cpu-cycles -p 763
^C
Performance counter stats for process id '763':
<not counted> cpu-cycles
(0.00%)
1.050253575 seconds time elapsed
I guess we need to tell the PMU driver that a single IRQ should be used
for all CPUs?
--
Stefan
>
>
>> Regards,
>> Lucas
>>
>>> Specifying all four CPUs shows no aversive effects on i.MX 6Dual
>>> SoCs.
>>>
>>> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>>> ---
>>> arch/arm/boot/dts/imx6q.dtsi | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
>>> index 8381d24eff7d..d2c1977c8b16 100644
>>> --- a/arch/arm/boot/dts/imx6q.dtsi
>>> +++ b/arch/arm/boot/dts/imx6q.dtsi
>>> @@ -537,6 +537,13 @@
>>> > <0x28 0x0000000c>; /* DCIC2_MUX_CTL */
>>> };
>>>
>>> +&pmu {
>>> > > + interrupt-affinity = <&{/cpus/cpu@0}>,
>>> > > + <&{/cpus/cpu@1}>,
>>> > > + <&{/cpus/cpu@2}>,
>>> > > + <&{/cpus/cpu@3}>;
>>> +};
>>> +
>>> &vpu {
>>> > compatible = "fsl,imx6q-vpu", "cnm,coda960";
>>> };
prev parent reply other threads:[~2019-01-21 9:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-18 13:59 [PATCH 1/2] ARM: dts: imx6q: add pmu interrupt-affinity Stefan Agner
2019-01-18 13:59 ` [PATCH 2/2] ARM: dts: imx6dl: " Stefan Agner
2019-01-18 14:12 ` [PATCH 1/2] ARM: dts: imx6q: " Lucas Stach
2019-01-18 15:41 ` Stefan Agner
2019-01-21 9:39 ` Stefan Agner [this message]
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=5f35af2bc4b5c97d69ff9ab1c71da1f7@agner.ch \
--to=stefan@agner.ch \
--cc=dev@pschenker.ch \
--cc=fabio.estevam@nxp.com \
--cc=kernel@pengutronix.de \
--cc=l.stach@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel.ziswiler@toradex.com \
--cc=mark.rutland@arm.com \
--cc=max.krummenacher@toradex.com \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=will.deacon@arm.com \
/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).