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@xxxxxxxx>
> ---
> 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@xxxxxxxx>
This looks functionally correct to me, so:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
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
