The backport of upstream patch 25da4618af240fbec61 ("xen/events: don't unmask an event channel when an eoi is pending") introduced a regression for stable kernels 5.10 and older: setting IRQ affinity for IRQs related to interdomain events would no longer work, as moving the IRQ to its new cpu was not included in the irq_ack callback for those events.
Fix that by adding the needed call.
Note that kernels 5.11 and later don't need the explicit moving of the IRQ to the target cpu in the irq_ack callback, due to a rework of the affinity setting in kernel 5.11.
Signed-off-by: Juergen Gross jgross@suse.com --- This patch should be applied to all stable kernel branches up to (including) linux-5.10.y, where upstream patch 25da4618af240fbec61 has been added.
Signed-off-by: Juergen Gross jgross@suse.com --- drivers/xen/events/events_base.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 7bd03f6e0422..ee5269331406 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -1809,7 +1809,7 @@ static void lateeoi_ack_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EOI_PENDING); - event_handler_exit(info); + ack_dynirq(data); } }
@@ -1820,7 +1820,7 @@ static void lateeoi_mask_ack_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EXPLICIT); - event_handler_exit(info); + ack_dynirq(data); } }
On 12.04.2021 08:28, Juergen Gross wrote:
The backport of upstream patch 25da4618af240fbec61 ("xen/events: don't unmask an event channel when an eoi is pending") introduced a regression for stable kernels 5.10 and older: setting IRQ affinity for IRQs related to interdomain events would no longer work, as moving the IRQ to its new cpu was not included in the irq_ack callback for those events.
Fix that by adding the needed call.
Note that kernels 5.11 and later don't need the explicit moving of the IRQ to the target cpu in the irq_ack callback, due to a rework of the affinity setting in kernel 5.11.
Signed-off-by: Juergen Gross jgross@suse.com
This patch should be applied to all stable kernel branches up to (including) linux-5.10.y, where upstream patch 25da4618af240fbec61 has been added.
Signed-off-by: Juergen Gross jgross@suse.com
This looks functionally correct to me, so: Reviewed-by: Jan Beulich jbeulich@suse.com
But I have remarks / questions:
--- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -1809,7 +1809,7 @@ static void lateeoi_ack_dynirq(struct irq_data *data) if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EOI_PENDING);
event_handler_exit(info);
ack_dynirq(data);} @@ -1820,7 +1820,7 @@ static void lateeoi_mask_ack_dynirq(struct irq_data *data) if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EXPLICIT);
event_handler_exit(info);
ack_dynirq(data);}
Can EVT_MASK_REASON_EOI_{PENDING,EXPLICIT} be cleared in a way racing event_handler_exit() and (if it was called directly from here) irq_move_masked_irq()? If not, the extra do_mask() / do_unmask() pair (granted living on an "unlikely" path) could be avoided.
Even leaving aside the extra overhead in ack_dynirq()'s unlikely code path, there's now some extra (redundant) processing. I guess this is assumed to be within noise?
Possibly related, but first of all seeing the redundancy between eoi_pirq() and ack_dynirq(): Wouldn't it make sense to break out the common part into a helper? (Really the former could simply call the latter as it seems.)
Jan
On 12.04.21 11:32, Jan Beulich wrote:
On 12.04.2021 08:28, Juergen Gross wrote:
The backport of upstream patch 25da4618af240fbec61 ("xen/events: don't unmask an event channel when an eoi is pending") introduced a regression for stable kernels 5.10 and older: setting IRQ affinity for IRQs related to interdomain events would no longer work, as moving the IRQ to its new cpu was not included in the irq_ack callback for those events.
Fix that by adding the needed call.
Note that kernels 5.11 and later don't need the explicit moving of the IRQ to the target cpu in the irq_ack callback, due to a rework of the affinity setting in kernel 5.11.
Signed-off-by: Juergen Gross jgross@suse.com
This patch should be applied to all stable kernel branches up to (including) linux-5.10.y, where upstream patch 25da4618af240fbec61 has been added.
Signed-off-by: Juergen Gross jgross@suse.com
This looks functionally correct to me, so: Reviewed-by: Jan Beulich jbeulich@suse.com
But I have remarks / questions:
--- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -1809,7 +1809,7 @@ static void lateeoi_ack_dynirq(struct irq_data *data) if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EOI_PENDING);
event_handler_exit(info);
ack_dynirq(data);@@ -1820,7 +1820,7 @@ static void lateeoi_mask_ack_dynirq(struct irq_data *data) if (VALID_EVTCHN(evtchn)) { do_mask(info, EVT_MASK_REASON_EXPLICIT);
event_handler_exit(info);
ack_dynirq(data);Can EVT_MASK_REASON_EOI_{PENDING,EXPLICIT} be cleared in a way racing event_handler_exit() and (if it was called directly from here) irq_move_masked_irq()? If not, the extra do_mask() / do_unmask() pair (granted living on an "unlikely" path) could be avoided.
No, they can't race. And yes, the path is really unlikely, so I didn't want to optimize this rare case.
Even leaving aside the extra overhead in ack_dynirq()'s unlikely code path, there's now some extra (redundant) processing. I guess this is assumed to be within noise?
Yes. All required data should be in the caches already, and the extra processing is only a few instructions.
Possibly related, but first of all seeing the redundancy between eoi_pirq() and ack_dynirq(): Wouldn't it make sense to break out the common part into a helper? (Really the former could simply call the latter as it seems.)
In theory, yes. OTOH this no longer applies to upstream, so i dind't bother doing that for stable.
Juergen
On 12.04.2021 11:39, Juergen Gross wrote:
On 12.04.21 11:32, Jan Beulich wrote:
Possibly related, but first of all seeing the redundancy between eoi_pirq() and ack_dynirq(): Wouldn't it make sense to break out the common part into a helper? (Really the former could simply call the latter as it seems.)
In theory, yes. OTOH this no longer applies to upstream, so i dind't bother doing that for stable.
Oh, I guess I should have check the tip of the tree first...
Jan
On Mon, Apr 12, 2021 at 08:28:45AM +0200, Juergen Gross wrote:
The backport of upstream patch 25da4618af240fbec61 ("xen/events: don't unmask an event channel when an eoi is pending") introduced a regression for stable kernels 5.10 and older: setting IRQ affinity for IRQs related to interdomain events would no longer work, as moving the IRQ to its new cpu was not included in the irq_ack callback for those events.
Fix that by adding the needed call.
Note that kernels 5.11 and later don't need the explicit moving of the IRQ to the target cpu in the irq_ack callback, due to a rework of the affinity setting in kernel 5.11.
Signed-off-by: Juergen Gross jgross@suse.com
This patch should be applied to all stable kernel branches up to (including) linux-5.10.y, where upstream patch 25da4618af240fbec61 has been added.
Now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Jan Beulich -
Juergen Gross