Xuan Zhuo
2023-Oct-19 12:55 UTC
[PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask
On Thu, 19 Oct 2023 12:16:24 +0200, Jakub Sitnicki <jakub at cloudflare.com> wrote:> Since commit 19e226e8cc5d ("virtio: Make vp_set_vq_affinity() take a > mask.") it is actually not needed to have a local copy of the cpu mask.Could you give more info to prove this? If you are right, I think you should delete all code about msix_affinity_masks? Thanks.> > Pass the cpu mask we got as argument to set the irq affinity hint. > > Cc: Caleb Raitto <caraitto at google.com> > Signed-off-by: Jakub Sitnicki <jakub at cloudflare.com> > --- > drivers/virtio/virtio_pci_common.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index c2524a7207cf..8927bc338f06 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -433,21 +433,14 @@ int vp_set_vq_affinity(struct virtqueue *vq, const struct cpumask *cpu_mask) > struct virtio_device *vdev = vq->vdev; > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index]; > - struct cpumask *mask; > unsigned int irq; > > if (!vq->callback) > return -EINVAL; > > if (vp_dev->msix_enabled) { > - mask = vp_dev->msix_affinity_masks[info->msix_vector]; > irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector); > - if (!cpu_mask) > - irq_set_affinity_hint(irq, NULL); > - else { > - cpumask_copy(mask, cpu_mask); > - irq_set_affinity_hint(irq, mask); > - } > + irq_set_affinity_hint(irq, cpu_mask); > } > return 0; > } > -- > 2.41.0 >
Jakub Sitnicki
2023-Oct-23 16:52 UTC
[PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask
On Thu, Oct 19, 2023 at 08:55 PM +08, Xuan Zhuo wrote:> On Thu, 19 Oct 2023 12:16:24 +0200, Jakub Sitnicki <jakub at cloudflare.com> wrote: >> Since commit 19e226e8cc5d ("virtio: Make vp_set_vq_affinity() take a >> mask.") it is actually not needed to have a local copy of the cpu mask. > > > Could you give more info to prove this? > > If you are right, I think you should delete all code about msix_affinity_masks?Sorry for the late reply. I've been away. It looks that msix_affinity_masks became unused - intentionally - in 2015, after commit 210d150e1f5d ("virtio_pci: Clear stale cpumask when setting irq affinity") [1]. Originally introduced in 2012 in commit 75a0a52be3c2 ("virtio: introduce an API to set affinity for a virtqueue") [2]. As I understand, it was meant to make it possible to set VQ affinity to more than once CPU. Now that we can pass a CPU mask, listing all CPUs, to set_vq_affinity, msix_affinity_masks seems to no longer have a purpose. So, IMO, you're right. We can remove it. Happy to do that in a follow up series. That is - if you're okay with these two patches in the current form. Thanks for reviewing. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=210d150e1f5da506875e376422ba31ead2d49621 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=75a0a52be3c27b58654fbed2c8f2ff401482b9a4