All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 2/9] x86/IRQ: deal with move cleanup count state in fixup_irqs()
Date: Fri, 3 May 2019 17:21:42 +0200	[thread overview]
Message-ID: <20190503152142.icdzlax4arik3beg@Air-de-Roger> (raw)
In-Reply-To: <5CC6DEA80200007800229E9D@prv1-mh.provo.novell.com>

On Mon, Apr 29, 2019 at 05:23:20AM -0600, Jan Beulich wrote:
> The cleanup IPI may get sent immediately before a CPU gets removed from
> the online map. In such a case the IPI would get handled on the CPU
> being offlined no earlier than in the interrupts disabled window after
> fixup_irqs()' main loop. This is too late, however, because a possible
> affinity change may incur the need for vector assignment, which will
> fail when the IRQ's move cleanup count is still non-zero.
> 
> To fix this
> - record the set of CPUs the cleanup IPIs gets actually sent to alongside
>   setting their count,
> - adjust the count in fixup_irqs(), accounting for all CPUs that the
>   cleanup IPI was sent to, but that are no longer online,
> - bail early from the cleanup IPI handler when the CPU is no longer
>   online, to prevent double accounting.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Just as a note, this whole interrupt migration business seems
extremely complex, and I wonder if Xen does really need it, or what's
exactly it's performance gain compared to more simple solutions. I
understand this is just fixes, but IMO it's making the logic even more
complex.

Maybe it would be simpler to have the interrupts hard-bound to pCPUs
and instead have a soft-affinity on the guest vCPUs that are assigned
as the destination?

> ---
> TBD: The proper recording of the IPI destinations actually makes the
>      move_cleanup_count field redundant. Do we want to drop it, at the
>      price of a few more CPU-mask operations?

AFAICT this is not a hot path, so I would remove the
move_cleanup_count field and just weight the cpu bitmap when needed.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH 2/9] x86/IRQ: deal with move cleanup count state in fixup_irqs()
Date: Fri, 3 May 2019 17:21:42 +0200	[thread overview]
Message-ID: <20190503152142.icdzlax4arik3beg@Air-de-Roger> (raw)
Message-ID: <20190503152142.hhNtkEqqacuyQ-edo11bInm_tOoiwYqwX-NsDQW0oKo@z> (raw)
In-Reply-To: <5CC6DEA80200007800229E9D@prv1-mh.provo.novell.com>

On Mon, Apr 29, 2019 at 05:23:20AM -0600, Jan Beulich wrote:
> The cleanup IPI may get sent immediately before a CPU gets removed from
> the online map. In such a case the IPI would get handled on the CPU
> being offlined no earlier than in the interrupts disabled window after
> fixup_irqs()' main loop. This is too late, however, because a possible
> affinity change may incur the need for vector assignment, which will
> fail when the IRQ's move cleanup count is still non-zero.
> 
> To fix this
> - record the set of CPUs the cleanup IPIs gets actually sent to alongside
>   setting their count,
> - adjust the count in fixup_irqs(), accounting for all CPUs that the
>   cleanup IPI was sent to, but that are no longer online,
> - bail early from the cleanup IPI handler when the CPU is no longer
>   online, to prevent double accounting.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Just as a note, this whole interrupt migration business seems
extremely complex, and I wonder if Xen does really need it, or what's
exactly it's performance gain compared to more simple solutions. I
understand this is just fixes, but IMO it's making the logic even more
complex.

Maybe it would be simpler to have the interrupts hard-bound to pCPUs
and instead have a soft-affinity on the guest vCPUs that are assigned
as the destination?

> ---
> TBD: The proper recording of the IPI destinations actually makes the
>      move_cleanup_count field redundant. Do we want to drop it, at the
>      price of a few more CPU-mask operations?

AFAICT this is not a hot path, so I would remove the
move_cleanup_count field and just weight the cpu bitmap when needed.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2019-05-03 15:22 UTC|newest]

Thread overview: 196+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-29 11:16 [PATCH 0/9] x86: IRQ management adjustments Jan Beulich
2019-04-29 11:16 ` [Xen-devel] " Jan Beulich
2019-04-29 11:22 ` [PATCH RFC 1/9] x86/IRQ: deal with move-in-progress state in fixup_irqs() Jan Beulich
2019-04-29 11:22   ` [Xen-devel] " Jan Beulich
2019-04-29 12:55   ` Jan Beulich
2019-04-29 12:55     ` [Xen-devel] " Jan Beulich
2019-04-29 13:08     ` Jan Beulich
2019-04-29 13:08       ` [Xen-devel] " Jan Beulich
2019-04-29 11:23 ` [PATCH 2/9] x86/IRQ: deal with move cleanup count " Jan Beulich
2019-04-29 11:23   ` [Xen-devel] " Jan Beulich
2019-05-03 15:21   ` Roger Pau Monné [this message]
2019-05-03 15:21     ` Roger Pau Monné
2019-05-06  7:44     ` Jan Beulich
2019-05-06  7:44       ` [Xen-devel] " Jan Beulich
2019-05-07  7:28     ` Jan Beulich
2019-05-07  7:28       ` [Xen-devel] " Jan Beulich
2019-05-07  8:12       ` Roger Pau Monné
2019-05-07  8:12         ` [Xen-devel] " Roger Pau Monné
2019-05-07  9:28         ` Jan Beulich
2019-05-07  9:28           ` [Xen-devel] " Jan Beulich
2019-04-29 11:23 ` [PATCH 3/9] x86/IRQ: improve dump_irqs() Jan Beulich
2019-04-29 11:23   ` [Xen-devel] " Jan Beulich
2019-05-03 15:43   ` Roger Pau Monné
2019-05-03 15:43     ` [Xen-devel] " Roger Pau Monné
2019-05-06  8:06     ` Jan Beulich
2019-05-06  8:06       ` [Xen-devel] " Jan Beulich
2019-04-29 11:24 ` [PATCH 4/9] x86/IRQ: desc->affinity should strictly represent the requested value Jan Beulich
2019-04-29 11:24   ` [Xen-devel] " Jan Beulich
2019-05-03 16:21   ` Roger Pau Monné
2019-05-03 16:21     ` [Xen-devel] " Roger Pau Monné
2019-05-06  8:14     ` Jan Beulich
2019-05-06  8:14       ` [Xen-devel] " Jan Beulich
2019-04-29 11:25 ` [PATCH 5/9] x86/IRQ: fix locking around vector management Jan Beulich
2019-04-29 11:25   ` [Xen-devel] " Jan Beulich
2019-05-06 11:48   ` Roger Pau Monné
2019-05-06 11:48     ` [Xen-devel] " Roger Pau Monné
2019-05-06 13:06     ` Jan Beulich
2019-05-06 13:06       ` [Xen-devel] " Jan Beulich
2019-04-29 11:25 ` [PATCH 6/9] x86/IRQ: reduce unused space in struct arch_irq_desc Jan Beulich
2019-04-29 11:25   ` [Xen-devel] " Jan Beulich
2019-04-29 11:46   ` Andrew Cooper
2019-04-29 11:46     ` [Xen-devel] " Andrew Cooper
2019-04-29 11:26 ` [PATCH 7/9] x86/IRQ: drop redundant cpumask_empty() from move_masked_irq() Jan Beulich
2019-04-29 11:26   ` [Xen-devel] " Jan Beulich
2019-05-06 13:39   ` Roger Pau Monné
2019-05-06 13:39     ` [Xen-devel] " Roger Pau Monné
2019-04-29 11:26 ` [PATCH 8/9] x86/IRQ: make fixup_irqs() skip unconnected internally used interrupts Jan Beulich
2019-04-29 11:26   ` [Xen-devel] " Jan Beulich
2019-05-06 13:52   ` Roger Pau Monné
2019-05-06 13:52     ` [Xen-devel] " Roger Pau Monné
2019-05-06 14:25     ` Jan Beulich
2019-05-06 14:25       ` [Xen-devel] " Jan Beulich
2019-05-06 14:37       ` Roger Pau Monné
2019-05-06 14:37         ` [Xen-devel] " Roger Pau Monné
2019-04-29 11:27 ` [PATCH 9/9] x86/IO-APIC: drop an unused variable from setup_IO_APIC_irqs() Jan Beulich
2019-04-29 11:27   ` [Xen-devel] " Jan Beulich
2019-04-29 11:40   ` Andrew Cooper
2019-04-29 11:40     ` [Xen-devel] " Andrew Cooper
2019-04-29 15:40 ` [PATCH v1b 1/9] x86/IRQ: deal with move-in-progress state in fixup_irqs() Jan Beulich
2019-04-29 15:40   ` [Xen-devel] " Jan Beulich
2019-05-03  9:19   ` Roger Pau Monné
2019-05-03  9:19     ` [Xen-devel] " Roger Pau Monné
2019-05-03 14:10     ` Jan Beulich
2019-05-03 14:10       ` [Xen-devel] " Jan Beulich
2019-05-06  7:15       ` Jan Beulich
2019-05-06  7:15         ` [Xen-devel] " Jan Beulich
2019-05-06 14:28         ` Roger Pau Monné
2019-05-06 14:28           ` [Xen-devel] " Roger Pau Monné
2019-05-06 15:00           ` Jan Beulich
2019-05-06 15:00             ` [Xen-devel] " Jan Beulich
2019-05-08 12:59 ` [PATCH v2 00/12] x86: IRQ management adjustments Jan Beulich
2019-05-08 12:59   ` [Xen-devel] " Jan Beulich
2019-05-08 13:03   ` [PATCH v2 01/12] x86/IRQ: deal with move-in-progress state in fixup_irqs() Jan Beulich
2019-05-08 13:03     ` [Xen-devel] " Jan Beulich
2019-05-13  9:04     ` Roger Pau Monné
2019-05-13  9:04       ` [Xen-devel] " Roger Pau Monné
2019-05-13  9:09       ` Jan Beulich
2019-05-13  9:09         ` [Xen-devel] " Jan Beulich
2019-05-08 13:03   ` [PATCH v2 02/12] x86/IRQ: deal with move cleanup count " Jan Beulich
2019-05-08 13:03     ` [Xen-devel] " Jan Beulich
2019-05-08 13:07   ` [PATCH v2 03/12] x86/IRQ: avoid UB (or worse) in trace_irq_mask() Jan Beulich
2019-05-08 13:07     ` [Xen-devel] " Jan Beulich
2019-05-13  9:08     ` Roger Pau Monné
2019-05-13  9:08       ` [Xen-devel] " Roger Pau Monné
2019-05-13 10:42     ` George Dunlap
2019-05-13 10:42       ` [Xen-devel] " George Dunlap
2019-05-13 12:05       ` Jan Beulich
2019-05-13 12:05         ` [Xen-devel] " Jan Beulich
2019-05-08 13:08   ` [PATCH v2 04/12] x86/IRQ: improve dump_irqs() Jan Beulich
2019-05-08 13:08     ` [Xen-devel] " Jan Beulich
2019-05-08 13:09   ` [PATCH v2 05/12] x86/IRQ: desc->affinity should strictly represent the requested value Jan Beulich
2019-05-08 13:09     ` [Xen-devel] " Jan Beulich
2019-05-08 13:10   ` [PATCH v2 06/12] x86/IRQ: consolidate use of ->arch.cpu_mask Jan Beulich
2019-05-08 13:10     ` [Xen-devel] " Jan Beulich
2019-05-13 11:32     ` Roger Pau Monné
2019-05-13 11:32       ` [Xen-devel] " Roger Pau Monné
2019-05-13 15:21       ` Jan Beulich
2019-05-13 15:21         ` [Xen-devel] " Jan Beulich
2019-05-08 13:10   ` [PATCH v2 07/12] x86/IRQ: fix locking around vector management Jan Beulich
2019-05-08 13:10     ` [Xen-devel] " Jan Beulich
2019-05-08 13:16     ` Jan Beulich
2019-05-08 13:16       ` [Xen-devel] " Jan Beulich
2019-05-11  0:11       ` Tian, Kevin
2019-05-11  0:11         ` [Xen-devel] " Tian, Kevin
2019-05-13 13:48     ` Roger Pau Monné
2019-05-13 13:48       ` [Xen-devel] " Roger Pau Monné
2019-05-13 14:19       ` Jan Beulich
2019-05-13 14:19         ` [Xen-devel] " Jan Beulich
2019-05-13 14:45         ` Roger Pau Monné
2019-05-13 14:45           ` [Xen-devel] " Roger Pau Monné
2019-05-13 15:05           ` Jan Beulich
2019-05-13 15:05             ` [Xen-devel] " Jan Beulich
2019-05-08 13:11   ` [PATCH v2 08/12] x86/IRQs: correct/tighten vector check in _clear_irq_vector() Jan Beulich
2019-05-08 13:11     ` [Xen-devel] " Jan Beulich
2019-05-13 14:01     ` Roger Pau Monné
2019-05-13 14:01       ` [Xen-devel] " Roger Pau Monné
2019-05-08 13:12   ` [PATCH v2 09/12] x86/IRQ: make fixup_irqs() skip unconnected internally used interrupts Jan Beulich
2019-05-08 13:12     ` [Xen-devel] " Jan Beulich
2019-05-08 13:13   ` [PATCH v2 10/12] x86/IRQ: reduce unused space in struct arch_irq_desc Jan Beulich
2019-05-08 13:13     ` [Xen-devel] " Jan Beulich
2019-05-08 13:13   ` [PATCH v2 11/12] x86/IRQ: drop redundant cpumask_empty() from move_masked_irq() Jan Beulich
2019-05-08 13:13     ` [Xen-devel] " Jan Beulich
2019-05-08 13:14   ` [PATCH v2 12/12] x86/IRQ: simplify and rename pirq_acktype() Jan Beulich
2019-05-08 13:14     ` [Xen-devel] " Jan Beulich
2019-05-13 14:14     ` Roger Pau Monné
2019-05-13 14:14       ` [Xen-devel] " Roger Pau Monné
2019-05-17 10:39 ` [PATCH v3 00/15] x86: IRQ management adjustments Jan Beulich
2019-05-17 10:39   ` [Xen-devel] " Jan Beulich
2019-05-17 10:44   ` [PATCH v3 01/15] x86/IRQ: deal with move-in-progress state in fixup_irqs() Jan Beulich
2019-05-17 10:44     ` [Xen-devel] " Jan Beulich
2019-07-03 15:39     ` Andrew Cooper
2019-07-04  9:32       ` Jan Beulich
2019-05-17 10:45   ` [PATCH v3 02/15] x86/IRQ: deal with move cleanup count " Jan Beulich
2019-05-17 10:45     ` [Xen-devel] " Jan Beulich
2019-07-03 16:32     ` Andrew Cooper
2019-05-17 10:46   ` [PATCH v3 03/15] x86/IRQ: improve dump_irqs() Jan Beulich
2019-05-17 10:46     ` [Xen-devel] " Jan Beulich
2019-07-03 16:39     ` Andrew Cooper
2019-05-17 10:46   ` [PATCH v3 04/15] x86/IRQ: desc->affinity should strictly represent the requested value Jan Beulich
2019-05-17 10:46     ` [Xen-devel] " Jan Beulich
2019-07-03 17:58     ` Andrew Cooper
2019-07-04  9:37       ` Jan Beulich
2019-05-17 10:47   ` [PATCH v3 05/15] x86/IRQ: consolidate use of ->arch.cpu_mask Jan Beulich
2019-05-17 10:47     ` [Xen-devel] " Jan Beulich
2019-07-03 18:07     ` Andrew Cooper
2019-05-17 10:47   ` [PATCH v3 06/15] x86/IRQ: fix locking around vector management Jan Beulich
2019-05-17 10:47     ` [Xen-devel] " Jan Beulich
2019-07-03 18:23     ` Andrew Cooper
2019-07-04  9:54       ` Jan Beulich
2019-05-17 10:48   ` [PATCH v3 07/15] x86/IRQ: target online CPUs when binding guest IRQ Jan Beulich
2019-05-17 10:48     ` [Xen-devel] " Jan Beulich
2019-05-20 11:40     ` Roger Pau Monné
2019-05-20 11:40       ` [Xen-devel] " Roger Pau Monné
2019-05-20 15:17       ` Jan Beulich
2019-05-20 15:17         ` [Xen-devel] " Jan Beulich
2019-05-22  9:41         ` Roger Pau Monné
2019-05-22  9:41           ` [Xen-devel] " Roger Pau Monné
2019-07-03 18:30     ` Andrew Cooper
2019-05-17 10:49   ` [PATCH v3 08/15] x86/IRQs: correct/tighten vector check in _clear_irq_vector() Jan Beulich
2019-05-17 10:49     ` [Xen-devel] " Jan Beulich
2019-07-03 18:31     ` Andrew Cooper
2019-05-17 10:49   ` [PATCH v3 09/15] x86/IRQ: make fixup_irqs() skip unconnected internally used interrupts Jan Beulich
2019-05-17 10:49     ` [Xen-devel] " Jan Beulich
2019-07-03 18:36     ` Andrew Cooper
2019-05-17 10:50   ` [PATCH v3 10/15] x86/IRQ: drop redundant cpumask_empty() from move_masked_irq() Jan Beulich
2019-05-17 10:50     ` [Xen-devel] " Jan Beulich
2019-07-03 18:38     ` Andrew Cooper
2019-05-17 10:51   ` [PATCH v3 11/15] x86/IRQ: simplify and rename pirq_acktype() Jan Beulich
2019-05-17 10:51     ` [Xen-devel] " Jan Beulich
2019-07-03 18:39     ` Andrew Cooper
2019-05-17 10:51   ` [PATCH v3 12/15] x86/IRQ: add explicit tracing-enabled check to trace_irq_mask() Jan Beulich
2019-05-17 10:51     ` [Xen-devel] " Jan Beulich
2019-05-20 11:46     ` Roger Pau Monné
2019-05-20 11:46       ` [Xen-devel] " Roger Pau Monné
2019-07-03 18:41     ` Andrew Cooper
2019-07-04 10:01       ` Jan Beulich
2019-05-17 10:52   ` [PATCH v3 13/15] x86/IRQ: tighten vector checks Jan Beulich
2019-05-17 10:52     ` [Xen-devel] " Jan Beulich
2019-05-20 14:04     ` Roger Pau Monné
2019-05-20 14:04       ` [Xen-devel] " Roger Pau Monné
2019-05-20 15:26       ` Jan Beulich
2019-05-20 15:26         ` [Xen-devel] " Jan Beulich
2019-05-22 16:42         ` Roger Pau Monné
2019-05-22 16:42           ` [Xen-devel] " Roger Pau Monné
2019-05-23  8:36           ` Jan Beulich
2019-05-23  8:36             ` [Xen-devel] " Jan Beulich
2019-07-03 18:42     ` Andrew Cooper
2019-05-17 10:52   ` [PATCH v3 14/15] x86/IRQ: eliminate some on-stack cpumask_t instances Jan Beulich
2019-05-17 10:52     ` [Xen-devel] " Jan Beulich
2019-05-20 14:22     ` Roger Pau Monné
2019-05-20 14:22       ` [Xen-devel] " Roger Pau Monné
2019-07-03 18:44       ` Andrew Cooper
2019-07-04 10:04         ` Jan Beulich
2019-05-17 10:53   ` [PATCH v3 15/15] x86/IRQ: move {,_}clear_irq_vector() Jan Beulich
2019-05-17 10:53     ` [Xen-devel] " Jan Beulich
2019-07-03 18:45     ` [Xen-devel] [PATCH v3 15/15] x86/IRQ: move {, _}clear_irq_vector() Andrew Cooper

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=20190503152142.icdzlax4arik3beg@Air-de-Roger \
    --to=roger.pau@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.