From 6abb2dd92868927c1d680546b9d13f208373d517 Mon Sep 17 00:00:00 2001 From: Joel Stanley Date: Thu, 13 Feb 2014 15:03:46 +1030 Subject: tools/virtio: fix missing kmemleak_ignore symbol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit bb478d8b167 virtio_ring: plug kmemleak false positive, kmemleak_ignore was introduced. This broke compilation of virtio_test: cc -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -c -o virtio_ring.o ../../drivers/virtio/virtio_ring.c ../../drivers/virtio/virtio_ring.c: In function ‘vring_add_indirect’: ../../drivers/virtio/virtio_ring.c:177:2: warning: implicit declaration of function ‘kmemleak_ignore’ [-Wimplicit-function-declaration] kmemleak_ignore(desc); ^ cc virtio_test.o virtio_ring.o -o virtio_test virtio_ring.o: In function `vring_add_indirect': tools/virtio/../../drivers/virtio/virtio_ring.c:177: undefined reference to `kmemleak_ignore' Add a dummy header for tools/virtio, and add #incldue to drivers/virtio/virtio_ring.c so it is picked up by the userspace tools. Signed-off-by: Joel Stanley Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 28b5338fff71..7ae3cba2f624 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -23,6 +23,7 @@ #include #include #include +#include #ifdef DEBUG /* For development, we want to crash whenever the ring is screwed. */ -- cgit v1.2.3 From 5e37f67063a14450f1fff3baee81efe7c146592a Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Thu, 13 Mar 2014 11:23:37 +1030 Subject: virtio: Use pci_enable_msix_exact() instead of pci_enable_msix() As result of deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() or pci_enable_msi_exact() and pci_enable_msix_range() or pci_enable_msix_exact() interfaces. Signed-off-by: Alexander Gordeev Acked-by: Michael S. Tsirkin Signed-off-by: Rusty Russell --- drivers/virtio/virtio_pci.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a416f9b2a7f6..101db3faf5d4 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -333,10 +333,8 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, for (i = 0; i < nvectors; ++i) vp_dev->msix_entries[i].entry = i; - /* pci_enable_msix returns positive if we can't get this many. */ - err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries, nvectors); - if (err > 0) - err = -ENOSPC; + err = pci_enable_msix_exact(vp_dev->pci_dev, + vp_dev->msix_entries, nvectors); if (err) goto error; vp_dev->msix_enabled = 1; -- cgit v1.2.3 From 1f74ef0f2d7d692fcd615621e0e734c3e7771413 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:38 +1030 Subject: virtio_balloon: don't softlockup on huge balloon changes. When adding or removing 100G from a balloon: BUG: soft lockup - CPU#0 stuck for 22s! [vballoon:367] We have a wait_event_interruptible(), but the condition is always true (more ballooning to do) so we don't ever sleep. We also have a wait_event() for the host to ack, but that is also always true as QEMU is synchronous for balloon operations. Reported-by: Gopesh Kumar Chaudhary Cc: stable@kernel.org Signed-off-by: Rusty Russell --- drivers/virtio/virtio_balloon.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 34bdabaecbd6..36e7859a31aa 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -310,6 +310,12 @@ static int balloon(void *_vballoon) else if (diff < 0) leak_balloon(vb, -diff); update_balloon_size(vb); + + /* + * For large balloon changes, we could spend a lot of time + * and always have work to do. Be nice if preempt disabled. + */ + cond_resched(); } return 0; } -- cgit v1.2.3 From a7c58146cf9a782113629021ba5420582fef265e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:39 +1030 Subject: virtio_net: don't crash if virtqueue is broken. A bad implementation of virtio might cause us to mark the virtqueue broken: we'll dev_err() in that case, and the device is useless, but let's not BUG_ON(). Signed-off-by: Rusty Russell --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 5632a99cbbd2..274e99722e35 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -938,7 +938,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd, sgs[out_num] = &stat; BUG_ON(out_num + 1 > ARRAY_SIZE(sgs)); - BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC) < 0); + virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC); if (unlikely(!virtqueue_kick(vi->cvq))) return status == VIRTIO_NET_OK; -- cgit v1.2.3 From 5261b85e586afe6ebe54e16e0a8acc32fc6d4902 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:39 +1030 Subject: virtio_blk: don't crash, report error if virtqueue is broken. A bad implementation of virtio might cause us to mark the virtqueue broken: we'll dev_err() in that case, and the device is useless, but let's not BUG_ON(). ENOMEM or ENOSPC implies the ring is full, and we should try again later (-ENOMEM is documented to happen, but doesn't, as we fall through to ENOSPC). EIO means it's broken. Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index b1cb3f4c4db4..a2db9ed288f2 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -158,6 +158,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req) unsigned long flags; unsigned int num; const bool last = (req->cmd_flags & REQ_END) != 0; + int err; BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems); @@ -198,11 +199,16 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req) } spin_lock_irqsave(&vblk->vq_lock, flags); - if (__virtblk_add_req(vblk->vq, vbr, vbr->sg, num) < 0) { + err = __virtblk_add_req(vblk->vq, vbr, vbr->sg, num); + if (err) { virtqueue_kick(vblk->vq); spin_unlock_irqrestore(&vblk->vq_lock, flags); blk_mq_stop_hw_queue(hctx); - return BLK_MQ_RQ_QUEUE_BUSY; + /* Out of mem doesn't actually happen, since we fall back + * to direct descriptors */ + if (err == -ENOMEM || err == -ENOSPC) + return BLK_MQ_RQ_QUEUE_BUSY; + return BLK_MQ_RQ_QUEUE_ERROR; } if (last) -- cgit v1.2.3 From 4951cc9083147015338f61a187e072daf02dfd4e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:40 +1030 Subject: virtio_balloon: don't crash if virtqueue is broken. A bad implementation of virtio might cause us to mark the virtqueue broken: we'll dev_err() in that case, and the device is useless, but let's not BUG(). Signed-off-by: Rusty Russell --- drivers/virtio/virtio_balloon.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 36e7859a31aa..25ebe8eecdb7 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -108,8 +108,7 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns); /* We should always be able to add one buffer to an empty queue. */ - if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) - BUG(); + virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); virtqueue_kick(vq); /* When host has read buffer, this completes via balloon_ack */ @@ -258,8 +257,7 @@ static void stats_handle_request(struct virtio_balloon *vb) if (!virtqueue_get_buf(vq, &len)) return; sg_init_one(&sg, vb->stats, sizeof(vb->stats)); - if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) - BUG(); + virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); virtqueue_kick(vq); } @@ -344,7 +342,7 @@ static int init_vqs(struct virtio_balloon *vb) /* * Prime this virtqueue with one buffer so the hypervisor can - * use it to signal us later. + * use it to signal us later (it can't be broken yet!). */ sg_init_one(&sg, vb->stats, sizeof vb->stats); if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL) -- cgit v1.2.3 From 9914a766174d50eb2343f204fef3ee23dbe07c4c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:40 +1030 Subject: virtio-rng: don't crash if virtqueue is broken. A bad implementation of virtio might cause us to mark the virtqueue broken: we'll dev_err() in that case, and the device is useless, but let's not BUG(). Signed-off-by: Rusty Russell --- drivers/char/hw_random/virtio-rng.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c index c12398d1517c..2ce0e225e58c 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -47,8 +47,7 @@ static void register_buffer(u8 *buf, size_t size) sg_init_one(&sg, buf, size); /* There should always be room for one buffer. */ - if (virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL) < 0) - BUG(); + virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL); virtqueue_kick(vq); } -- cgit v1.2.3 From 70670444c20a10717acdc1f4c1e420852995496d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 13 Mar 2014 11:23:40 +1030 Subject: virtio: fail adding buffer on broken queues. Heinz points out that adding buffers to a broken virtqueue (which should "never happen") still works. Failing allows drivers to detect and complain about broken devices. Now drivers are robust, we can add this extra check. Reported-by: Heinz Graalfs Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 7ae3cba2f624..1e443629f76d 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -204,6 +204,11 @@ static inline int virtqueue_add(struct virtqueue *_vq, BUG_ON(data == NULL); + if (unlikely(vq->broken)) { + END_USE(vq); + return -EIO; + } + #ifdef DEBUG { ktime_t now = ktime_get(); @@ -310,7 +315,7 @@ add_head: * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * - * Returns zero or a negative error (ie. ENOSPC, ENOMEM). + * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO). */ int virtqueue_add_sgs(struct virtqueue *_vq, struct scatterlist *sgs[], @@ -348,7 +353,7 @@ EXPORT_SYMBOL_GPL(virtqueue_add_sgs); * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * - * Returns zero or a negative error (ie. ENOSPC, ENOMEM). + * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO). */ int virtqueue_add_outbuf(struct virtqueue *vq, struct scatterlist sg[], unsigned int num, @@ -370,7 +375,7 @@ EXPORT_SYMBOL_GPL(virtqueue_add_outbuf); * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * - * Returns zero or a negative error (ie. ENOSPC, ENOMEM). + * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO). */ int virtqueue_add_inbuf(struct virtqueue *vq, struct scatterlist sg[], unsigned int num, -- cgit v1.2.3 From fc4324b4597c4eb8907207e82f9a6acec84dd335 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Mar 2014 17:08:24 +1030 Subject: virtio-blk: base queue-depth on virtqueue ringsize or module param Venkatash spake thus: virtio-blk set the default queue depth to 64 requests, which was insufficient for high-IOPS devices. Instead set the blk-queue depth to the device's virtqueue depth divided by two (each I/O requires at least two VQ entries). But behold, Ted added a module parameter: Also allow the queue depth to be something which can be set at module load time or via a kernel boot-time parameter, for testing/benchmarking purposes. And I rewrote it substantially, mainly to take VIRTIO_RING_F_INDIRECT_DESC into account. As QEMU sets the vq size for PCI to 128, Venkatash's patch wouldn't have made a change. This version does (since QEMU also offers VIRTIO_RING_F_INDIRECT_DESC. Inspired-by: "Theodore Ts'o" Based-on-the-true-story-of: Venkatesh Srinivas Cc: "Michael S. Tsirkin" Cc: virtio-dev@lists.oasis-open.org Cc: virtualization@lists.linux-foundation.org Cc: Frank Swiderski Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index a2db9ed288f2..196222271a50 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -491,10 +491,11 @@ static struct blk_mq_ops virtio_mq_ops = { static struct blk_mq_reg virtio_mq_reg = { .ops = &virtio_mq_ops, .nr_hw_queues = 1, - .queue_depth = 64, + .queue_depth = 0, /* Set in virtblk_probe */ .numa_node = NUMA_NO_NODE, .flags = BLK_MQ_F_SHOULD_MERGE, }; +module_param_named(queue_depth, virtio_mq_reg.queue_depth, uint, 0444); static void virtblk_init_vbr(void *data, struct blk_mq_hw_ctx *hctx, struct request *rq, unsigned int nr) @@ -558,6 +559,13 @@ static int virtblk_probe(struct virtio_device *vdev) goto out_free_vq; } + /* Default queue sizing is to fill the ring. */ + if (!virtio_mq_reg.queue_depth) { + virtio_mq_reg.queue_depth = vblk->vq->num_free; + /* ... but without indirect descs, we use 2 descs per req */ + if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)) + virtio_mq_reg.queue_depth /= 2; + } virtio_mq_reg.cmd_size = sizeof(struct virtblk_req) + sizeof(struct scatterlist) * sg_elems; -- cgit v1.2.3