Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke

From: Yipeng Zou <zouyipeng@huawei.com>
To: "Gowans, James" <jgowans@amazon.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>
Cc: "maz@kernel.org" <maz@kernel.org>,
	"Raslan, KarimAllah" <karahmed@amazon.com>,
	"Woodhouse, David" <dwmw@amazon.co.uk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke
Date: Wed, 22 Mar 2023 18:38:18 +0800	[thread overview]
Message-ID: <ef8051f2-056f-9ea1-e5ba-51339fdb7877@huawei.com> (raw)
In-Reply-To: <fd5588e5b0b1b5d21b1c0df7290844907ebcf367.camel@amazon.com>

在 2023/3/22 15:48, Gowans, James 写道:
> On Wed, 2023-03-22 at 14:26 +0800, Yipeng Zou wrote:
>>> 在 2023/3/17 19:49, Gowans, James 写道:
>>> What are your thoughts on this approach compared to your proposal?
>> Hi,
>>
>> I also agree with you, enhance the existing generic handlers is a good
>> way to go.
>>
>> Too many generic handlers really confuse developers.
> Thomas, would you be open to taking the patch to tweak the handle_fasteoi_irq
> handler? Or is there a different solution to this problem which you prefer?

Our workaround is generally similar, but the implementation details are 
somewhat different.

Maybe let us look for some comments from maintainer and other people.

>> About CONFIG_GENERIC_PENDING_IRQ is actually some attempts we made
>> before under the suggestion of Thomas.
>>
>> This patch is valid for our problem. However, the current config is only
>> supported on x86, and some code modifications are required on arm.
> Thanks for the patch! I have been trying out CONFIG_GENERIC_PENDING_IRQ too, but
> couldn't get it to work; it seems the IRQ never actually moved. I see from your
> patch that we would need to tweak the callbacks and explicitly do the affinity
> move in the EOI handler of the chip; the generic code won't do it for us.
>
>> This has led to some changes in the original behavior of modifying
>> interrupting affinity, from the next interrupt taking effect to the next
>> to the next interrupt taking effect.
> So this means that even if it's safe to change the affinity right now, the
> change will actually be delayed until the *next* interrupt? Specifically because
> interrupt doesn't have the IRQD_MOVE_PCNTXT state flag isn't set hence
> irq_set_affinity_locked won't call irq_try_set_affinity?

Yes, modify of the interrupt affinity will be delayed until the *next* 
interrupt eoi handler(in hard_irq context).

This is the difference from x86, which do irq_move_irq in ack handler, 
and then transfer the current interrupt to the new CPU without other affect.

> JG

-- 
Regards,
Yipeng Zou