From: sudeep.holla@arm.com (Sudeep Holla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: use cpu_online_mask when using forced irq_set_affinity
Date: Tue, 26 Aug 2014 16:19:12 +0100 [thread overview]
Message-ID: <53FCA570.1090809@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1408251136380.3323@nanos>
Hi Thomas,
Thanks for your feedback.
On 25/08/14 10:55, Thomas Gleixner wrote:
> On Thu, 17 Jul 2014, Sudeep Holla wrote:
>>>> Any suggestions on this ? Since commit 01f8fa4f01d8 and ffde1de64012 are
>>>> now in
>>>> stable releases, CPU0 hotplug is broken there now.
>>>
>>> Maybe we should ask Thomas, as he's (a) the maintainer of the irqchip
>>> stuff, and (b) the author of the patch causing the breakage.
>>>
>>> From what I can see looking at the x86 code, the work-around in
>>> ffde1de64012 is wrong.
>>>
>>
>> Can provide your thoughts on how to solve this issue ?
>
> ffde1de64012 is not about offlining a cpu, it's about onlining where
> we need to make sure that we assign the affinity to a not yet online
> marked cpu.
>
>> Is it expected from all the irqchip implementation to use force flag in
>> irq_set_affinity to ignore cpu_online_mask similar to GIC ?
>
> No, it's only relevant for the cases where we need to route irqs to
> not yet online cpus.
>
Ok. IIUC Russell's main concern was if irqchip implementation uses force
flag differently, then we can't change the core code to false. Also
x86 core code also uses forced irq_set_affinity in arch/x86/kernel/irq.c
Russell, any comments on this or are you fine with changing to false.
Regards,
Sudeep
> Now the wreckage of offlining was definitely not intended and I wonder
> why set_affinity() is called there with force = true. This was
> introduced in commit 1dbfa187dad. I acked it back then, but I have no
> idea why, because the force argument did not have any effect at that
> time.
>
> Changing it to false should solve the issue.
>
> Thanks,
>
> tglx
>
> diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
> index 2c4257604513..5c4d38e32a51 100644
> --- a/arch/arm/kernel/irq.c
> +++ b/arch/arm/kernel/irq.c
> @@ -175,7 +175,7 @@ static bool migrate_one_irq(struct irq_desc *desc)
> c = irq_data_get_irq_chip(d);
> if (!c->irq_set_affinity)
> pr_debug("IRQ%u: unable to set affinity\n", d->irq);
> - else if (c->irq_set_affinity(d, affinity, true) == IRQ_SET_MASK_OK && ret)
> + else if (c->irq_set_affinity(d, affinity, false) == IRQ_SET_MASK_OK && ret)
> cpumask_copy(d->affinity, affinity);
>
> return ret;
>
next prev parent reply other threads:[~2014-08-26 15:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-09 16:40 [PATCH] arm: use cpu_online_mask when using forced irq_set_affinity Sudeep Holla
2014-05-23 11:26 ` Sudeep Holla
2014-05-23 12:10 ` Russell King - ARM Linux
2014-05-23 12:51 ` Sudeep Holla
2014-06-20 13:04 ` Sudeep Holla
2014-06-20 15:16 ` Russell King - ARM Linux
2014-07-17 9:58 ` Sudeep Holla
2014-08-25 9:55 ` Thomas Gleixner
2014-08-26 15:19 ` Sudeep Holla [this message]
2014-08-28 9:32 ` Thomas Gleixner
2014-08-28 10:12 ` Sudeep Holla
2014-08-28 10:16 ` Russell King - ARM Linux
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=53FCA570.1090809@arm.com \
--to=sudeep.holla@arm.com \
--cc=linux-arm-kernel@lists.infradead.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).