All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki via Virtualization <virtualization@lists.linux-foundation.org>
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: Caleb Raitto <caraitto@google.com>,
	kernel-team@cloudflare.com, "Michael S. Tsirkin" <mst@redhat.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask
Date: Mon, 23 Oct 2023 18:52:45 +0200	[thread overview]
Message-ID: <87il6x2rj6.fsf@cloudflare.com> (raw)
In-Reply-To: <1697720122.49851-2-xuanzhuo@linux.alibaba.com>

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@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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Sitnicki <jakub@cloudflare.com>
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	linux-kernel@vger.kernel.org, kernel-team@cloudflare.com,
	Caleb Raitto <caraitto@google.com>,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask
Date: Mon, 23 Oct 2023 18:52:45 +0200	[thread overview]
Message-ID: <87il6x2rj6.fsf@cloudflare.com> (raw)
In-Reply-To: <1697720122.49851-2-xuanzhuo@linux.alibaba.com>

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@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

  reply	other threads:[~2023-10-23 17:11 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-19 10:16 [PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask Jakub Sitnicki via Virtualization
2023-10-19 10:16 ` Jakub Sitnicki
2023-10-19 10:16 ` [PATCH 2/2] virtio_pci: Switch away from deprecated irq_set_affinity_hint Jakub Sitnicki via Virtualization
2023-10-19 10:16   ` Jakub Sitnicki
2023-10-19 12:54   ` Xuan Zhuo
2023-10-19 12:54     ` Xuan Zhuo
2023-10-19 12:55 ` [PATCH 1/2] virtio_pci: Don't make an extra copy of cpu affinity mask Xuan Zhuo
2023-10-19 12:55   ` Xuan Zhuo
2023-10-23 16:52   ` Jakub Sitnicki via Virtualization [this message]
2023-10-23 16:52     ` Jakub Sitnicki
2023-10-24  2:31     ` Xuan Zhuo
2023-10-24  2:31       ` Xuan Zhuo
2023-10-24  8:17       ` Jakub Sitnicki via Virtualization
2023-10-24  8:17         ` Jakub Sitnicki
2023-10-24 10:53         ` Xuan Zhuo
2023-10-24 10:53           ` Xuan Zhuo
2023-10-24 11:26           ` Jakub Sitnicki via Virtualization
2023-10-24 11:26             ` Jakub Sitnicki
2023-10-24 11:46             ` Xuan Zhuo
2023-10-24 11:46               ` Xuan Zhuo
2023-10-24 11:55               ` Jakub Sitnicki via Virtualization
2023-10-24 11:55                 ` Jakub Sitnicki

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=87il6x2rj6.fsf@cloudflare.com \
    --to=virtualization@lists.linux-foundation.org \
    --cc=caraitto@google.com \
    --cc=jakub@cloudflare.com \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=xuanzhuo@linux.alibaba.com \
    /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.