Re: [Xen-devel] [PATCH 2/9] x86/IRQ: deal with move cleanup count state in fixup_irqs()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.05.19 at 10:12, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, May 07, 2019 at 01:28:36AM -0600, Jan Beulich wrote:
>> >>> On 03.05.19 at 17:21, <roger.pau@xxxxxxxxxx> wrote:
>> > On Mon, Apr 29, 2019 at 05:23:20AM -0600, Jan Beulich wrote:
>> >> ---
>> >> 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.
>> 
>> FTR: It's not fully redundant - the patch removing it that I had
>> added was actually the reason for seeing the ASSERT() trigger
>> that you did ask to add in patch 1. The reason is that the field
>> serves as a flag for irq_move_cleanup_interrupt() whether to
>> act on an IRQ in the first place. Without it, the function will
>> prematurely clean up the vector, thus confusing subsequent
>> code trying to do the cleanup when it's actually due.
> 
> So weighing desc->arch.old_cpu_mask is not equivalent to
> move_cleanup_count?

Not exactly, no: While the field gets set from the cpumask_weight()
result, it matter _when_ that happens. Prior to that point, what bits
are set in the mask is of no interest to irq_move_cleanup_interrupt().

Jan



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