Re: [PATCH v4 0/2] PCI: hv: Refactor hv_irq_unmask() to use hv_vpset and cpumask_to_vpset()

From: Vitaly Kuznetsov
Date: Thu Feb 28 2019 - 05:56:20 EST


Maya Nakamura <m.maya.nakamura@xxxxxxxxx> writes:

> This patchset removes a duplicate definition of VP set (hv_vp_set) and
> uses the common definition (hv_vpset) that is used in other places. It
> changes the order of the members in struct hv_pcibus_device due to
> flexible array in hv_vpset.
>
> It also removes the duplicate implementation of cpumask_to_vpset(), uses
> the shared implementation, and exports hv_max_vp_index, which is
> required by cpumask_to_vpset().
>
> Based on Vitaly's findings, two changes were applied: replace GFP_KERNEL
> with GFP_ATOMIC for alloc_cpumask_var() because hv_irq_unmask() runs
> while a spinlock is held, and add __aligned(8) to struct
> retarget_msi_interrupt because Hyper-V requires that hypercall arguments
> be aligned on an 8 byte boundary.
>
> Vitaly, thank you for finding the issues, and Lorenzo and Michael, thank
> you for your guidance and support!
>

Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
Tested-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

I, however, agree with Lorenzo that '__aligned(8)' change deserves its
own patch as it fixes an already present issue (which may actually be
masked but still present).

Going forward I'd like to get rid of the newly added cpumask allocation:
while it is very unlikely to fail and it only happens when we reassign
IRQs to some different CPU (thus it's not a hotpath or anything)
failures are still possible. This work can definitely be postponed, I
don't think it should block your patches.

--
Vitaly