* [PATCH V2 2/3] vhost: introduce vhost_vq_more_avail()
From: Jason Wang @ 2015-12-01 6:39 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1448951985-12385-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 13 +++++++++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 163b365..4f45a03 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1633,6 +1633,19 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
}
EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
+bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ __virtio16 avail_idx;
+ int r;
+
+ r = __get_user(avail_idx, &vq->avail->idx);
+ if (r)
+ return false;
+
+ return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
+}
+EXPORT_SYMBOL_GPL(vhost_vq_more_avail);
+
/* OK, now we need to know about added descriptors. */
bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 43284ad..2f3c57c 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -159,6 +159,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
struct vring_used_elem *heads, unsigned count);
void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
+bool vhost_vq_more_avail(struct vhost_dev *, struct vhost_virtqueue *);
bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
--
2.5.0
^ permalink raw reply related
* [PATCH V2 1/3] vhost: introduce vhost_has_work()
From: Jason Wang @ 2015-12-01 6:39 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1448951985-12385-1-git-send-email-jasowang@redhat.com>
This path introduces a helper which can give a hint for whether or not
there's a work queued in the work list.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 7 +++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index eec2f11..163b365 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -245,6 +245,13 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)
}
EXPORT_SYMBOL_GPL(vhost_work_queue);
+/* A lockless hint for busy polling code to exit the loop */
+bool vhost_has_work(struct vhost_dev *dev)
+{
+ return !list_empty(&dev->work_list);
+}
+EXPORT_SYMBOL_GPL(vhost_has_work);
+
void vhost_poll_queue(struct vhost_poll *poll)
{
vhost_work_queue(poll->dev, &poll->work);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d3f7674..43284ad 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -37,6 +37,7 @@ struct vhost_poll {
void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
+bool vhost_has_work(struct vhost_dev *dev);
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
unsigned long mask, struct vhost_dev *dev);
--
2.5.0
^ permalink raw reply related
* [PATCH V2 0/3] basic busy polling support for vhost_net
From: Jason Wang @ 2015-12-01 6:39 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
Hi all:
This series tries to add basic busy polling for vhost net. The idea is
simple: at the end of tx/rx processing, busy polling for new tx added
descriptor and rx receive socket for a while. The maximum number of
time (in us) could be spent on busy polling was specified ioctl.
Test A were done through:
- 50 us as busy loop timeout
- Netperf 2.6
- Two machines with back to back connected ixgbe
- Guest with 1 vcpu and 1 queue
Results:
- For stream workload, ioexits were reduced dramatically in medium
size (1024-2048) of tx (at most -43%) and almost all rx (at most
-84%) as a result of polling. This compensate for the possible
wasted cpu cycles more or less. That porbably why we can still see
some increasing in the normalized throughput in some cases.
- Throughput of tx were increased (at most 50%) expect for the huge
write (16384). And we can send more packets in the case (+tpkts were
increased).
- Very minor rx regression in some cases.
- Improvemnt on TCP_RR (at most 17%).
Guest TX:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
64/ 1/ +18%/ -10%/ +7%/ +11%/ 0%
64/ 2/ +14%/ -13%/ +7%/ +10%/ 0%
64/ 4/ +8%/ -17%/ +7%/ +9%/ 0%
64/ 8/ +11%/ -15%/ +7%/ +10%/ 0%
256/ 1/ +35%/ +9%/ +21%/ +12%/ -11%
256/ 2/ +26%/ +2%/ +20%/ +9%/ -10%
256/ 4/ +23%/ 0%/ +21%/ +10%/ -9%
256/ 8/ +23%/ 0%/ +21%/ +9%/ -9%
512/ 1/ +31%/ +9%/ +23%/ +18%/ -12%
512/ 2/ +30%/ +8%/ +24%/ +15%/ -10%
512/ 4/ +26%/ +5%/ +24%/ +14%/ -11%
512/ 8/ +32%/ +9%/ +23%/ +15%/ -11%
1024/ 1/ +39%/ +16%/ +29%/ +22%/ -26%
1024/ 2/ +35%/ +14%/ +30%/ +21%/ -22%
1024/ 4/ +34%/ +13%/ +32%/ +21%/ -25%
1024/ 8/ +36%/ +14%/ +32%/ +19%/ -26%
2048/ 1/ +50%/ +27%/ +34%/ +26%/ -42%
2048/ 2/ +43%/ +21%/ +36%/ +25%/ -43%
2048/ 4/ +41%/ +20%/ +37%/ +27%/ -43%
2048/ 8/ +40%/ +18%/ +35%/ +25%/ -42%
16384/ 1/ 0%/ -12%/ -1%/ +8%/ +15%
16384/ 2/ 0%/ -10%/ +1%/ +4%/ +5%
16384/ 4/ 0%/ -11%/ -3%/ 0%/ +3%
16384/ 8/ 0%/ -10%/ -4%/ 0%/ +1%
Guest RX:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
64/ 1/ -2%/ -21%/ +1%/ +2%/ -75%
64/ 2/ +1%/ -9%/ +12%/ 0%/ -55%
64/ 4/ 0%/ -6%/ +5%/ -1%/ -44%
64/ 8/ -5%/ -5%/ +7%/ -23%/ -50%
256/ 1/ -8%/ -18%/ +16%/ +15%/ -63%
256/ 2/ 0%/ -8%/ +9%/ -2%/ -26%
256/ 4/ 0%/ -7%/ -8%/ +20%/ -41%
256/ 8/ -8%/ -11%/ -9%/ -24%/ -78%
512/ 1/ -6%/ -19%/ +20%/ +18%/ -29%
512/ 2/ 0%/ -10%/ -14%/ -8%/ -31%
512/ 4/ -1%/ -5%/ -11%/ -9%/ -38%
512/ 8/ -7%/ -9%/ -17%/ -22%/ -81%
1024/ 1/ 0%/ -16%/ +12%/ +9%/ -11%
1024/ 2/ 0%/ -11%/ 0%/ +3%/ -30%
1024/ 4/ 0%/ -4%/ +2%/ +6%/ -15%
1024/ 8/ -3%/ -4%/ -8%/ -8%/ -70%
2048/ 1/ -8%/ -23%/ +36%/ +22%/ -11%
2048/ 2/ 0%/ -12%/ +1%/ +3%/ -29%
2048/ 4/ 0%/ -3%/ -17%/ -15%/ -84%
2048/ 8/ 0%/ -3%/ +1%/ -3%/ +10%
16384/ 1/ 0%/ -11%/ +4%/ +7%/ -22%
16384/ 2/ 0%/ -7%/ +4%/ +4%/ -33%
16384/ 4/ 0%/ -2%/ -2%/ -4%/ -23%
16384/ 8/ -1%/ -2%/ +1%/ -22%/ -40%
TCP_RR:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
1/ 1/ +11%/ -26%/ +11%/ +11%/ +10%
1/ 25/ +11%/ -15%/ +11%/ +11%/ 0%
1/ 50/ +9%/ -16%/ +10%/ +10%/ 0%
1/ 100/ +9%/ -15%/ +9%/ +9%/ 0%
64/ 1/ +11%/ -31%/ +11%/ +11%/ +11%
64/ 25/ +12%/ -14%/ +12%/ +12%/ 0%
64/ 50/ +11%/ -14%/ +12%/ +12%/ 0%
64/ 100/ +11%/ -15%/ +11%/ +11%/ 0%
256/ 1/ +11%/ -27%/ +11%/ +11%/ +10%
256/ 25/ +17%/ -11%/ +16%/ +16%/ -1%
256/ 50/ +16%/ -11%/ +17%/ +17%/ +1%
256/ 100/ +17%/ -11%/ +18%/ +18%/ +1%
Test B were done through:
- 50us as busy loop timeout
- Netperf 2.6
- Two machines with back to back connected ixgbe
- Two guests each wich 1 vcpu and 1 queue
- pin two vhost threads to the same cpu on host to simulate the cpu
contending
Results:
- In this radical case, we can still get at most 14% improvement on
TCP_RR.
- For guest tx stream, minor improvemnt with at most 5% regression in
one byte case. For guest rx stream, at most 5% regression were seen.
Guest TX:
size /-+% /
1 /-5.55%/
64 /+1.11%/
256 /+2.33%/
512 /-0.03%/
1024 /+1.14%/
4096 /+0.00%/
16384/+0.00%/
Guest RX:
size /-+% /
1 /-5.11%/
64 /-0.55%/
256 /-2.35%/
512 /-3.39%/
1024 /+6.8% /
4096 /-0.01%/
16384/+0.00%/
TCP_RR:
size /-+% /
1 /+9.79% /
64 /+4.51% /
256 /+6.47% /
512 /-3.37% /
1024 /+6.15% /
4096 /+14.88%/
16384/-2.23% /
Changes from V1:
- Remove the buggy vq_error() in vhost_vq_more_avail().
- Leave vhost_enable_notify() untouched.
Changes from RFC V3:
- small tweak on the code to avoid multiple duplicate conditions in
critical path when busy loop is not enabled.
- Add the test result of multiple VMs
Changes from RFC V2:
- poll also at the end of rx handling
- factor out the polling logic and optimize the code a little bit
- add two ioctls to get and set the busy poll timeout
- test on ixgbe (which can give more stable and reproducable numbers)
instead of mlx4.
Changes from RFC V1:
- Add a comment for vhost_has_work() to explain why it could be
lockless
- Add param description for busyloop_timeout
- Split out the busy polling logic into a new helper
- Check and exit the loop when there's a pending signal
- Disable preemption during busy looping to make sure lock_clock() was
correctly used.
Jason Wang (3):
vhost: introduce vhost_has_work()
vhost: introduce vhost_vq_more_avail()
vhost_net: basic polling support
drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
drivers/vhost/vhost.c | 35 ++++++++++++++++++++++
drivers/vhost/vhost.h | 3 ++
include/uapi/linux/vhost.h | 11 +++++++
4 files changed, 116 insertions(+), 5 deletions(-)
--
2.5.0
^ permalink raw reply
* Re: [PATCH net-next 3/3] vhost_net: basic polling support
From: Jason Wang @ 2015-12-01 5:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20151130124233-mutt-send-email-mst@redhat.com>
On 11/30/2015 06:44 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 25, 2015 at 03:11:29PM +0800, Jason Wang wrote:
>> > This patch tries to poll for new added tx buffer or socket receive
>> > queue for a while at the end of tx/rx processing. The maximum time
>> > spent on polling were specified through a new kind of vring ioctl.
>> >
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> One further enhancement would be to actually poll
> the underlying device. This should be reasonably
> straight-forward with macvtap (especially in the
> passthrough mode).
>
>
Yes, it is. I have some patches to do this by replacing
skb_queue_empty() with sk_busy_loop() but for tap. Tests does not show
any improvement but some regression. Maybe it's better to test macvtap.
^ permalink raw reply
* Re: [PATCH net-next 2/3] vhost: introduce vhost_vq_more_avail()
From: Jason Wang @ 2015-12-01 5:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20151130082202.GA7396@redhat.com>
On 11/30/2015 04:22 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 25, 2015 at 03:11:28PM +0800, Jason Wang wrote:
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/vhost.c | 26 +++++++++++++++++---------
>> drivers/vhost/vhost.h | 1 +
>> 2 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 163b365..b86c5aa 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -1633,10 +1633,25 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
>> }
>> EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
>>
>> +bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> +{
>> + __virtio16 avail_idx;
>> + int r;
>> +
>> + r = __get_user(avail_idx, &vq->avail->idx);
>> + if (r) {
>> + vq_err(vq, "Failed to check avail idx at %p: %d\n",
>> + &vq->avail->idx, r);
>> + return false;
> In patch 3 you are calling this under preempt disable,
> so this actually can fail and it isn't a VQ error.
>
Yes.
>> + }
>> +
>> + return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
>> +}
>> +EXPORT_SYMBOL_GPL(vhost_vq_more_avail);
>> +
>> /* OK, now we need to know about added descriptors. */
>> bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> {
>> - __virtio16 avail_idx;
>> int r;
>>
>> if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
>> @@ -1660,14 +1675,7 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> /* They could have slipped one in as we were doing that: make
>> * sure it's written, then check again. */
>> smp_mb();
>> - r = __get_user(avail_idx, &vq->avail->idx);
>> - if (r) {
>> - vq_err(vq, "Failed to check avail idx at %p: %d\n",
>> - &vq->avail->idx, r);
>> - return false;
>> - }
>> -
>> - return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
>> + return vhost_vq_more_avail(dev, vq);
>> }
>> EXPORT_SYMBOL_GPL(vhost_enable_notify);
>>
> This path does need an error though.
> It's probably easier to just leave this call site alone.
Ok, will leave this function as is and remove the vq_err() in
vhost_vq_more_avail().
Thanks
>
>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>> index 43284ad..2f3c57c 100644
>> --- a/drivers/vhost/vhost.h
>> +++ b/drivers/vhost/vhost.h
>> @@ -159,6 +159,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
>> struct vring_used_elem *heads, unsigned count);
>> void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
>> void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>> +bool vhost_vq_more_avail(struct vhost_dev *, struct vhost_virtqueue *);
>> bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>>
>> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
>> --
>> 2.5.0
^ permalink raw reply
* Re: [RFC PATCH 0/9] vhost-nvme: new qemu nvme backend using nvme target
From: Ming Lin @ 2015-11-30 23:20 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <56531F5F.3050709@redhat.com>
On Mon, 2015-11-23 at 15:14 +0100, Paolo Bonzini wrote:
>
> On 23/11/2015 09:17, Ming Lin wrote:
> > On Sat, 2015-11-21 at 14:11 +0100, Paolo Bonzini wrote:
> >>
> >> On 20/11/2015 01:20, Ming Lin wrote:
> >>> One improvment could be to use google's NVMe vendor extension that
> >>> I send in another thread, aslo here:
> >>> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
> >>>
> >>> Qemu side:
> >>> http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
> >>> Kernel side also here:
> >>> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
> >>
> >> How much do you get with vhost-nvme plus vendor extension, compared to
> >> 190 MB/s for QEMU?
> >
> > There is still some bug. I'll update.
>
> Sure.
Fixed it after Thanksgiving holiday :)
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0-ext
Combined with previous test results(lower to higher):
qemu-nvme: 148MB/s
vhost-nvme + google-ext: 230MB/s
qemu-nvme + google-ext + eventfd: 294MB/s
virtio-scsi: 296MB/s
virtio-blk: 344MB/s
"vhost-nvme + google-ext" didn't get good enough performance.
Still tuning.
^ permalink raw reply
* Re: [PATCH v2] vhost: replace % with & on data path
From: David Miller @ 2015-11-30 16:17 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448874894-18398-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 30 Nov 2015 11:15:23 +0200
> We know vring num is a power of 2, so use &
> to mask the high bits.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] vhost: replace % with & on data path
From: David Miller @ 2015-11-30 16:13 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 30 Nov 2015 10:34:07 +0200
> We know vring num is a power of 2, so use &
> to mask the high bits.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/vhost.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 080422f..85f0f0a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1366,10 +1366,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
> /* Only get avail ring entries after they have been exposed by guest. */
> smp_rmb();
>
> + }
> +
!!!
^ permalink raw reply
* Re: [PATCH net-next 3/3] vhost_net: basic polling support
From: Michael S. Tsirkin @ 2015-11-30 10:44 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448435489-5949-4-git-send-email-jasowang@redhat.com>
On Wed, Nov 25, 2015 at 03:11:29PM +0800, Jason Wang wrote:
> This patch tries to poll for new added tx buffer or socket receive
> queue for a while at the end of tx/rx processing. The maximum time
> spent on polling were specified through a new kind of vring ioctl.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
One further enhancement would be to actually poll
the underlying device. This should be reasonably
straight-forward with macvtap (especially in the
passthrough mode).
> ---
> drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
> drivers/vhost/vhost.c | 15 ++++++++++
> drivers/vhost/vhost.h | 1 +
> include/uapi/linux/vhost.h | 11 +++++++
> 4 files changed, 94 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e..ce6da77 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> rcu_read_unlock_bh();
> }
>
> +static inline unsigned long busy_clock(void)
> +{
> + return local_clock() >> 10;
> +}
> +
> +static bool vhost_can_busy_poll(struct vhost_dev *dev,
> + unsigned long endtime)
> +{
> + return likely(!need_resched()) &&
> + likely(!time_after(busy_clock(), endtime)) &&
> + likely(!signal_pending(current)) &&
> + !vhost_has_work(dev) &&
> + single_task_running();
> +}
> +
> +static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> + struct vhost_virtqueue *vq,
> + struct iovec iov[], unsigned int iov_size,
> + unsigned int *out_num, unsigned int *in_num)
> +{
> + unsigned long uninitialized_var(endtime);
> +
> + if (vq->busyloop_timeout) {
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> + while (vhost_can_busy_poll(vq->dev, endtime) &&
> + !vhost_vq_more_avail(vq->dev, vq))
> + cpu_relax();
> + preempt_enable();
> + }
> +
> + return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> + out_num, in_num, NULL, NULL);
> +}
> +
> /* Expects to be always run from workqueue - which acts as
> * read-size critical section for our kind of RCU. */
> static void handle_tx(struct vhost_net *net)
> @@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
> % UIO_MAXIOV == nvq->done_idx))
> break;
>
> - head = vhost_get_vq_desc(vq, vq->iov,
> - ARRAY_SIZE(vq->iov),
> - &out, &in,
> - NULL, NULL);
> + head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
> + ARRAY_SIZE(vq->iov),
> + &out, &in);
> /* On error, stop handling until the next kick. */
> if (unlikely(head < 0))
> break;
> @@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
> return len;
> }
>
> +static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
> +{
> + struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> + struct vhost_virtqueue *vq = &nvq->vq;
> + unsigned long uninitialized_var(endtime);
> +
> + if (vq->busyloop_timeout) {
> + mutex_lock(&vq->mutex);
> + vhost_disable_notify(&net->dev, vq);
> +
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> +
> + while (vhost_can_busy_poll(&net->dev, endtime) &&
> + skb_queue_empty(&sk->sk_receive_queue) &&
> + !vhost_vq_more_avail(&net->dev, vq))
> + cpu_relax();
> +
> + preempt_enable();
> +
> + if (vhost_enable_notify(&net->dev, vq))
> + vhost_poll_queue(&vq->poll);
> + mutex_unlock(&vq->mutex);
> + }
> +
> + return peek_head_len(sk);
> +}
> +
> /* This is a multi-buffer version of vhost_get_desc, that works if
> * vq has read descriptors only.
> * @vq - the relevant virtqueue
> @@ -553,7 +615,7 @@ static void handle_rx(struct vhost_net *net)
> vq->log : NULL;
> mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
>
> - while ((sock_len = peek_head_len(sock->sk))) {
> + while ((sock_len = vhost_net_peek_head_len(net, sock->sk))) {
> sock_len += sock_hlen;
> vhost_len = sock_len + vhost_hlen;
> headcount = get_rx_bufs(vq, vq->heads, vhost_len,
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index b86c5aa..857af6c 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -285,6 +285,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->memory = NULL;
> vq->is_le = virtio_legacy_is_little_endian();
> vhost_vq_reset_user_be(vq);
> + vq->busyloop_timeout = 0;
> }
>
> static int vhost_worker(void *data)
> @@ -747,6 +748,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> struct vhost_vring_state s;
> struct vhost_vring_file f;
> struct vhost_vring_addr a;
> + struct vhost_vring_busyloop_timeout t;
> u32 idx;
> long r;
>
> @@ -919,6 +921,19 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> case VHOST_GET_VRING_ENDIAN:
> r = vhost_get_vring_endian(vq, idx, argp);
> break;
> + case VHOST_SET_VRING_BUSYLOOP_TIMEOUT:
> + if (copy_from_user(&t, argp, sizeof(t))) {
> + r = -EFAULT;
> + break;
> + }
> + vq->busyloop_timeout = t.timeout;
> + break;
> + case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
> + t.index = idx;
> + t.timeout = vq->busyloop_timeout;
> + if (copy_to_user(argp, &t, sizeof(t)))
> + r = -EFAULT;
> + break;
> default:
> r = -ENOIOCTLCMD;
> }
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 2f3c57c..4b7d4fa 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -115,6 +115,7 @@ struct vhost_virtqueue {
> /* Ring endianness requested by userspace for cross-endian support. */
> bool user_be;
> #endif
> + u32 busyloop_timeout;
> };
>
> struct vhost_dev {
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index ab373191..eaf6c33 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -27,6 +27,11 @@ struct vhost_vring_file {
>
> };
>
> +struct vhost_vring_busyloop_timeout {
> + unsigned int index;
> + unsigned int timeout;
> +};
> +
> struct vhost_vring_addr {
> unsigned int index;
> /* Option flags. */
> @@ -126,6 +131,12 @@ struct vhost_memory {
> #define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
> /* Set eventfd to signal an error */
> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> +/* Set busy loop timeout */
> +#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23, \
> + struct vhost_vring_busyloop_timeout)
> +/* Get busy loop timeout */
> +#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24, \
> + struct vhost_vring_busyloop_timeout)
>
> /* VHOST_NET specific defines */
>
> --
> 2.5.0
^ permalink raw reply
* Re: [PATCH] tools/virtio: move list macro stubs
From: Cornelia Huck @ 2015-11-30 10:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: David Hildenbrand, linux-kernel, virtualization
In-Reply-To: <1448872891-13717-1-git-send-email-mst@redhat.com>
On Mon, 30 Nov 2015 10:41:35 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Makes them more generally available.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> tools/virtio/linux/kernel.h | 6 ++++++
> tools/virtio/linux/virtio.h | 6 ------
> 2 files changed, 6 insertions(+), 6 deletions(-)
Create a dummy list.h? Or would that be overkill?
^ permalink raw reply
* Re: [PATCH] tools/virtio: fix byteswap logic
From: Cornelia Huck @ 2015-11-30 10:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, linux-kernel
In-Reply-To: <1448872410-11553-1-git-send-email-mst@redhat.com>
On Mon, 30 Nov 2015 10:33:34 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> commit cf561f0d2eb74574ad9985a2feab134267a9d298 ("virtio: introduce
> virtio_is_little_endian() helper") changed byteswap logic to
> skip feature bit checks for LE platforms, but didn't
> update tools/virtio, so vring_bench started failing.
>
> Update the copy under tools/virtio/ (TODO: find a way to avoid this code
> duplication).
Not to mention that it is easy to miss the code duplication...
>
> Cc: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> tools/virtio/linux/virtio_config.h | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH] tools/virtio: fix byteswap logic
From: Greg Kurz @ 2015-11-30 9:38 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization
In-Reply-To: <1448872410-11553-1-git-send-email-mst@redhat.com>
On Mon, 30 Nov 2015 10:33:34 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> commit cf561f0d2eb74574ad9985a2feab134267a9d298 ("virtio: introduce
> virtio_is_little_endian() helper") changed byteswap logic to
> skip feature bit checks for LE platforms, but didn't
> update tools/virtio, so vring_bench started failing.
>
Oops... yet another oversight...
> Update the copy under tools/virtio/ (TODO: find a way to avoid this code
> duplication).
>
> Cc: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> tools/virtio/linux/virtio_config.h | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/tools/virtio/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h
> index 806d683..57a6964 100644
> --- a/tools/virtio/linux/virtio_config.h
> +++ b/tools/virtio/linux/virtio_config.h
> @@ -40,33 +40,39 @@ static inline void __virtio_clear_bit(struct virtio_device *vdev,
> #define virtio_has_feature(dev, feature) \
> (__virtio_test_bit((dev), feature))
>
> +static inline bool virtio_is_little_endian(struct virtio_device *vdev)
> +{
> + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1) ||
> + virtio_legacy_is_little_endian();
> +}
> +
> +/* Memory accessors */
> static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
> {
> - return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
> }
>
> static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
> {
> - return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
> }
>
> static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
> {
> - return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
> }
>
> static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
> {
> - return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
> }
>
> static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
> {
> - return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
> }
>
> static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
> {
> - return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
> + return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
> }
> -
^ permalink raw reply
* Re: [PATCH v2] vhost: replace % with & on data path
From: Christian Borntraeger @ 2015-11-30 9:21 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel; +Cc: netdev, kvm, virtualization
In-Reply-To: <1448874894-18398-1-git-send-email-mst@redhat.com>
On 11/30/2015 10:15 AM, Michael S. Tsirkin wrote:
> We know vring num is a power of 2, so use &
> to mask the high bits.
Makes a lot of sense and virtio_ring.c seems to use the same logic.
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Changes from v1: drop an unrelated chunk
>
> drivers/vhost/vhost.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 080422f..ad2146a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1369,7 +1369,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
> /* Grab the next descriptor number they're advertising, and increment
> * the index we've seen. */
> if (unlikely(__get_user(ring_head,
> - &vq->avail->ring[last_avail_idx % vq->num]))) {
> + &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
> vq_err(vq, "Failed to read head: idx %d address %p\n",
> last_avail_idx,
> &vq->avail->ring[last_avail_idx % vq->num]);
> @@ -1489,7 +1489,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
> u16 old, new;
> int start;
>
> - start = vq->last_used_idx % vq->num;
> + start = vq->last_used_idx & (vq->num - 1);
> used = vq->used->ring + start;
> if (count == 1) {
> if (__put_user(heads[0].id, &used->id)) {
> @@ -1531,7 +1531,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
> {
> int start, n, r;
>
> - start = vq->last_used_idx % vq->num;
> + start = vq->last_used_idx & (vq->num - 1);
> n = vq->num - start;
> if (n < count) {
> r = __vhost_add_used_n(vq, heads, n);
>
^ permalink raw reply
* Re: [PATCH] vhost: replace % with & on data path
From: Michael S. Tsirkin @ 2015-11-30 9:21 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448872969.3032.1.camel@perches.com>
On Mon, Nov 30, 2015 at 12:42:49AM -0800, Joe Perches wrote:
> On Mon, 2015-11-30 at 10:34 +0200, Michael S. Tsirkin wrote:
> > We know vring num is a power of 2, so use &
> > to mask the high bits.
> []
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> []
> > @@ -1366,10 +1366,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
> > /* Only get avail ring entries after they have been exposed by guest. */
> > smp_rmb();
> >
> > + }
>
> ?
Yes, I noticed this - I moved this chunk from the next patch
in my tree by mistake.
Will fix, thanks!
--
MST
^ permalink raw reply
* Re: [PATCH] vhost: replace % with & on data path
From: kbuild test robot @ 2015-11-30 9:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, virtualization, kbuild-all, kvm, linux-kernel
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 18891 bytes --]
Hi Michael,
[auto build test ERROR on: v4.4-rc3]
[also build test ERROR on: next-20151127]
url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/vhost-replace-with-on-data-path/20151130-163704
config: x86_64-randconfig-s0-11301655 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/vhost/vhost.c: In function 'vhost_get_vq_desc':
drivers/vhost/vhost.c:1345:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^
drivers/vhost/vhost.c:1344:13: warning: unused variable 'ring_head' [-Wunused-variable]
__virtio16 ring_head;
^
drivers/vhost/vhost.c:1341:24: warning: unused variable 'found' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1341:18: warning: unused variable 'head' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1341:15: warning: unused variable 'i' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1340:20: warning: unused variable 'desc' [-Wunused-variable]
struct vring_desc desc;
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
drivers/vhost/vhost.c: At top level:
>> include/linux/compiler.h:147:2: error: expected identifier or '(' before 'if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
include/linux/compiler.h:161:3: error: expected identifier or '(' before ')' token
}))
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
>> drivers/vhost/vhost.c:1373:2: note: in expansion of macro 'if'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1381:2: warning: data definition has no type or storage class
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1381:2: error: type defaults to 'int' in declaration of 'head' [-Werror=implicit-int]
drivers/vhost/vhost.c:1381:24: error: 'vq' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1381:28: error: 'ring_head' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
>> include/linux/compiler.h:147:2: error: expected identifier or '(' before 'if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
drivers/vhost/vhost.c:1384:2: note: in expansion of macro 'if'
if (unlikely(head >= vq->num)) {
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
drivers/vhost/vhost.c:1384:2: note: in expansion of macro 'if'
if (unlikely(head >= vq->num)) {
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1384:6: note: in expansion of macro 'unlikely'
if (unlikely(head >= vq->num)) {
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
drivers/vhost/vhost.c:1384:2: note: in expansion of macro 'if'
if (unlikely(head >= vq->num)) {
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1384:6: note: in expansion of macro 'unlikely'
if (unlikely(head >= vq->num)) {
^
include/linux/compiler.h:161:3: error: expected identifier or '(' before ')' token
}))
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
drivers/vhost/vhost.c:1384:2: note: in expansion of macro 'if'
if (unlikely(head >= vq->num)) {
^
drivers/vhost/vhost.c:1391:2: warning: data definition has no type or storage class
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1391:3: error: type defaults to 'int' in declaration of 'out_num' [-Werror=implicit-int]
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1391:14: error: 'in_num' undeclared here (not in a function)
*out_num = *in_num = 0;
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
>> include/linux/compiler.h:147:2: error: expected identifier or '(' before 'if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
drivers/vhost/vhost.c:1392:2: note: in expansion of macro 'if'
if (unlikely(log))
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
drivers/vhost/vhost.c:1392:2: note: in expansion of macro 'if'
if (unlikely(log))
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1392:6: note: in expansion of macro 'unlikely'
if (unlikely(log))
^
include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
drivers/vhost/vhost.c:1392:2: note: in expansion of macro 'if'
if (unlikely(log))
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1392:6: note: in expansion of macro 'unlikely'
if (unlikely(log))
^
include/linux/compiler.h:161:3: error: expected identifier or '(' before ')' token
}))
^
include/linux/compiler.h:145:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^
drivers/vhost/vhost.c:1392:2: note: in expansion of macro 'if'
if (unlikely(log))
^
drivers/vhost/vhost.c:1395:2: warning: data definition has no type or storage class
i = head;
^
drivers/vhost/vhost.c:1395:2: error: type defaults to 'int' in declaration of 'i' [-Werror=implicit-int]
drivers/vhost/vhost.c:1395:6: error: initializer element is not constant
i = head;
^
drivers/vhost/vhost.c:1396:2: error: expected identifier or '(' before 'do'
do {
^
drivers/vhost/vhost.c:1454:4: error: expected identifier or '(' before 'while'
} while ((i = next_desc(vq, &desc)) != -1);
^
drivers/vhost/vhost.c:1457:4: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token
vq->last_avail_idx++;
^
In file included from arch/x86/include/asm/bug.h:35:0,
from include/linux/bug.h:4,
from include/linux/thread_info.h:11,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/wait.h:8,
from include/linux/eventfd.h:12,
from drivers/vhost/vhost.c:14:
include/asm-generic/bug.h:55:27: error: expected identifier or '(' before 'do'
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
^
drivers/vhost/vhost.c:1461:2: note: in expansion of macro 'BUG_ON'
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
^
include/asm-generic/bug.h:55:66: error: expected identifier or '(' before 'while'
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
^
drivers/vhost/vhost.c:1461:2: note: in expansion of macro 'BUG_ON'
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
^
drivers/vhost/vhost.c:1462:2: error: expected identifier or '(' before 'return'
return head;
^
drivers/vhost/vhost.c:1463:1: error: expected identifier or '(' before '}' token
}
^
drivers/vhost/vhost.c:1235:12: warning: 'get_indirect' defined but not used [-Wunused-function]
static int get_indirect(struct vhost_virtqueue *vq,
^
drivers/vhost/vhost.c: In function 'vhost_get_vq_desc':
drivers/vhost/vhost.c:1369:2: warning: control reaches end of non-void function [-Wreturn-type]
}
^
cc1: some warnings being treated as errors
vim +/if +1373 drivers/vhost/vhost.c
3a4d5c94 Michael S. Tsirkin 2010-01-14 1357 vq_err(vq, "Guest moved used index from %u to %u",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1358 last_avail_idx, vq->avail_idx);
d5675bd2 Michael S. Tsirkin 2010-06-24 1359 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1360 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1361
3a4d5c94 Michael S. Tsirkin 2010-01-14 1362 /* If there's nothing new since last we looked, return invalid. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 1363 if (vq->avail_idx == last_avail_idx)
3a4d5c94 Michael S. Tsirkin 2010-01-14 1364 return vq->num;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1365
3a4d5c94 Michael S. Tsirkin 2010-01-14 1366 /* Only get avail ring entries after they have been exposed by guest. */
5659338c Michael S. Tsirkin 2010-02-01 1367 smp_rmb();
3a4d5c94 Michael S. Tsirkin 2010-01-14 1368
07a08023 Michael S. Tsirkin 2015-11-30 1369 }
07a08023 Michael S. Tsirkin 2015-11-30 1370
3a4d5c94 Michael S. Tsirkin 2010-01-14 1371 /* Grab the next descriptor number they're advertising, and increment
3a4d5c94 Michael S. Tsirkin 2010-01-14 1372 * the index we've seen. */
3b1bbe89 Michael S. Tsirkin 2014-10-24 @1373 if (unlikely(__get_user(ring_head,
07a08023 Michael S. Tsirkin 2015-11-30 1374 &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1375 vq_err(vq, "Failed to read head: idx %d address %p\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1376 last_avail_idx,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1377 &vq->avail->ring[last_avail_idx % vq->num]);
d5675bd2 Michael S. Tsirkin 2010-06-24 1378 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1379 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1380
3b1bbe89 Michael S. Tsirkin 2014-10-24 1381 head = vhost16_to_cpu(vq, ring_head);
:::::: The code at line 1373 was first introduced by commit
:::::: 3b1bbe89351a8003857aeb5cbef3595f5d0ee609 vhost: virtio 1.0 endian-ness support
:::::: TO: Michael S. Tsirkin <mst@redhat.com>
:::::: CC: Michael S. Tsirkin <mst@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 28813 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH RFC v2] virtio: skip avail/used index reads
From: Michael S. Tsirkin @ 2015-11-30 9:18 UTC (permalink / raw)
To: linux-kernel; +Cc: virtio, kvm, netdev, virtualization, David Gibson
This adds a new vring feature bit: when enabled, host and guest poll the
available/used ring directly instead of looking at the index field
first.
To guarantee it is possible to detect updates, the high bits (above
vring.num - 1) in the ring head ID value are modified to match the index
bits - these change on each wrap-around. Writer also XORs this with
0x8000 such that rings can be zero-initialized.
Reader is modified to ignore these high bits when looking
up descriptors.
The point is to reduce the number of cacheline misses
for both reads and writes.
I see a performance improvement of about 20% on multithreaded benchmarks
(e.g. virtio-test), but regression of about 2% on vring_bench.
I think this has to do with the fact that complete_multi_user
is implemented suboptimally.
TODO:
investigate single-threaded regression
look at more aggressive ring layout changes
better name for a feature flag
split the patch to make it easier to review
This is on top of the following patches in my tree:
virtio_ring: Shadow available ring flags & index
vhost: replace % with & on data path
tools/virtio: fix byteswap logic
tools/virtio: move list macro stubs
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v1:
add a missing chunk in vhost_get_vq_desc
drivers/vhost/vhost.h | 3 +-
include/linux/vringh.h | 3 +
include/uapi/linux/virtio_ring.h | 3 +
drivers/vhost/vhost.c | 104 ++++++++++++++++++--------
drivers/vhost/vringh.c | 153 +++++++++++++++++++++++++++++++++------
drivers/virtio/virtio_ring.c | 40 ++++++++--
tools/virtio/virtio_test.c | 14 +++-
7 files changed, 256 insertions(+), 64 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d3f7674..aeeb15d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,8 @@ enum {
(1ULL << VIRTIO_RING_F_EVENT_IDX) |
(1ULL << VHOST_F_LOG_ALL) |
(1ULL << VIRTIO_F_ANY_LAYOUT) |
- (1ULL << VIRTIO_F_VERSION_1)
+ (1ULL << VIRTIO_F_VERSION_1) |
+ (1ULL << VIRTIO_RING_F_POLL)
};
static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index bc6c28d..13a9e3e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -40,6 +40,9 @@ struct vringh {
/* Can we get away with weak barriers? */
bool weak_barriers;
+ /* Poll ring directly */
+ bool poll;
+
/* Last available index we saw (ie. where we're up to). */
u16 last_avail_idx;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index c072959..bf3ca1d 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -62,6 +62,9 @@
* at the end of the used ring. Guest should ignore the used->flags field. */
#define VIRTIO_RING_F_EVENT_IDX 29
+/* Support ring polling */
+#define VIRTIO_RING_F_POLL 33
+
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ad2146a..cdbabf5 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1346,25 +1346,27 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Check it isn't doing very strange things with descriptor numbers. */
last_avail_idx = vq->last_avail_idx;
- if (unlikely(__get_user(avail_idx, &vq->avail->idx))) {
- vq_err(vq, "Failed to access avail idx at %p\n",
- &vq->avail->idx);
- return -EFAULT;
- }
- vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
+ if (!vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ if (unlikely(__get_user(avail_idx, &vq->avail->idx))) {
+ vq_err(vq, "Failed to access avail idx at %p\n",
+ &vq->avail->idx);
+ return -EFAULT;
+ }
+ vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
- if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
- vq_err(vq, "Guest moved used index from %u to %u",
- last_avail_idx, vq->avail_idx);
- return -EFAULT;
- }
+ if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
+ vq_err(vq, "Guest moved used index from %u to %u",
+ last_avail_idx, vq->avail_idx);
+ return -EFAULT;
+ }
- /* If there's nothing new since last we looked, return invalid. */
- if (vq->avail_idx == last_avail_idx)
- return vq->num;
+ /* If there's nothing new since last we looked, return invalid. */
+ if (vq->avail_idx == last_avail_idx)
+ return vq->num;
- /* Only get avail ring entries after they have been exposed by guest. */
- smp_rmb();
+ /* Only get avail ring entries after they have been exposed by guest. */
+ smp_rmb();
+ }
/* Grab the next descriptor number they're advertising, and increment
* the index we've seen. */
@@ -1378,11 +1380,22 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
head = vhost16_to_cpu(vq, ring_head);
- /* If their number is silly, that's an error. */
- if (unlikely(head >= vq->num)) {
- vq_err(vq, "Guest says index %u > %u is available",
- head, vq->num);
- return -EINVAL;
+ if (vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ /* If there's nothing new since last we looked, return invalid. */
+ if ((head ^ last_avail_idx ^ 0x8000) & ~(vq->num - 1))
+ return vq->num;
+
+ /* Only get avail ring entries after they have been exposed by guest. */
+ smp_rmb();
+
+ head &= vq->num - 1;
+ } else {
+ /* If their number is silly, that's an error. */
+ if (unlikely(head >= vq->num)) {
+ vq_err(vq, "Guest says index %u > %u is available",
+ head, vq->num);
+ return -EINVAL;
+ }
}
/* When we start there are none of either input nor output. */
@@ -1481,6 +1494,27 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
}
EXPORT_SYMBOL_GPL(vhost_add_used);
+static inline int __vhost_add_used_poll(struct vhost_virtqueue *vq,
+ struct vring_used_elem *heads,
+ struct vring_used_elem __user *used,
+ int idx)
+{
+ __virtio32 head = heads[0].id ^
+ cpu_to_vhost32(vq, ~(vq->num - 1) &
+ ((vq->last_used_idx + idx) ^ 0x8000));
+
+ if (__put_user(heads[0].len, &used->len)) {
+ vq_err(vq, "Failed to write used len");
+ return -EFAULT;
+ }
+ smp_wmb();
+ if (__put_user(head, &used->id)) {
+ vq_err(vq, "Failed to write used id");
+ return -EFAULT;
+ }
+ return 0;
+}
+
static int __vhost_add_used_n(struct vhost_virtqueue *vq,
struct vring_used_elem *heads,
unsigned count)
@@ -1491,18 +1525,28 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
- if (count == 1) {
- if (__put_user(heads[0].id, &used->id)) {
- vq_err(vq, "Failed to write used id");
+ if (vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < count; ++i)
+ ret |= __vhost_add_used_poll(vq, heads + i, used + i, i);
+ if (ret)
return -EFAULT;
- }
- if (__put_user(heads[0].len, &used->len)) {
- vq_err(vq, "Failed to write used len");
+ } else {
+ if (count == 1) {
+ if (__put_user(heads[0].id, &used->id)) {
+ vq_err(vq, "Failed to write used id");
+ return -EFAULT;
+ }
+ if (__put_user(heads[0].len, &used->len)) {
+ vq_err(vq, "Failed to write used len");
+ return -EFAULT;
+ }
+ } else if (__copy_to_user(used, heads, count * sizeof *used)) {
+ vq_err(vq, "Failed to write used");
return -EFAULT;
}
- } else if (__copy_to_user(used, heads, count * sizeof *used)) {
- vq_err(vq, "Failed to write used");
- return -EFAULT;
}
if (unlikely(vq->log_used)) {
/* Make sure data is seen before log. */
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 3bb02c6..d8aac79 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -36,18 +36,21 @@ static inline int __vringh_get_head(const struct vringh *vrh,
u16 avail_idx, i, head;
int err;
- err = getu16(vrh, &avail_idx, &vrh->vring.avail->idx);
- if (err) {
- vringh_bad("Failed to access avail idx at %p",
- &vrh->vring.avail->idx);
- return err;
- }
+ if (!vrh->poll) {
+ err = getu16(vrh, &avail_idx, &vrh->vring.avail->idx);
+ if (err) {
+ vringh_bad("Failed to access avail idx at %p",
+ &vrh->vring.avail->idx);
+ return err;
+ }
- if (*last_avail_idx == avail_idx)
- return vrh->vring.num;
+ if (*last_avail_idx == avail_idx)
+ return vrh->vring.num;
- /* Only get avail ring entries after they have been exposed by guest. */
- virtio_rmb(vrh->weak_barriers);
+ /* Only get avail ring entries after they have been exposed by guest. */
+ virtio_rmb(vrh->weak_barriers);
+
+ }
i = *last_avail_idx & (vrh->vring.num - 1);
@@ -58,10 +61,20 @@ static inline int __vringh_get_head(const struct vringh *vrh,
return err;
}
- if (head >= vrh->vring.num) {
- vringh_bad("Guest says index %u > %u is available",
- head, vrh->vring.num);
- return -EINVAL;
+ if (vrh->poll) {
+ if ((head ^ *last_avail_idx ^ 0x8000) & ~(vrh->vring.num - 1))
+ return vrh->vring.num;
+
+ /* Only get descriptor entries after they have been exposed by guest. */
+ virtio_rmb(vrh->weak_barriers);
+
+ head &= vrh->vring.num - 1;
+ } else {
+ if (head >= vrh->vring.num) {
+ vringh_bad("Guest says index %u > %u is available",
+ head, vrh->vring.num);
+ return -EINVAL;
+ }
}
(*last_avail_idx)++;
@@ -397,6 +410,57 @@ fail:
return err;
}
+static inline int __vringh_complete_poll(struct vringh *vrh,
+ int (*putu32)(const struct vringh *vrh,
+ __virtio32 *p, u32 val),
+ int (*putu16)(const struct vringh *vrh,
+ __virtio16 *p, u16 val),
+ __virtio32 head, __virtio32 len,
+ bool last)
+{
+ struct vring_used *used_ring;
+ int err;
+ u16 used_idx, off;
+
+ used_ring = vrh->vring.used;
+ used_idx = vrh->last_used_idx + vrh->completed;
+
+ off = used_idx & (vrh->vring.num - 1);
+
+ err = putu32(vrh, &used_ring->ring[off].len, len);
+ if (err) {
+ vringh_bad("Failed to write used length %u at %p",
+ off, &used_ring->ring[off]);
+ return err;
+ }
+
+ /* Make sure length is written before we update index. */
+ virtio_wmb(vrh->weak_barriers);
+
+ head ^= cpu_to_vringh32(vrh, (used_idx & ~(vrh->vring.num - 1)) ^ 0x8000);
+ err = putu32(vrh, &used_ring->ring[off].id, head);
+ if (err) {
+ vringh_bad("Failed to write used id %u at %p",
+ off, &used_ring->ring[off]);
+ return err;
+ }
+
+ if (last) {
+ /* Make sure buffer is written before we update index. */
+ virtio_wmb(vrh->weak_barriers);
+
+ err = putu16(vrh, &vrh->vring.used->idx, used_idx + 1);
+ if (err) {
+ vringh_bad("Failed to update used index at %p",
+ &vrh->vring.used->idx);
+ return err;
+ }
+ }
+
+ vrh->completed++;
+ return 0;
+}
+
static inline int __vringh_complete(struct vringh *vrh,
const struct vring_used_elem *used,
unsigned int num_used,
@@ -556,6 +620,13 @@ static inline int getu16_user(const struct vringh *vrh, u16 *val, const __virtio
return rc;
}
+static inline int putu32_user(const struct vringh *vrh, __virtio32 *p, u32 val)
+{
+ __virtio32 v = cpu_to_vringh32(vrh, val);
+ return put_user(v, (__force __virtio32 __user *)p);
+}
+
+
static inline int putu16_user(const struct vringh *vrh, __virtio16 *p, u16 val)
{
__virtio16 v = cpu_to_vringh16(vrh, val);
@@ -615,6 +686,7 @@ int vringh_init_user(struct vringh *vrh, u64 features,
vrh->little_endian = (features & (1ULL << VIRTIO_F_VERSION_1));
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
+ vrh->poll = (features & (1ULL << VIRTIO_RING_F_POLL));
vrh->weak_barriers = weak_barriers;
vrh->completed = 0;
vrh->last_avail_idx = 0;
@@ -752,11 +824,19 @@ EXPORT_SYMBOL(vringh_abandon_user);
*/
int vringh_complete_user(struct vringh *vrh, u16 head, u32 len)
{
- struct vring_used_elem used;
+ if (vrh->poll) {
+ return __vringh_complete_poll(vrh, putu32_user,
+ putu16_user,
+ cpu_to_vringh32(vrh, head),
+ cpu_to_vringh32(vrh, len),
+ true);
+ } else {
+ struct vring_used_elem used;
- used.id = cpu_to_vringh32(vrh, head);
- used.len = cpu_to_vringh32(vrh, len);
- return __vringh_complete(vrh, &used, 1, putu16_user, putused_user);
+ used.id = cpu_to_vringh32(vrh, head);
+ used.len = cpu_to_vringh32(vrh, len);
+ return __vringh_complete(vrh, &used, 1, putu16_user, putused_user);
+ }
}
EXPORT_SYMBOL(vringh_complete_user);
@@ -773,8 +853,21 @@ int vringh_complete_multi_user(struct vringh *vrh,
const struct vring_used_elem used[],
unsigned num_used)
{
- return __vringh_complete(vrh, used, num_used,
- putu16_user, putused_user);
+ if (vrh->poll) {
+ int i, r;
+
+ for (i = 0; i < num_used; ++i) {
+ r = __vringh_complete_poll(vrh, putu32_user, putu16_user,
+ used[i].id, used[i].len,
+ i == num_used - 1);
+ if (r)
+ return r;
+ }
+ return 0;
+ } else {
+ return __vringh_complete(vrh, used, num_used,
+ putu16_user, putused_user);
+ }
}
EXPORT_SYMBOL(vringh_complete_multi_user);
@@ -830,6 +923,12 @@ static inline int putu16_kern(const struct vringh *vrh, __virtio16 *p, u16 val)
return 0;
}
+static inline int putu32_kern(const struct vringh *vrh, __virtio32 *p, u32 val)
+{
+ ACCESS_ONCE(*p) = cpu_to_vringh32(vrh, val);
+ return 0;
+}
+
static inline int copydesc_kern(void *dst, const void *src, size_t len)
{
memcpy(dst, src, len);
@@ -876,6 +975,7 @@ int vringh_init_kern(struct vringh *vrh, u64 features,
vrh->little_endian = (features & (1ULL << VIRTIO_F_VERSION_1));
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
+ vrh->poll = (features & (1ULL << VIRTIO_RING_F_POLL));
vrh->weak_barriers = weak_barriers;
vrh->completed = 0;
vrh->last_avail_idx = 0;
@@ -987,12 +1087,17 @@ EXPORT_SYMBOL(vringh_abandon_kern);
*/
int vringh_complete_kern(struct vringh *vrh, u16 head, u32 len)
{
- struct vring_used_elem used;
+ if (vrh->poll) {
+ return __vringh_complete_poll(vrh, putu32_kern, putu16_kern,
+ head, len, true);
+ } else {
+ struct vring_used_elem used;
- used.id = cpu_to_vringh32(vrh, head);
- used.len = cpu_to_vringh32(vrh, len);
+ used.id = cpu_to_vringh32(vrh, head);
+ used.len = cpu_to_vringh32(vrh, len);
- return __vringh_complete(vrh, &used, 1, putu16_kern, putused_kern);
+ return __vringh_complete(vrh, &used, 1, putu16_kern, putused_kern);
+ }
}
EXPORT_SYMBOL(vringh_complete_kern);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 6262015..f25fd64 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -80,6 +80,9 @@ struct vring_virtqueue {
/* Last used index we've seen. */
u16 last_used_idx;
+ /* Poll ring instead of the index */
+ bool poll;
+
/* Last written value to avail->flags */
u16 avail_flags_shadow;
@@ -239,9 +242,14 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* Set token. */
vq->data[head] = data;
+ avail = vq->avail_idx_shadow & (vq->vring.num - 1);
+
+ if (vq->poll) {
+ virtio_wmb(vq->weak_barriers);
+ head ^= ((vq->avail_idx_shadow ^ 0x8000) & ~(vq->vring.num - 1));
+ }
/* Put entry in available array (but don't update avail->idx until they
* do sync). */
- avail = vq->avail_idx_shadow & (vq->vring.num - 1);
vq->vring.avail->ring[avail] = cpu_to_virtio16(_vq->vdev, head);
/* Descriptors and available array need to be set before we expose the
@@ -488,17 +496,32 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
return NULL;
}
- if (!more_used(vq)) {
- pr_debug("No more buffers in queue\n");
- END_USE(vq);
- return NULL;
- }
+ if (!vq->poll) {
+ if (!more_used(vq)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
- /* Only get used array entries after they have been exposed by host. */
- virtio_rmb(vq->weak_barriers);
+ /* Only get used array entries after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+ }
last_used = (vq->last_used_idx & (vq->vring.num - 1));
i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
+
+ if (vq->poll) {
+ if ((i ^ vq->last_used_idx ^ 0x8000) & ~(vq->vring.num - 1)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
+ i &= vq->vring.num - 1;
+
+ /* Only get used array entries after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+ }
+
*len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
if (unlikely(i >= vq->vring.num)) {
@@ -764,6 +787,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
+ vq->poll = virtio_has_feature(vdev, VIRTIO_RING_F_POLL);
/* No callback? Tell other side not to bother us. */
if (!callback) {
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index e044589..585cd21 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -220,6 +220,14 @@ const struct option longopts[] = {
.val = 'e',
},
{
+ .name = "ring-poll",
+ .val = 'R',
+ },
+ {
+ .name = "no-ring-poll",
+ .val = 'r',
+ },
+ {
.name = "indirect",
.val = 'I',
},
@@ -261,7 +269,8 @@ int main(int argc, char **argv)
{
struct vdev_info dev;
unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
- (1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
+ (1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1) |
+ (1ULL << VIRTIO_RING_F_POLL);
int o;
bool delayed = false;
@@ -276,6 +285,9 @@ int main(int argc, char **argv)
case 'e':
features &= ~(1ULL << VIRTIO_RING_F_EVENT_IDX);
break;
+ case 'r':
+ features &= ~(1ULL << VIRTIO_RING_F_POLL);
+ break;
case 'h':
help();
goto done;
--
MST
^ permalink raw reply related
* [PATCH v2] vhost: replace % with & on data path
From: Michael S. Tsirkin @ 2015-11-30 9:15 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kvm, virtualization
We know vring num is a power of 2, so use &
to mask the high bits.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v1: drop an unrelated chunk
drivers/vhost/vhost.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 080422f..ad2146a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1369,7 +1369,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Grab the next descriptor number they're advertising, and increment
* the index we've seen. */
if (unlikely(__get_user(ring_head,
- &vq->avail->ring[last_avail_idx % vq->num]))) {
+ &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
vq_err(vq, "Failed to read head: idx %d address %p\n",
last_avail_idx,
&vq->avail->ring[last_avail_idx % vq->num]);
@@ -1489,7 +1489,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
u16 old, new;
int start;
- start = vq->last_used_idx % vq->num;
+ start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
if (count == 1) {
if (__put_user(heads[0].id, &used->id)) {
@@ -1531,7 +1531,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
{
int start, n, r;
- start = vq->last_used_idx % vq->num;
+ start = vq->last_used_idx & (vq->num - 1);
n = vq->num - start;
if (n < count) {
r = __vhost_add_used_n(vq, heads, n);
--
MST
^ permalink raw reply related
* Re: [PATCH] vhost: replace % with & on data path
From: Michael S. Tsirkin @ 2015-11-30 9:11 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kvm, virtualization
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
On Mon, Nov 30, 2015 at 10:34:07AM +0200, Michael S. Tsirkin wrote:
> We know vring num is a power of 2, so use &
> to mask the high bits.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/vhost.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 080422f..85f0f0a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1366,10 +1366,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
> /* Only get avail ring entries after they have been exposed by guest. */
> smp_rmb();
>
> + }
> +
Oops. This sneaked in from an unrelated patch.
Pls ignore, will repost.
> /* Grab the next descriptor number they're advertising, and increment
> * the index we've seen. */
> if (unlikely(__get_user(ring_head,
> - &vq->avail->ring[last_avail_idx % vq->num]))) {
> + &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
> vq_err(vq, "Failed to read head: idx %d address %p\n",
> last_avail_idx,
> &vq->avail->ring[last_avail_idx % vq->num]);
> @@ -1489,7 +1491,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
> u16 old, new;
> int start;
>
> - start = vq->last_used_idx % vq->num;
> + start = vq->last_used_idx & (vq->num - 1);
> used = vq->used->ring + start;
> if (count == 1) {
> if (__put_user(heads[0].id, &used->id)) {
> @@ -1531,7 +1533,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
> {
> int start, n, r;
>
> - start = vq->last_used_idx % vq->num;
> + start = vq->last_used_idx & (vq->num - 1);
> n = vq->num - start;
> if (n < count) {
> r = __vhost_add_used_n(vq, heads, n);
> --
> MST
^ permalink raw reply
* Re: [PATCH] vhost: replace % with & on data path
From: kbuild test robot @ 2015-11-30 9:05 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, virtualization, kbuild-all, kvm, linux-kernel
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 17093 bytes --]
Hi Michael,
[auto build test ERROR on: v4.4-rc3]
[also build test ERROR on: next-20151127]
url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/vhost-replace-with-on-data-path/20151130-163704
config: i386-randconfig-s1-201548 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
drivers/vhost/vhost.c: In function 'vhost_get_vq_desc':
>> drivers/vhost/vhost.c:1345:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^
>> drivers/vhost/vhost.c:1344:13: warning: unused variable 'ring_head' [-Wunused-variable]
__virtio16 ring_head;
^
>> drivers/vhost/vhost.c:1341:24: warning: unused variable 'found' [-Wunused-variable]
unsigned int i, head, found = 0;
^
>> drivers/vhost/vhost.c:1341:18: warning: unused variable 'head' [-Wunused-variable]
unsigned int i, head, found = 0;
^
>> drivers/vhost/vhost.c:1341:15: warning: unused variable 'i' [-Wunused-variable]
unsigned int i, head, found = 0;
^
>> drivers/vhost/vhost.c:1340:20: warning: unused variable 'desc' [-Wunused-variable]
struct vring_desc desc;
^
drivers/vhost/vhost.c: At top level:
>> drivers/vhost/vhost.c:1373:2: error: expected identifier or '(' before 'if'
if (unlikely(__get_user(ring_head,
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
>> arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:137:45: note: in definition of macro 'unlikely'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
>> drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
>> arch/x86/include/asm/uaccess.h:414:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:137:53: note: in definition of macro 'unlikely'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
arch/x86/include/asm/uaccess.h:479:2: note: in expansion of macro '__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
^
>> drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
>> include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
>> drivers/vhost/vhost.c:1373:6: note: in expansion of macro 'unlikely'
if (unlikely(__get_user(ring_head,
^
>> drivers/vhost/vhost.c:1381:2: warning: data definition has no type or storage class
head = vhost16_to_cpu(vq, ring_head);
^
>> drivers/vhost/vhost.c:1381:2: error: type defaults to 'int' in declaration of 'head' [-Werror=implicit-int]
>> drivers/vhost/vhost.c:1381:24: error: 'vq' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
>> drivers/vhost/vhost.c:1381:28: error: 'ring_head' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1384:2: error: expected identifier or '(' before 'if'
if (unlikely(head >= vq->num)) {
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
>> include/linux/compiler.h:126:4: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
^
drivers/vhost/vhost.c:1384:6: note: in expansion of macro 'unlikely'
if (unlikely(head >= vq->num)) {
^
drivers/vhost/vhost.c:1391:2: warning: data definition has no type or storage class
*out_num = *in_num = 0;
^
>> drivers/vhost/vhost.c:1391:3: error: type defaults to 'int' in declaration of 'out_num' [-Werror=implicit-int]
*out_num = *in_num = 0;
^
>> drivers/vhost/vhost.c:1391:14: error: 'in_num' undeclared here (not in a function)
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1392:2: error: expected identifier or '(' before 'if'
if (unlikely(log))
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/x86/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
vim +1373 drivers/vhost/vhost.c
d5675bd2 Michael S. Tsirkin 2010-06-24 1334 * returned on error. */
47283bef Michael S. Tsirkin 2014-06-05 1335 int vhost_get_vq_desc(struct vhost_virtqueue *vq,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1336 struct iovec iov[], unsigned int iov_size,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1337 unsigned int *out_num, unsigned int *in_num,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1338 struct vhost_log *log, unsigned int *log_num)
3a4d5c94 Michael S. Tsirkin 2010-01-14 1339 {
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1340 struct vring_desc desc;
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1341 unsigned int i, head, found = 0;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1342 u16 last_avail_idx;
3b1bbe89 Michael S. Tsirkin 2014-10-24 1343 __virtio16 avail_idx;
3b1bbe89 Michael S. Tsirkin 2014-10-24 @1344 __virtio16 ring_head;
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1345 int ret;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1346
3a4d5c94 Michael S. Tsirkin 2010-01-14 1347 /* Check it isn't doing very strange things with descriptor numbers. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 1348 last_avail_idx = vq->last_avail_idx;
3b1bbe89 Michael S. Tsirkin 2014-10-24 1349 if (unlikely(__get_user(avail_idx, &vq->avail->idx))) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1350 vq_err(vq, "Failed to access avail idx at %p\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1351 &vq->avail->idx);
d5675bd2 Michael S. Tsirkin 2010-06-24 1352 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1353 }
3b1bbe89 Michael S. Tsirkin 2014-10-24 1354 vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
3a4d5c94 Michael S. Tsirkin 2010-01-14 1355
7b3384fc Michael S. Tsirkin 2010-07-01 1356 if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1357 vq_err(vq, "Guest moved used index from %u to %u",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1358 last_avail_idx, vq->avail_idx);
d5675bd2 Michael S. Tsirkin 2010-06-24 1359 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1360 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1361
3a4d5c94 Michael S. Tsirkin 2010-01-14 1362 /* If there's nothing new since last we looked, return invalid. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 1363 if (vq->avail_idx == last_avail_idx)
3a4d5c94 Michael S. Tsirkin 2010-01-14 1364 return vq->num;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1365
3a4d5c94 Michael S. Tsirkin 2010-01-14 1366 /* Only get avail ring entries after they have been exposed by guest. */
5659338c Michael S. Tsirkin 2010-02-01 1367 smp_rmb();
3a4d5c94 Michael S. Tsirkin 2010-01-14 1368
07a08023 Michael S. Tsirkin 2015-11-30 @1369 }
07a08023 Michael S. Tsirkin 2015-11-30 1370
3a4d5c94 Michael S. Tsirkin 2010-01-14 1371 /* Grab the next descriptor number they're advertising, and increment
3a4d5c94 Michael S. Tsirkin 2010-01-14 1372 * the index we've seen. */
3b1bbe89 Michael S. Tsirkin 2014-10-24 @1373 if (unlikely(__get_user(ring_head,
07a08023 Michael S. Tsirkin 2015-11-30 1374 &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1375 vq_err(vq, "Failed to read head: idx %d address %p\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1376 last_avail_idx,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1377 &vq->avail->ring[last_avail_idx % vq->num]);
d5675bd2 Michael S. Tsirkin 2010-06-24 1378 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1379 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1380
3b1bbe89 Michael S. Tsirkin 2014-10-24 @1381 head = vhost16_to_cpu(vq, ring_head);
3b1bbe89 Michael S. Tsirkin 2014-10-24 1382
3a4d5c94 Michael S. Tsirkin 2010-01-14 1383 /* If their number is silly, that's an error. */
7b3384fc Michael S. Tsirkin 2010-07-01 @1384 if (unlikely(head >= vq->num)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1385 vq_err(vq, "Guest says index %u > %u is available",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1386 head, vq->num);
d5675bd2 Michael S. Tsirkin 2010-06-24 1387 return -EINVAL;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1388 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1389
3a4d5c94 Michael S. Tsirkin 2010-01-14 1390 /* When we start there are none of either input nor output. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1391 *out_num = *in_num = 0;
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1392 if (unlikely(log))
3a4d5c94 Michael S. Tsirkin 2010-01-14 1393 *log_num = 0;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1394
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1395 i = head;
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1396 do {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1397 unsigned iov_count = *in_num + *out_num;
7b3384fc Michael S. Tsirkin 2010-07-01 1398 if (unlikely(i >= vq->num)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1399 vq_err(vq, "Desc index is %u > %u, head = %u",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1400 i, vq->num, head);
d5675bd2 Michael S. Tsirkin 2010-06-24 1401 return -EINVAL;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1402 }
7b3384fc Michael S. Tsirkin 2010-07-01 1403 if (unlikely(++found > vq->num)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1404 vq_err(vq, "Loop detected: last one at %u "
3a4d5c94 Michael S. Tsirkin 2010-01-14 1405 "vq size %u head %u\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1406 i, vq->num, head);
d5675bd2 Michael S. Tsirkin 2010-06-24 1407 return -EINVAL;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1408 }
fcc042a2 Michael S. Tsirkin 2011-03-06 1409 ret = __copy_from_user(&desc, vq->desc + i, sizeof desc);
7b3384fc Michael S. Tsirkin 2010-07-01 1410 if (unlikely(ret)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1411 vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1412 i, vq->desc + i);
d5675bd2 Michael S. Tsirkin 2010-06-24 1413 return -EFAULT;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1414 }
3b1bbe89 Michael S. Tsirkin 2014-10-24 1415 if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_INDIRECT)) {
47283bef Michael S. Tsirkin 2014-06-05 1416 ret = get_indirect(vq, iov, iov_size,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1417 out_num, in_num,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1418 log, log_num, &desc);
7b3384fc Michael S. Tsirkin 2010-07-01 1419 if (unlikely(ret < 0)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1420 vq_err(vq, "Failure detected "
3a4d5c94 Michael S. Tsirkin 2010-01-14 1421 "in indirect descriptor at idx %d\n", i);
d5675bd2 Michael S. Tsirkin 2010-06-24 1422 return ret;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1423 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1424 continue;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1425 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1426
3b1bbe89 Michael S. Tsirkin 2014-10-24 1427 ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
3b1bbe89 Michael S. Tsirkin 2014-10-24 1428 vhost32_to_cpu(vq, desc.len), iov + iov_count,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1429 iov_size - iov_count);
7b3384fc Michael S. Tsirkin 2010-07-01 1430 if (unlikely(ret < 0)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1431 vq_err(vq, "Translation failure %d descriptor idx %d\n",
3a4d5c94 Michael S. Tsirkin 2010-01-14 1432 ret, i);
d5675bd2 Michael S. Tsirkin 2010-06-24 1433 return ret;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1434 }
3b1bbe89 Michael S. Tsirkin 2014-10-24 1435 if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1436 /* If this is an input descriptor,
3a4d5c94 Michael S. Tsirkin 2010-01-14 1437 * increment that count. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 1438 *in_num += ret;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1439 if (unlikely(log)) {
3b1bbe89 Michael S. Tsirkin 2014-10-24 1440 log[*log_num].addr = vhost64_to_cpu(vq, desc.addr);
3b1bbe89 Michael S. Tsirkin 2014-10-24 1441 log[*log_num].len = vhost32_to_cpu(vq, desc.len);
3a4d5c94 Michael S. Tsirkin 2010-01-14 1442 ++*log_num;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1443 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1444 } else {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1445 /* If it's an output descriptor, they're all supposed
3a4d5c94 Michael S. Tsirkin 2010-01-14 1446 * to come before any input descriptors. */
7b3384fc Michael S. Tsirkin 2010-07-01 1447 if (unlikely(*in_num)) {
3a4d5c94 Michael S. Tsirkin 2010-01-14 1448 vq_err(vq, "Descriptor has out after in: "
3a4d5c94 Michael S. Tsirkin 2010-01-14 1449 "idx %d\n", i);
d5675bd2 Michael S. Tsirkin 2010-06-24 1450 return -EINVAL;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1451 }
3a4d5c94 Michael S. Tsirkin 2010-01-14 1452 *out_num += ret;
3a4d5c94 Michael S. Tsirkin 2010-01-14 1453 }
3b1bbe89 Michael S. Tsirkin 2014-10-24 @1454 } while ((i = next_desc(vq, &desc)) != -1);
3a4d5c94 Michael S. Tsirkin 2010-01-14 1455
3a4d5c94 Michael S. Tsirkin 2010-01-14 1456 /* On success, increment avail index. */
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1457 vq->last_avail_idx++;
8ea8cf89 Michael S. Tsirkin 2011-05-20 1458
8ea8cf89 Michael S. Tsirkin 2011-05-20 1459 /* Assume notifications from guest are disabled at this point,
8ea8cf89 Michael S. Tsirkin 2011-05-20 1460 * if they aren't we would need to update avail_event index. */
8ea8cf89 Michael S. Tsirkin 2011-05-20 @1461 BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1462 return head;
3a4d5c94 Michael S. Tsirkin 2010-01-14 @1463 }
6ac1afbf Asias He 2013-05-06 1464 EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
3a4d5c94 Michael S. Tsirkin 2010-01-14 1465
3a4d5c94 Michael S. Tsirkin 2010-01-14 1466 /* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */
:::::: The code at line 1373 was first introduced by commit
:::::: 3b1bbe89351a8003857aeb5cbef3595f5d0ee609 vhost: virtio 1.0 endian-ness support
:::::: TO: Michael S. Tsirkin <mst@redhat.com>
:::::: CC: Michael S. Tsirkin <mst@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23955 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] vhost: replace % with & on data path
From: kbuild test robot @ 2015-11-30 9:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, virtualization, kbuild-all, kvm, linux-kernel
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 9273 bytes --]
Hi Michael,
[auto build test ERROR on: v4.4-rc3]
[also build test ERROR on: next-20151127]
url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/vhost-replace-with-on-data-path/20151130-163704
config: s390-performance_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390
All errors (new ones prefixed by >>):
drivers/vhost/vhost.c: In function 'vhost_get_vq_desc':
drivers/vhost/vhost.c:1345:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^
drivers/vhost/vhost.c:1344:13: warning: unused variable 'ring_head' [-Wunused-variable]
__virtio16 ring_head;
^
drivers/vhost/vhost.c:1341:24: warning: unused variable 'found' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1341:18: warning: unused variable 'head' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1341:15: warning: unused variable 'i' [-Wunused-variable]
unsigned int i, head, found = 0;
^
drivers/vhost/vhost.c:1340:20: warning: unused variable 'desc' [-Wunused-variable]
struct vring_desc desc;
^
drivers/vhost/vhost.c: At top level:
drivers/vhost/vhost.c:1373:2: error: expected identifier or '(' before 'if'
if (unlikely(__get_user(ring_head,
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/asm-generic/fcntl.h:4,
from arch/s390/include/uapi/asm/fcntl.h:1,
from include/uapi/linux/fcntl.h:4,
from include/linux/fcntl.h:4,
from include/linux/eventfd.h:11,
from drivers/vhost/vhost.c:14:
>> arch/s390/include/asm/uaccess.h:250:2: error: expected identifier or '(' before ')' token
})
^
include/linux/compiler.h:166:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
drivers/vhost/vhost.c:1373:15: note: in expansion of macro '__get_user'
if (unlikely(__get_user(ring_head,
^
drivers/vhost/vhost.c:1381:2: warning: data definition has no type or storage class
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1381:2: error: type defaults to 'int' in declaration of 'head' [-Werror=implicit-int]
drivers/vhost/vhost.c:1381:24: error: 'vq' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1381:28: error: 'ring_head' undeclared here (not in a function)
head = vhost16_to_cpu(vq, ring_head);
^
drivers/vhost/vhost.c:1384:2: error: expected identifier or '(' before 'if'
if (unlikely(head >= vq->num)) {
^
drivers/vhost/vhost.c:1391:2: warning: data definition has no type or storage class
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1391:3: error: type defaults to 'int' in declaration of 'out_num' [-Werror=implicit-int]
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1391:14: error: 'in_num' undeclared here (not in a function)
*out_num = *in_num = 0;
^
drivers/vhost/vhost.c:1392:2: error: expected identifier or '(' before 'if'
if (unlikely(log))
^
drivers/vhost/vhost.c:1395:2: warning: data definition has no type or storage class
i = head;
^
drivers/vhost/vhost.c:1395:2: error: type defaults to 'int' in declaration of 'i' [-Werror=implicit-int]
drivers/vhost/vhost.c:1395:2: error: initializer element is not constant
drivers/vhost/vhost.c:1396:2: error: expected identifier or '(' before 'do'
do {
^
drivers/vhost/vhost.c:1454:4: error: expected identifier or '(' before 'while'
} while ((i = next_desc(vq, &desc)) != -1);
^
drivers/vhost/vhost.c:1457:4: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token
vq->last_avail_idx++;
^
In file included from arch/s390/include/asm/bug.h:69:0,
from include/linux/bug.h:4,
from include/linux/thread_info.h:11,
from include/asm-generic/preempt.h:4,
from arch/s390/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/wait.h:8,
from include/linux/eventfd.h:12,
from drivers/vhost/vhost.c:14:
include/asm-generic/bug.h:55:27: error: expected identifier or '(' before 'do'
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
^
drivers/vhost/vhost.c:1461:2: note: in expansion of macro 'BUG_ON'
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
^
include/asm-generic/bug.h:55:66: error: expected identifier or '(' before 'while'
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
^
drivers/vhost/vhost.c:1461:2: note: in expansion of macro 'BUG_ON'
BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
^
drivers/vhost/vhost.c:1462:2: error: expected identifier or '(' before 'return'
return head;
^
drivers/vhost/vhost.c:1463:1: error: expected identifier or '(' before '}' token
}
^
drivers/vhost/vhost.c:1235:12: warning: 'get_indirect' defined but not used [-Wunused-function]
static int get_indirect(struct vhost_virtqueue *vq,
^
drivers/vhost/vhost.c: In function 'vhost_get_vq_desc':
drivers/vhost/vhost.c:1369:2: warning: control reaches end of non-void function [-Wreturn-type]
}
^
cc1: some warnings being treated as errors
vim +250 arch/s390/include/asm/uaccess.h
cfa785e62 arch/s390/include/asm/uaccess.h Heiko Carstens 2014-01-22 234 sizeof(*(ptr))); \
97fa5a664 include/asm-s390/uaccess.h Al Viro 2006-02-03 235 (x) = *(__force __typeof__(*(ptr)) *) &__x; \
1047aa772 include/asm-s390/uaccess.h Martin Schwidefsky 2005-11-07 236 break; \
1047aa772 include/asm-s390/uaccess.h Martin Schwidefsky 2005-11-07 237 }; \
1047aa772 include/asm-s390/uaccess.h Martin Schwidefsky 2005-11-07 238 case 8: { \
1047aa772 include/asm-s390/uaccess.h Martin Schwidefsky 2005-11-07 239 unsigned long long __x; \
cfa785e62 arch/s390/include/asm/uaccess.h Heiko Carstens 2014-01-22 240 __gu_err = __get_user_fn(&__x, ptr, \
cfa785e62 arch/s390/include/asm/uaccess.h Heiko Carstens 2014-01-22 241 sizeof(*(ptr))); \
97fa5a664 include/asm-s390/uaccess.h Al Viro 2006-02-03 242 (x) = *(__force __typeof__(*(ptr)) *) &__x; \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 243 break; \
1047aa772 include/asm-s390/uaccess.h Martin Schwidefsky 2005-11-07 244 }; \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 245 default: \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 246 __get_user_bad(); \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 247 break; \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 248 } \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 249 __gu_err; \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 @250 })
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 251
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 252 #define get_user(x, ptr) \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 253 ({ \
dab4079d5 arch/s390/include/asm/uaccess.h Heiko Carstens 2009-06-12 254 might_fault(); \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 255 __get_user(x, ptr); \
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 256 })
^1da177e4 include/asm-s390/uaccess.h Linus Torvalds 2005-04-16 257
4f41c2b45 arch/s390/include/asm/uaccess.h Heiko Carstens 2014-01-23 258 int __get_user_bad(void) __attribute__((noreturn));
:::::: The code at line 250 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 15608 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH RFC] virtio: skip avail/used index reads
From: Michael S. Tsirkin @ 2015-11-30 8:53 UTC (permalink / raw)
To: linux-kernel; +Cc: virtio, kvm, netdev, virtualization, linux-api, David Gibson
This adds a new vring feature bit: when enabled, host and guest poll the
available/used ring directly instead of looking at the index field
first.
To guarantee it is possible to detect updates, the high bits (above
vring.num - 1) in the ring head ID value are modified to match the index
bits - these change on each wrap-around. Writer also XORs this with
0x8000 such that rings can be zero-initialized.
Reader is modified to ignore these high bits when looking
up descriptors.
The point is to reduce the number of cacheline misses
for both reads and writes.
I see a speedup of about 20% on a multithreaded micro-benchmark
(virtio-test), but regression of about 2% on a single-threaded one
(vring_bench). I think this has to do with the fact that
complete_multi_user is implemented suboptimally.
TODO:
investigate single-threaded regression
better name for a feature flag
split the patch to make it easier to review
look at more aggressive ring layout changes
write a spec patch
This is on top of the following patches in my tree:
virtio_ring: Shadow available ring flags & index
vhost: replace % with & on data path
tools/virtio: fix byteswap logic
tools/virtio: move list macro stubs
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.h | 3 +-
include/linux/vringh.h | 3 +
include/uapi/linux/virtio_ring.h | 3 +
drivers/vhost/vhost.c | 104 ++++++++++++++++++--------
drivers/vhost/vringh.c | 153 +++++++++++++++++++++++++++++++++------
drivers/virtio/virtio_ring.c | 40 ++++++++--
tools/virtio/virtio_test.c | 14 +++-
7 files changed, 255 insertions(+), 65 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d3f7674..aeeb15d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,8 @@ enum {
(1ULL << VIRTIO_RING_F_EVENT_IDX) |
(1ULL << VHOST_F_LOG_ALL) |
(1ULL << VIRTIO_F_ANY_LAYOUT) |
- (1ULL << VIRTIO_F_VERSION_1)
+ (1ULL << VIRTIO_F_VERSION_1) |
+ (1ULL << VIRTIO_RING_F_POLL)
};
static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index bc6c28d..13a9e3e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -40,6 +40,9 @@ struct vringh {
/* Can we get away with weak barriers? */
bool weak_barriers;
+ /* Poll ring directly */
+ bool poll;
+
/* Last available index we saw (ie. where we're up to). */
u16 last_avail_idx;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index c072959..bf3ca1d 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -62,6 +62,9 @@
* at the end of the used ring. Guest should ignore the used->flags field. */
#define VIRTIO_RING_F_EVENT_IDX 29
+/* Support ring polling */
+#define VIRTIO_RING_F_POLL 33
+
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 85f0f0a..cdbabf5 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1346,26 +1346,26 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Check it isn't doing very strange things with descriptor numbers. */
last_avail_idx = vq->last_avail_idx;
- if (unlikely(__get_user(avail_idx, &vq->avail->idx))) {
- vq_err(vq, "Failed to access avail idx at %p\n",
- &vq->avail->idx);
- return -EFAULT;
- }
- vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
-
- if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
- vq_err(vq, "Guest moved used index from %u to %u",
- last_avail_idx, vq->avail_idx);
- return -EFAULT;
- }
+ if (!vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ if (unlikely(__get_user(avail_idx, &vq->avail->idx))) {
+ vq_err(vq, "Failed to access avail idx at %p\n",
+ &vq->avail->idx);
+ return -EFAULT;
+ }
+ vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
- /* If there's nothing new since last we looked, return invalid. */
- if (vq->avail_idx == last_avail_idx)
- return vq->num;
+ if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
+ vq_err(vq, "Guest moved used index from %u to %u",
+ last_avail_idx, vq->avail_idx);
+ return -EFAULT;
+ }
- /* Only get avail ring entries after they have been exposed by guest. */
- smp_rmb();
+ /* If there's nothing new since last we looked, return invalid. */
+ if (vq->avail_idx == last_avail_idx)
+ return vq->num;
+ /* Only get avail ring entries after they have been exposed by guest. */
+ smp_rmb();
}
/* Grab the next descriptor number they're advertising, and increment
@@ -1380,11 +1380,22 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
head = vhost16_to_cpu(vq, ring_head);
- /* If their number is silly, that's an error. */
- if (unlikely(head >= vq->num)) {
- vq_err(vq, "Guest says index %u > %u is available",
- head, vq->num);
- return -EINVAL;
+ if (vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ /* If there's nothing new since last we looked, return invalid. */
+ if ((head ^ last_avail_idx ^ 0x8000) & ~(vq->num - 1))
+ return vq->num;
+
+ /* Only get avail ring entries after they have been exposed by guest. */
+ smp_rmb();
+
+ head &= vq->num - 1;
+ } else {
+ /* If their number is silly, that's an error. */
+ if (unlikely(head >= vq->num)) {
+ vq_err(vq, "Guest says index %u > %u is available",
+ head, vq->num);
+ return -EINVAL;
+ }
}
/* When we start there are none of either input nor output. */
@@ -1483,6 +1494,27 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
}
EXPORT_SYMBOL_GPL(vhost_add_used);
+static inline int __vhost_add_used_poll(struct vhost_virtqueue *vq,
+ struct vring_used_elem *heads,
+ struct vring_used_elem __user *used,
+ int idx)
+{
+ __virtio32 head = heads[0].id ^
+ cpu_to_vhost32(vq, ~(vq->num - 1) &
+ ((vq->last_used_idx + idx) ^ 0x8000));
+
+ if (__put_user(heads[0].len, &used->len)) {
+ vq_err(vq, "Failed to write used len");
+ return -EFAULT;
+ }
+ smp_wmb();
+ if (__put_user(head, &used->id)) {
+ vq_err(vq, "Failed to write used id");
+ return -EFAULT;
+ }
+ return 0;
+}
+
static int __vhost_add_used_n(struct vhost_virtqueue *vq,
struct vring_used_elem *heads,
unsigned count)
@@ -1493,18 +1525,28 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
- if (count == 1) {
- if (__put_user(heads[0].id, &used->id)) {
- vq_err(vq, "Failed to write used id");
+ if (vhost_has_feature(vq, VIRTIO_RING_F_POLL)) {
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < count; ++i)
+ ret |= __vhost_add_used_poll(vq, heads + i, used + i, i);
+ if (ret)
return -EFAULT;
- }
- if (__put_user(heads[0].len, &used->len)) {
- vq_err(vq, "Failed to write used len");
+ } else {
+ if (count == 1) {
+ if (__put_user(heads[0].id, &used->id)) {
+ vq_err(vq, "Failed to write used id");
+ return -EFAULT;
+ }
+ if (__put_user(heads[0].len, &used->len)) {
+ vq_err(vq, "Failed to write used len");
+ return -EFAULT;
+ }
+ } else if (__copy_to_user(used, heads, count * sizeof *used)) {
+ vq_err(vq, "Failed to write used");
return -EFAULT;
}
- } else if (__copy_to_user(used, heads, count * sizeof *used)) {
- vq_err(vq, "Failed to write used");
- return -EFAULT;
}
if (unlikely(vq->log_used)) {
/* Make sure data is seen before log. */
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 3bb02c6..d8aac79 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -36,18 +36,21 @@ static inline int __vringh_get_head(const struct vringh *vrh,
u16 avail_idx, i, head;
int err;
- err = getu16(vrh, &avail_idx, &vrh->vring.avail->idx);
- if (err) {
- vringh_bad("Failed to access avail idx at %p",
- &vrh->vring.avail->idx);
- return err;
- }
+ if (!vrh->poll) {
+ err = getu16(vrh, &avail_idx, &vrh->vring.avail->idx);
+ if (err) {
+ vringh_bad("Failed to access avail idx at %p",
+ &vrh->vring.avail->idx);
+ return err;
+ }
- if (*last_avail_idx == avail_idx)
- return vrh->vring.num;
+ if (*last_avail_idx == avail_idx)
+ return vrh->vring.num;
- /* Only get avail ring entries after they have been exposed by guest. */
- virtio_rmb(vrh->weak_barriers);
+ /* Only get avail ring entries after they have been exposed by guest. */
+ virtio_rmb(vrh->weak_barriers);
+
+ }
i = *last_avail_idx & (vrh->vring.num - 1);
@@ -58,10 +61,20 @@ static inline int __vringh_get_head(const struct vringh *vrh,
return err;
}
- if (head >= vrh->vring.num) {
- vringh_bad("Guest says index %u > %u is available",
- head, vrh->vring.num);
- return -EINVAL;
+ if (vrh->poll) {
+ if ((head ^ *last_avail_idx ^ 0x8000) & ~(vrh->vring.num - 1))
+ return vrh->vring.num;
+
+ /* Only get descriptor entries after they have been exposed by guest. */
+ virtio_rmb(vrh->weak_barriers);
+
+ head &= vrh->vring.num - 1;
+ } else {
+ if (head >= vrh->vring.num) {
+ vringh_bad("Guest says index %u > %u is available",
+ head, vrh->vring.num);
+ return -EINVAL;
+ }
}
(*last_avail_idx)++;
@@ -397,6 +410,57 @@ fail:
return err;
}
+static inline int __vringh_complete_poll(struct vringh *vrh,
+ int (*putu32)(const struct vringh *vrh,
+ __virtio32 *p, u32 val),
+ int (*putu16)(const struct vringh *vrh,
+ __virtio16 *p, u16 val),
+ __virtio32 head, __virtio32 len,
+ bool last)
+{
+ struct vring_used *used_ring;
+ int err;
+ u16 used_idx, off;
+
+ used_ring = vrh->vring.used;
+ used_idx = vrh->last_used_idx + vrh->completed;
+
+ off = used_idx & (vrh->vring.num - 1);
+
+ err = putu32(vrh, &used_ring->ring[off].len, len);
+ if (err) {
+ vringh_bad("Failed to write used length %u at %p",
+ off, &used_ring->ring[off]);
+ return err;
+ }
+
+ /* Make sure length is written before we update index. */
+ virtio_wmb(vrh->weak_barriers);
+
+ head ^= cpu_to_vringh32(vrh, (used_idx & ~(vrh->vring.num - 1)) ^ 0x8000);
+ err = putu32(vrh, &used_ring->ring[off].id, head);
+ if (err) {
+ vringh_bad("Failed to write used id %u at %p",
+ off, &used_ring->ring[off]);
+ return err;
+ }
+
+ if (last) {
+ /* Make sure buffer is written before we update index. */
+ virtio_wmb(vrh->weak_barriers);
+
+ err = putu16(vrh, &vrh->vring.used->idx, used_idx + 1);
+ if (err) {
+ vringh_bad("Failed to update used index at %p",
+ &vrh->vring.used->idx);
+ return err;
+ }
+ }
+
+ vrh->completed++;
+ return 0;
+}
+
static inline int __vringh_complete(struct vringh *vrh,
const struct vring_used_elem *used,
unsigned int num_used,
@@ -556,6 +620,13 @@ static inline int getu16_user(const struct vringh *vrh, u16 *val, const __virtio
return rc;
}
+static inline int putu32_user(const struct vringh *vrh, __virtio32 *p, u32 val)
+{
+ __virtio32 v = cpu_to_vringh32(vrh, val);
+ return put_user(v, (__force __virtio32 __user *)p);
+}
+
+
static inline int putu16_user(const struct vringh *vrh, __virtio16 *p, u16 val)
{
__virtio16 v = cpu_to_vringh16(vrh, val);
@@ -615,6 +686,7 @@ int vringh_init_user(struct vringh *vrh, u64 features,
vrh->little_endian = (features & (1ULL << VIRTIO_F_VERSION_1));
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
+ vrh->poll = (features & (1ULL << VIRTIO_RING_F_POLL));
vrh->weak_barriers = weak_barriers;
vrh->completed = 0;
vrh->last_avail_idx = 0;
@@ -752,11 +824,19 @@ EXPORT_SYMBOL(vringh_abandon_user);
*/
int vringh_complete_user(struct vringh *vrh, u16 head, u32 len)
{
- struct vring_used_elem used;
+ if (vrh->poll) {
+ return __vringh_complete_poll(vrh, putu32_user,
+ putu16_user,
+ cpu_to_vringh32(vrh, head),
+ cpu_to_vringh32(vrh, len),
+ true);
+ } else {
+ struct vring_used_elem used;
- used.id = cpu_to_vringh32(vrh, head);
- used.len = cpu_to_vringh32(vrh, len);
- return __vringh_complete(vrh, &used, 1, putu16_user, putused_user);
+ used.id = cpu_to_vringh32(vrh, head);
+ used.len = cpu_to_vringh32(vrh, len);
+ return __vringh_complete(vrh, &used, 1, putu16_user, putused_user);
+ }
}
EXPORT_SYMBOL(vringh_complete_user);
@@ -773,8 +853,21 @@ int vringh_complete_multi_user(struct vringh *vrh,
const struct vring_used_elem used[],
unsigned num_used)
{
- return __vringh_complete(vrh, used, num_used,
- putu16_user, putused_user);
+ if (vrh->poll) {
+ int i, r;
+
+ for (i = 0; i < num_used; ++i) {
+ r = __vringh_complete_poll(vrh, putu32_user, putu16_user,
+ used[i].id, used[i].len,
+ i == num_used - 1);
+ if (r)
+ return r;
+ }
+ return 0;
+ } else {
+ return __vringh_complete(vrh, used, num_used,
+ putu16_user, putused_user);
+ }
}
EXPORT_SYMBOL(vringh_complete_multi_user);
@@ -830,6 +923,12 @@ static inline int putu16_kern(const struct vringh *vrh, __virtio16 *p, u16 val)
return 0;
}
+static inline int putu32_kern(const struct vringh *vrh, __virtio32 *p, u32 val)
+{
+ ACCESS_ONCE(*p) = cpu_to_vringh32(vrh, val);
+ return 0;
+}
+
static inline int copydesc_kern(void *dst, const void *src, size_t len)
{
memcpy(dst, src, len);
@@ -876,6 +975,7 @@ int vringh_init_kern(struct vringh *vrh, u64 features,
vrh->little_endian = (features & (1ULL << VIRTIO_F_VERSION_1));
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
+ vrh->poll = (features & (1ULL << VIRTIO_RING_F_POLL));
vrh->weak_barriers = weak_barriers;
vrh->completed = 0;
vrh->last_avail_idx = 0;
@@ -987,12 +1087,17 @@ EXPORT_SYMBOL(vringh_abandon_kern);
*/
int vringh_complete_kern(struct vringh *vrh, u16 head, u32 len)
{
- struct vring_used_elem used;
+ if (vrh->poll) {
+ return __vringh_complete_poll(vrh, putu32_kern, putu16_kern,
+ head, len, true);
+ } else {
+ struct vring_used_elem used;
- used.id = cpu_to_vringh32(vrh, head);
- used.len = cpu_to_vringh32(vrh, len);
+ used.id = cpu_to_vringh32(vrh, head);
+ used.len = cpu_to_vringh32(vrh, len);
- return __vringh_complete(vrh, &used, 1, putu16_kern, putused_kern);
+ return __vringh_complete(vrh, &used, 1, putu16_kern, putused_kern);
+ }
}
EXPORT_SYMBOL(vringh_complete_kern);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 6262015..f25fd64 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -80,6 +80,9 @@ struct vring_virtqueue {
/* Last used index we've seen. */
u16 last_used_idx;
+ /* Poll ring instead of the index */
+ bool poll;
+
/* Last written value to avail->flags */
u16 avail_flags_shadow;
@@ -239,9 +242,14 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* Set token. */
vq->data[head] = data;
+ avail = vq->avail_idx_shadow & (vq->vring.num - 1);
+
+ if (vq->poll) {
+ virtio_wmb(vq->weak_barriers);
+ head ^= ((vq->avail_idx_shadow ^ 0x8000) & ~(vq->vring.num - 1));
+ }
/* Put entry in available array (but don't update avail->idx until they
* do sync). */
- avail = vq->avail_idx_shadow & (vq->vring.num - 1);
vq->vring.avail->ring[avail] = cpu_to_virtio16(_vq->vdev, head);
/* Descriptors and available array need to be set before we expose the
@@ -488,17 +496,32 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
return NULL;
}
- if (!more_used(vq)) {
- pr_debug("No more buffers in queue\n");
- END_USE(vq);
- return NULL;
- }
+ if (!vq->poll) {
+ if (!more_used(vq)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
- /* Only get used array entries after they have been exposed by host. */
- virtio_rmb(vq->weak_barriers);
+ /* Only get used array entries after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+ }
last_used = (vq->last_used_idx & (vq->vring.num - 1));
i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
+
+ if (vq->poll) {
+ if ((i ^ vq->last_used_idx ^ 0x8000) & ~(vq->vring.num - 1)) {
+ pr_debug("No more buffers in queue\n");
+ END_USE(vq);
+ return NULL;
+ }
+ i &= vq->vring.num - 1;
+
+ /* Only get used array entries after they have been exposed by host. */
+ virtio_rmb(vq->weak_barriers);
+ }
+
*len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
if (unlikely(i >= vq->vring.num)) {
@@ -764,6 +787,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
+ vq->poll = virtio_has_feature(vdev, VIRTIO_RING_F_POLL);
/* No callback? Tell other side not to bother us. */
if (!callback) {
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index e044589..585cd21 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -220,6 +220,14 @@ const struct option longopts[] = {
.val = 'e',
},
{
+ .name = "ring-poll",
+ .val = 'R',
+ },
+ {
+ .name = "no-ring-poll",
+ .val = 'r',
+ },
+ {
.name = "indirect",
.val = 'I',
},
@@ -261,7 +269,8 @@ int main(int argc, char **argv)
{
struct vdev_info dev;
unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
- (1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
+ (1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1) |
+ (1ULL << VIRTIO_RING_F_POLL);
int o;
bool delayed = false;
@@ -276,6 +285,9 @@ int main(int argc, char **argv)
case 'e':
features &= ~(1ULL << VIRTIO_RING_F_EVENT_IDX);
break;
+ case 'r':
+ features &= ~(1ULL << VIRTIO_RING_F_POLL);
+ break;
case 'h':
help();
goto done;
--
MST
^ permalink raw reply related
* Re: [PATCH] vhost: replace % with & on data path
From: Joe Perches @ 2015-11-30 8:42 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel; +Cc: netdev, kvm, virtualization
In-Reply-To: <1448872427-11623-1-git-send-email-mst@redhat.com>
On Mon, 2015-11-30 at 10:34 +0200, Michael S. Tsirkin wrote:
> We know vring num is a power of 2, so use &
> to mask the high bits.
[]
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
[]
> @@ -1366,10 +1366,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
> /* Only get avail ring entries after they have been exposed by guest. */
> smp_rmb();
>
> + }
?
^ permalink raw reply
* [PATCH] tools/virtio: move list macro stubs
From: Michael S. Tsirkin @ 2015-11-30 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: David Hildenbrand, virtualization
Makes them more generally available.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/linux/kernel.h | 6 ++++++
tools/virtio/linux/virtio.h | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h
index 0a3da64..4db7d56 100644
--- a/tools/virtio/linux/kernel.h
+++ b/tools/virtio/linux/kernel.h
@@ -110,4 +110,10 @@ static inline void free_page(unsigned long addr)
(void) (&_min1 == &_min2); \
_min1 < _min2 ? _min1 : _min2; })
+/* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
+#define list_add_tail(a, b) do {} while (0)
+#define list_del(a) do {} while (0)
+#define list_for_each_entry(a, b, c) while (0)
+/* end of stubs */
+
#endif /* KERNEL_H */
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index a3e0701..ee125e7 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -3,12 +3,6 @@
#include <linux/scatterlist.h>
#include <linux/kernel.h>
-/* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
-#define list_add_tail(a, b) do {} while (0)
-#define list_del(a) do {} while (0)
-#define list_for_each_entry(a, b, c) while (0)
-/* end of stubs */
-
struct virtio_device {
void *dev;
u64 features;
--
MST
^ permalink raw reply related
* [PATCH] vhost: replace % with & on data path
From: Michael S. Tsirkin @ 2015-11-30 8:34 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kvm, virtualization
We know vring num is a power of 2, so use &
to mask the high bits.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 080422f..85f0f0a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1366,10 +1366,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Only get avail ring entries after they have been exposed by guest. */
smp_rmb();
+ }
+
/* Grab the next descriptor number they're advertising, and increment
* the index we've seen. */
if (unlikely(__get_user(ring_head,
- &vq->avail->ring[last_avail_idx % vq->num]))) {
+ &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
vq_err(vq, "Failed to read head: idx %d address %p\n",
last_avail_idx,
&vq->avail->ring[last_avail_idx % vq->num]);
@@ -1489,7 +1491,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
u16 old, new;
int start;
- start = vq->last_used_idx % vq->num;
+ start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
if (count == 1) {
if (__put_user(heads[0].id, &used->id)) {
@@ -1531,7 +1533,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
{
int start, n, r;
- start = vq->last_used_idx % vq->num;
+ start = vq->last_used_idx & (vq->num - 1);
n = vq->num - start;
if (n < count) {
r = __vhost_add_used_n(vq, heads, n);
--
MST
^ permalink raw reply related
* [PATCH] tools/virtio: fix byteswap logic
From: Michael S. Tsirkin @ 2015-11-30 8:33 UTC (permalink / raw)
To: linux-kernel; +Cc: virtualization
commit cf561f0d2eb74574ad9985a2feab134267a9d298 ("virtio: introduce
virtio_is_little_endian() helper") changed byteswap logic to
skip feature bit checks for LE platforms, but didn't
update tools/virtio, so vring_bench started failing.
Update the copy under tools/virtio/ (TODO: find a way to avoid this code
duplication).
Cc: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/linux/virtio_config.h | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/virtio/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h
index 806d683..57a6964 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -40,33 +40,39 @@ static inline void __virtio_clear_bit(struct virtio_device *vdev,
#define virtio_has_feature(dev, feature) \
(__virtio_test_bit((dev), feature))
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+ return virtio_has_feature(vdev, VIRTIO_F_VERSION_1) ||
+ virtio_legacy_is_little_endian();
+}
+
+/* Memory accessors */
static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
{
- return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
{
- return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
}
static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
{
- return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
{
- return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
}
static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
{
- return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
{
- return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
}
-
--
MST
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox