Re: [PATCH] x86/vector: Allow to free vector for managed IRQ

[Ming Lei]

On Fri, Mar 13, 2020 at 03:24:08PM +0100, Thomas Gleixner wrote:
> Peter Xu <peterx@xxxxxxxxxx> writes:
> 
> > After we introduced the "managed_irq" sub-parameter for isolcpus, it's
> > possible to free a kernel managed irq vector now.
> >
> > It can be triggered easily by booting a VM with a few vcpus, with one
> > virtio-blk device and then mark some cores as HK_FLAG_MANAGED_IRQ (in
> > below case, there're 4 vcpus, with vcpu 3 isolated with managed_irq):
> >
> > [    2.889911] ------------[ cut here ]------------
> > [    2.889964] WARNING: CPU: 3 PID: 0 at arch/x86/kernel/apic/vector.c:853 free_moved_vector+0x126/0x160
> 
> <SNIP>
> 
> > [    2.890026] softirqs last disabled at (8757): [<ffffffffbb0ecccd>] irq_enter+0x4d/0x70
> > [    2.890027] ---[ end trace deb5d563d2acb13f ]---
> 
> What is this backtrace for? It's completly useless as it merily shows
> that the warning triggers. Also even if it'd be useful then it wants to
> be trimmed properly.
> 
> > I believe the same thing will happen to bare metals.
> 
> Believe is not really relevant in engineering.
> 
> The problem has nothing to do with virt or bare metal. It's a genuine
> issue.
> 
> > When allocating the IRQ for the device, activate_managed() will try to
> > allocate a vector based on what we've calculated for kernel managed
> > IRQs (which does not take HK_FLAG_MANAGED_IRQ into account).  However
> > when we bind the IRQ to the IRQ handler, we'll do irq_startup() and
> > irq_do_set_affinity(), in which we will start to consider the whole
> > HK_FLAG_MANAGED_IRQ logic.  This means the chosen core can be
> > different from when we do the allocation.  When that happens, we'll
> > need to be able to properly free the old vector on the old core.
> 
> There's lots of 'we' in that text. We do nothing really. Please describe
> things in neutral and factual language.
> 
> Also there is another way to trigger this: Offline all non-isolated CPUs
> in the mask and then bring one online again.
> 
> Ming, I really have to ask why these two situations were not tested
> before the final submission of that isolation patch. Both issues have
> been discussed during review of the different versions. So the warning
> should have triggered back then already....

Hi Thomas,

I run CPU hotplug & unplug stress test with isolcpus:managed_irq, however
the test just checks if the irq's effective vector is setup correctly and
IO can be run as expected.

Looks dmesg warning is missed to check, sorry for that.

Thanks,
Ming