Re: [PATCH 1/3] virtio_pci_modern: Remove unnecessary num zero check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 2023-03-08 a.m.9:23, Michael S. Tsirkin wrote:
External email: Use caution opening links or attachments

On Tue, Mar 07, 2023 at 05:57:03AM +0200, Feng Liu wrote:
is_power_of_2() already performs the zero check. Hence avoid duplicate
check. While at it, move the query of size check also adjacent to where
its used for the disabled vq.

Signed-off-by: Feng Liu <feliu@xxxxxxxxxx>
Reviewed-by: Jiri Pirko <jiri@xxxxxxxxxx>
Reviewed-by: Parav Pandit <parav@xxxxxxxxxx>
Reviewed-by: Gavin Li <gavinl@xxxxxxxxxx>
Reviewed-by: Bodong Wang <bodong@xxxxxxxxxx>
  drivers/virtio/virtio_pci_modern.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 9e496e288cfa..3d7144f8f959 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -306,10 +306,10 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
               return ERR_PTR(-EINVAL);

       /* Check if queue is either not available or already active. */
-     num = vp_modern_get_queue_size(mdev, index);
-     if (!num || vp_modern_get_queue_enable(mdev, index))
+     if (vp_modern_get_queue_enable(mdev, index))
               return ERR_PTR(-ENOENT);

+     num = vp_modern_get_queue_size(mdev, index);
       if (!is_power_of_2(num)) {
               dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
               return ERR_PTR(-EINVAL);

As Jason said the right thing to do is to remove the power of 2
limitation. However please don't just hack it up and post,
test it with a variety of queue sizes and with at least
PAGE_POISONING and as many debugging options as you can
to make sure this is not triggering bugs anywhere.

Hi Michael and Jason

Thanks a lot for your tips. I'll be working on these things and fixing this in the next version of this patch series and add testing info


Virtualization mailing list

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux