* Re: [PATCH V3 3/3] vhost_net: basic polling support
From: Jason Wang @ 2016-02-29 5:17 UTC (permalink / raw)
To: Christian Borntraeger, kvm, mst, virtualization, netdev,
linux-kernel
Cc: yang.zhang.wz, RAPOPORT
In-Reply-To: <56D36D25.6070903@de.ibm.com>
On 02/29/2016 05:56 AM, Christian Borntraeger wrote:
> On 02/26/2016 09:42 AM, 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>
>> > ---
>> > drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
>> > drivers/vhost/vhost.c | 14 ++++++++
>> > drivers/vhost/vhost.h | 1 +
>> > include/uapi/linux/vhost.h | 6 ++++
>> > 4 files changed, 95 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> > index 9eda69e..c91af93 100644
>> > --- a/drivers/vhost/net.c
>> > +++ b/drivers/vhost/net.c
>> > @@ -287,6 +287,44 @@ 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);
>> > + int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
>> > + out_num, in_num, NULL, NULL);
>> > +
>> > + if (r == vq->num && vq->busyloop_timeout) {
>> > + preempt_disable();
>> > + endtime = busy_clock() + vq->busyloop_timeout;
>> > + while (vhost_can_busy_poll(vq->dev, endtime) &&
>> > + vhost_vq_avail_empty(vq->dev, vq))
>> > + cpu_relax();
> Can you use cpu_relax_lowlatency (which should be the same as cpu_relax for almost
> everybody but s390? cpu_relax (without low latency might give up the time slice
> when running under another hypervisor (like LPAR on s390), which might not be what
> we want here.
Ok, will do this in next version.
^ permalink raw reply
* Re: [PATCH V3 3/3] vhost_net: basic polling support
From: Jason Wang @ 2016-02-29 5:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: yang.zhang.wz, RAPOPORT, kvm, netdev, linux-kernel,
virtualization
In-Reply-To: <20160228140937.GA8855@redhat.com>
On 02/28/2016 10:09 PM, Michael S. Tsirkin wrote:
> On Fri, Feb 26, 2016 at 04:42:44PM +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>
> Looks good overall, but I still see one problem.
>
>> > ---
>> > drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
>> > drivers/vhost/vhost.c | 14 ++++++++
>> > drivers/vhost/vhost.h | 1 +
>> > include/uapi/linux/vhost.h | 6 ++++
>> > 4 files changed, 95 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> > index 9eda69e..c91af93 100644
>> > --- a/drivers/vhost/net.c
>> > +++ b/drivers/vhost/net.c
>> > @@ -287,6 +287,44 @@ 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();
> So I find it quite unfortunate that this still uses single_task_running.
> This means that for example a SCHED_IDLE task will prevent polling from
> becoming active, and that seems like a bug, or at least
> an undocumented feature :).
Yes, it may need more thoughts.
>
> Unfortunately this logic affects the behaviour as observed
> by userspace, so we can't merge it like this and tune
> afterwards, since otherwise mangement tools will start
> depending on this logic.
>
>
How about remove single_task_running() first here and optimize on top?
We probably need something like this to handle overcommitment.
^ permalink raw reply
* Re: [PATCH V3 3/3] vhost_net: basic polling support
From: Christian Borntraeger @ 2016-02-28 21:56 UTC (permalink / raw)
To: Jason Wang, kvm, mst, virtualization, netdev, linux-kernel
Cc: yang.zhang.wz, RAPOPORT
In-Reply-To: <1456476164-17242-4-git-send-email-jasowang@redhat.com>
On 02/26/2016 09:42 AM, 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>
> ---
> drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
> drivers/vhost/vhost.c | 14 ++++++++
> drivers/vhost/vhost.h | 1 +
> include/uapi/linux/vhost.h | 6 ++++
> 4 files changed, 95 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e..c91af93 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -287,6 +287,44 @@ 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);
> + int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> + out_num, in_num, NULL, NULL);
> +
> + if (r == vq->num && vq->busyloop_timeout) {
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> + while (vhost_can_busy_poll(vq->dev, endtime) &&
> + vhost_vq_avail_empty(vq->dev, vq))
> + cpu_relax();
Can you use cpu_relax_lowlatency (which should be the same as cpu_relax for almost
everybody but s390? cpu_relax (without low latency might give up the time slice
when running under another hypervisor (like LPAR on s390), which might not be what
we want here.
[...]
> +static int vhost_net_rx_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);
> + int len = peek_head_len(sk);
> +
> + if (!len && vq->busyloop_timeout) {
> + /* Both tx vq and rx socket were polled here */
> + 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_avail_empty(&net->dev, vq))
> + cpu_relax();
here as well.
^ permalink raw reply
* Re: [PATCH V3 3/3] vhost_net: basic polling support
From: Michael S. Tsirkin @ 2016-02-28 14:09 UTC (permalink / raw)
To: Jason Wang
Cc: yang.zhang.wz, RAPOPORT, kvm, netdev, linux-kernel,
virtualization
In-Reply-To: <1456476164-17242-4-git-send-email-jasowang@redhat.com>
On Fri, Feb 26, 2016 at 04:42:44PM +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>
Looks good overall, but I still see one problem.
> ---
> drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
> drivers/vhost/vhost.c | 14 ++++++++
> drivers/vhost/vhost.h | 1 +
> include/uapi/linux/vhost.h | 6 ++++
> 4 files changed, 95 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e..c91af93 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -287,6 +287,44 @@ 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();
So I find it quite unfortunate that this still uses single_task_running.
This means that for example a SCHED_IDLE task will prevent polling from
becoming active, and that seems like a bug, or at least
an undocumented feature :).
Unfortunately this logic affects the behaviour as observed
by userspace, so we can't merge it like this and tune
afterwards, since otherwise mangement tools will start
depending on this logic.
> +}
> +
> +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);
> + int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> + out_num, in_num, NULL, NULL);
> +
> + if (r == vq->num && vq->busyloop_timeout) {
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> + while (vhost_can_busy_poll(vq->dev, endtime) &&
> + vhost_vq_avail_empty(vq->dev, vq))
> + cpu_relax();
> + preempt_enable();
> + r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> + out_num, in_num, NULL, NULL);
> + }
> +
> + return r;
> +}
> +
> /* 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 +369,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 +472,38 @@ static int peek_head_len(struct sock *sk)
> return len;
> }
>
> +static int vhost_net_rx_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);
> + int len = peek_head_len(sk);
> +
> + if (!len && vq->busyloop_timeout) {
> + /* Both tx vq and rx socket were polled here */
> + 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_avail_empty(&net->dev, vq))
> + cpu_relax();
> +
> + preempt_enable();
> +
> + if (vhost_enable_notify(&net->dev, vq))
> + vhost_poll_queue(&vq->poll);
> + mutex_unlock(&vq->mutex);
> +
> + len = peek_head_len(sk);
> + }
> +
> + return len;
> +}
> +
> /* This is a multi-buffer version of vhost_get_desc, that works if
> * vq has read descriptors only.
> * @vq - the relevant virtqueue
> @@ -553,7 +622,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_rx_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 c4ff9f2..5abfce9 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)
> @@ -919,6 +920,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(&s, argp, sizeof(s))) {
> + r = -EFAULT;
> + break;
> + }
> + vq->busyloop_timeout = s.num;
> + break;
> + case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
> + s.index = idx;
> + s.num = vq->busyloop_timeout;
> + if (copy_to_user(argp, &s, sizeof(s)))
> + r = -EFAULT;
> + break;
> default:
> r = -ENOIOCTLCMD;
> }
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a7a43f0..9a02158 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..61a8777 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -126,6 +126,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 (in us) */
> +#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23, \
> + struct vhost_vring_state)
> +/* Get busy loop timeout (in us) */
> +#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24, \
> + struct vhost_vring_state)
>
> /* VHOST_NET specific defines */
>
> --
> 2.5.0
^ permalink raw reply
* Call For Papers - CISTI 2016 Workshops - Deadline March 15
From: Maria Lemos @ 2016-02-28 13:39 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 4561 bytes --]
-----------------------------------------------------------------------------
CISTI'2016 Workshops
Gran Canaria, Canary Islands, Spain
June 15 - 18, 2016
http://www.aisti.eu/cisti2016/index.php/es/xpto
-----------------------------------------------------------------------------
Introduction
------------
We are pleased to invite the academic and business community to submit their papers to the CISTI'2016 Workshops (11th Iberian Conference on Information Systems and Technologies), to be held in Gran Canaria, Canary Islands, Spain, between the 15th and 18th of June 2016.
Authors are encouraged to submit original scientific contributions such as state-of-art reviews and new research perspectives, groundbreaking ideas and/or architectures, solutions and/or applications for real problems, empirical and/or evaluation works, case studies, etc., in conformity with the themes and specific call-for paper of each Workshop (http://www.aisti.eu/cisti2016/index.php/es/xpto)
--------------------
CISTI 2016 Workshops
--------------------
CISTI will feature the following Workshops:
- 3rd Workshop on Applied Statistics and Data Analysis using Computer Science (ASDACS)
- 8th Workshop on Intelligent Systems and Applications (WISA)
- 2nd Workshop on Gaming, Simulation and Play (WGSP)
- 3rd Workshop on ICT for Auditing (WICTA)
- 3rd Workshop on Computational Biomedical Image Processing and Analysis (WCBIPA)
- Workshop on Quality Assessment and Systems in Health, Education and Services (QAS)
- Workshop on Internet, Businesses, Societies and Social Web (INSWS)
- Workshop on New Pedagogical Approaches with Technologies (NPAT)
- Workshop on Judicial Informatics/Informática Júridica (InJu)
- Workshop on Software Engineering for Scientific Applications / Ingeniería de Software para Aplicaciones Científicas (ISAC)
- Workshop on Software Product Families/Familia de Productos de Software (FaProS)
- Workshop on Business Management, Communication and Digital Social Network/Gestión de Empresa, Comunicación y Redes Sociales Digitales (WGECRSD)
----------------
Paper Submission
----------------
We invite prospective authors to submit their contributions, not yet published, reporting on either work under development or concluded projects about theoretical aspects or practical applications related to the topics of the workshop. Contributions can be submitted in one of two formats:
• Full papers - original, relevant and previously unpublished research results, related to any of the topics of the conference, with a maximum of 6 pages.
• Short papers - project reports, research in progress, with no more than 4 pages.
Papers will be presented and discussed in time slots of 20 (full papers) or 15 minutes (short papers). The contributions can be written either in English, Portuguese or Spanish, and should be submitted in PDF format, through the conference webpage (https://easychair.org/conferences/?conf=cistiworkshops2016).
All papers must follow the formatting rules of the Conference (http://www.aisti.eu/cisti2016/index.php/en/callfor-papers) with templates at: http://www.aisti.eu/cisti2015/templates.zip. Please note that, the contributions to be evaluated by the Scientific Committee should not include authors’ identification, e-mail or affiliation (blinded submission).
-----------
Publication
-----------
To ensure that the contribution (full paper or short paper) is published in the Proceedings, at least one of the authors must be fully registered, and the paper must comply with the suggested layout and page-limit. Additionally, all recommended modifications must be addressed by the authors before they submit the final version.
No more than one paper per registration will be published in the Conference Proceedings. Full papers will be published in both book and CD formats, with an ISBN. Short papers will be published in CD only, with an ISBN.
Published full and short papers will be sent to EI, IEEE XPlore, INSPEC, ISI, SCOPUS and Google Scholar. Detailed and up-to-date information may be found at CISTI 2016 website:http://www.aisti.eu/cisti2016.
---------------
Important Dates
---------------
(please note that some Workshops have different deadlines)
- Paper submission: March 15, 2016
- Author Notification: March 29, 2016
- Camera Ready: April 10, 2016
- Conference: June 15-18, 2016
We are counting on you. Please submit your contribution.
CISTI 2016 Chairs
http://www.aisti.eu/cisti2016/
[-- Attachment #2: 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 V3 0/3] basic busy polling support for vhost_net
From: Michael S. Tsirkin @ 2016-02-28 9:12 UTC (permalink / raw)
To: David Miller
Cc: yang.zhang.wz, RAPOPORT, kvm, netdev, linux-kernel,
virtualization
In-Reply-To: <20160226.114502.171815325392990783.davem@davemloft.net>
On Fri, Feb 26, 2016 at 11:45:02AM -0500, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Fri, 26 Feb 2016 16:42:41 +0800
>
> > 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.
>
> I'm assuming this will go through Michael's tree.
Definitely.
^ permalink raw reply
* Re: [PATCH V3 0/3] basic busy polling support for vhost_net
From: David Miller @ 2016-02-26 16:45 UTC (permalink / raw)
To: jasowang
Cc: yang.zhang.wz, RAPOPORT, kvm, mst, netdev, linux-kernel,
virtualization
In-Reply-To: <1456476164-17242-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 26 Feb 2016 16:42:41 +0800
> 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.
I'm assuming this will go through Michael's tree.
^ permalink raw reply
* [PATCH V3 3/3] vhost_net: basic polling support
From: Jason Wang @ 2016-02-26 8:42 UTC (permalink / raw)
To: kvm, mst, virtualization, netdev, linux-kernel; +Cc: yang.zhang.wz, RAPOPORT
In-Reply-To: <1456476164-17242-1-git-send-email-jasowang@redhat.com>
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>
---
drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
drivers/vhost/vhost.c | 14 ++++++++
drivers/vhost/vhost.h | 1 +
include/uapi/linux/vhost.h | 6 ++++
4 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e..c91af93 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -287,6 +287,44 @@ 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);
+ int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ out_num, in_num, NULL, NULL);
+
+ if (r == vq->num && vq->busyloop_timeout) {
+ preempt_disable();
+ endtime = busy_clock() + vq->busyloop_timeout;
+ while (vhost_can_busy_poll(vq->dev, endtime) &&
+ vhost_vq_avail_empty(vq->dev, vq))
+ cpu_relax();
+ preempt_enable();
+ r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ out_num, in_num, NULL, NULL);
+ }
+
+ return r;
+}
+
/* 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 +369,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 +472,38 @@ static int peek_head_len(struct sock *sk)
return len;
}
+static int vhost_net_rx_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);
+ int len = peek_head_len(sk);
+
+ if (!len && vq->busyloop_timeout) {
+ /* Both tx vq and rx socket were polled here */
+ 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_avail_empty(&net->dev, vq))
+ cpu_relax();
+
+ preempt_enable();
+
+ if (vhost_enable_notify(&net->dev, vq))
+ vhost_poll_queue(&vq->poll);
+ mutex_unlock(&vq->mutex);
+
+ len = peek_head_len(sk);
+ }
+
+ return len;
+}
+
/* This is a multi-buffer version of vhost_get_desc, that works if
* vq has read descriptors only.
* @vq - the relevant virtqueue
@@ -553,7 +622,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_rx_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 c4ff9f2..5abfce9 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)
@@ -919,6 +920,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(&s, argp, sizeof(s))) {
+ r = -EFAULT;
+ break;
+ }
+ vq->busyloop_timeout = s.num;
+ break;
+ case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
+ s.index = idx;
+ s.num = vq->busyloop_timeout;
+ if (copy_to_user(argp, &s, sizeof(s)))
+ r = -EFAULT;
+ break;
default:
r = -ENOIOCTLCMD;
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a7a43f0..9a02158 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..61a8777 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -126,6 +126,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 (in us) */
+#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23, \
+ struct vhost_vring_state)
+/* Get busy loop timeout (in us) */
+#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24, \
+ struct vhost_vring_state)
/* VHOST_NET specific defines */
--
2.5.0
^ permalink raw reply related
* [PATCH V3 2/3] vhost: introduce vhost_vq_avail_empty()
From: Jason Wang @ 2016-02-26 8:42 UTC (permalink / raw)
To: kvm, mst, virtualization, netdev, linux-kernel; +Cc: yang.zhang.wz, RAPOPORT
In-Reply-To: <1456476164-17242-1-git-send-email-jasowang@redhat.com>
This patch introduces a helper which will return true if we're sure
that the available ring is empty for a specific vq. When we're not
sure, e.g vq access failure, return false instead. This could be used
for busy polling code to exit the busy loop.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 14 ++++++++++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 90ac092..c4ff9f2 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1633,6 +1633,20 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
}
EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
+/* return true if we're sure that available ring is empty */
+bool vhost_vq_avail_empty(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_avail_empty);
+
/* 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..a7a43f0 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_avail_empty(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 V3 1/3] vhost: introduce vhost_has_work()
From: Jason Wang @ 2016-02-26 8:42 UTC (permalink / raw)
To: kvm, mst, virtualization, netdev, linux-kernel; +Cc: yang.zhang.wz, RAPOPORT
In-Reply-To: <1456476164-17242-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. This could be used for busy
polling code to exit the busy loop.
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 ad2146a..90ac092 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 V3 0/3] basic busy polling support for vhost_net
From: Jason Wang @ 2016-02-26 8:42 UTC (permalink / raw)
To: kvm, mst, virtualization, netdev, linux-kernel; +Cc: yang.zhang.wz, RAPOPORT
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 mlx4
- Guest with 8 vcpus and 1 queue
Results:
- TCP_RR was imporved obviously (at most 27%). And cpu utilizaton was
also improved in this case.
- No obvious differences in Guest RX throughput.
- Guest TX throughput was also improved.
TCP_RR:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
1/ 1/ +27%/ 0%/ +27%/ +27%/ +25%
1/ 50/ +2%/ +1%/ +2%/ +2%/ -4%
1/ 100/ +2%/ +1%/ +3%/ +3%/ -14%
1/ 200/ +2%/ +2%/ +5%/ +5%/ -15%
64/ 1/ +20%/ -13%/ +20%/ +20%/ +20%
64/ 50/ +17%/ +14%/ +16%/ +16%/ -11%
64/ 100/ +14%/ +12%/ +14%/ +14%/ -35%
64/ 200/ +16%/ +15%/ +9%/ +9%/ -28%
256/ 1/ +19%/ -6%/ +19%/ +19%/ +18%
256/ 50/ +18%/ +15%/ +16%/ +16%/ +3%
256/ 100/ +11%/ +9%/ +12%/ +12%/ -1%
256/ 200/ +5%/ +8%/ +4%/ +4%/ +64%
512/ 1/ +20%/ 0%/ +20%/ +20%/ -2%
512/ 50/ +12%/ +10%/ +12%/ +12%/ +8%
512/ 100/ +11%/ +7%/ +10%/ +10%/ -5%
512/ 200/ +3%/ +2%/ +3%/ +3%/ -5%
1024/ 1/ +19%/ -2%/ +19%/ +19%/ +18%
1024/ 50/ +13%/ +10%/ +12%/ +12%/ 0%
1024/ 100/ +9%/ +8%/ +8%/ +8%/ -16%
1024/ 200/ +3%/ +4%/ +3%/ +3%/ -14%
Guest RX:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
64/ 1/ -12%/ -10%/ +2%/ +1%/ +42%
64/ 4/ -3%/ -5%/ +2%/ -1%/ 0%
64/ 8/ -1%/ -5%/ -1%/ -2%/ 0%
512/ 1/ +5%/ -13%/ +6%/ +9%/ +17%
512/ 4/ -3%/ -9%/ +6%/ +4%/ -14%
512/ 8/ -2%/ -7%/ 0%/ 0%/ -1%
1024/ 1/ +18%/ +31%/ -12%/ -11%/ -31%
1024/ 4/ 0%/ -9%/ -1%/ -6%/ -7%
1024/ 8/ -3%/ -8%/ -2%/ -4%/ 0%
2048/ 1/ 0%/ -1%/ 0%/ -4%/ +5%
2048/ 4/ 0%/ +2%/ 0%/ 0%/ 0%
2048/ 8/ 0%/ -6%/ 0%/ -3%/ -1%
4096/ 1/ -1%/ +2%/ -14%/ -5%/ +8%
4096/ 4/ 0%/ +1%/ 0%/ +1%/ -1%
4096/ 8/ -1%/ -1%/ -2%/ -2%/ -3%
16384/ 1/ 0%/ 0%/ +4%/ +5%/ 0%
16384/ 4/ 0%/ +5%/ +7%/ +9%/ 0%
16384/ 8/ +1%/ +1%/ +3%/ +3%/ +2%
65535/ 1/ 0%/ +12%/ -1%/ +2%/ -2%
65535/ 4/ 0%/ 0%/ -2%/ -2%/ +2%
65535/ 8/ -1%/ -1%/ -4%/ -4%/ 0%
Guest TX:
size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
64/ 1/ -16%/ -21%/ -2%/ -12%/ +1%
64/ 4/ -6%/ -2%/ -1%/ +6%/ -7%
64/ 8/ +4%/ +4%/ -2%/ +1%/ +30%
512/ 1/ -32%/ -33%/ -11%/ +62%/ +314%
512/ 4/ +30%/ +20%/ -22%/ -17%/ -14%
512/ 8/ +24%/ +12%/ -21%/ -10%/ -6%
1024/ 1/ +1%/ -7%/ +2%/ +51%/ +75%
1024/ 4/ +10%/ +9%/ -11%/ -19%/ -10%
1024/ 8/ +13%/ +7%/ -11%/ -13%/ -12%
2048/ 1/ +17%/ 0%/ +1%/ +35%/ +78%
2048/ 4/ +15%/ +14%/ -17%/ -24%/ -15%
2048/ 8/ +11%/ +9%/ -15%/ -20%/ -12%
4096/ 1/ +3%/ -7%/ 0%/ +21%/ +48%
4096/ 4/ +3%/ +4%/ -9%/ -19%/ +41%
4096/ 8/ +15%/ +13%/ -33%/ -28%/ -15%
16384/ 1/ +5%/ -8%/ -4%/ -10%/ +323%
16384/ 4/ +13%/ +5%/ -15%/ -11%/ +147%
16384/ 8/ +8%/ +6%/ -25%/ -27%/ -31%
65535/ 1/ +8%/ 0%/ +5%/ 0%/ +45%
65535/ 4/ +10%/ +1%/ +7%/ -8%/ +151%
65535/ 8/ +5%/ 0%/ +1%/ -16%/ -29%
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 V2:
- rename vhost_vq_more_avail() to vhost_vq_avail_empty(). And return
false we __get_user() fails.
- do not bother premmptions/timers for good path.
- use vhost_vring_state as ioctl parameter instead of reinveting a new
one.
- add the unit of timeout (us) to the comment of new added ioctls
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_avail_empty()
vhost_net: basic polling support
drivers/vhost/net.c | 79 +++++++++++++++++++++++++++++++++++++++++++---
drivers/vhost/vhost.c | 35 ++++++++++++++++++++
drivers/vhost/vhost.h | 3 ++
include/uapi/linux/vhost.h | 6 ++++
4 files changed, 118 insertions(+), 5 deletions(-)
--
2.5.0
^ permalink raw reply
* Re: [PATCH v2] virtio_blk: VIRTIO_BLK_F_WCE->VIRTIO_BLK_F_FLUSH
From: Paolo Bonzini @ 2016-02-25 8:38 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel; +Cc: virtualization
In-Reply-To: <1456351070-10204-1-git-send-email-mst@redhat.com>
On 24/02/2016 22:58, Michael S. Tsirkin wrote:
> Latest virtio spec says the feature bit name is VIRTIO_BLK_F_FLUSH,
> VIRTIO_BLK_F_WCE is the legacy name. virtio blk header says exactly the
> reverse - fix that and update driver code to match.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Changs from v1:
> comments updated
>
> include/uapi/linux/virtio_blk.h | 6 +++---
> drivers/block/virtio_blk.c | 11 ++++++++---
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index 19c66fc..9ebe4d9 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -43,11 +43,11 @@
> #ifndef VIRTIO_BLK_NO_LEGACY
> #define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? */
> #define VIRTIO_BLK_F_SCSI 7 /* Supports scsi command passthru */
> -#define VIRTIO_BLK_F_WCE 9 /* Writeback mode enabled after reset */
> +#define VIRTIO_BLK_F_FLUSH 9 /* Flush command supported */
> #define VIRTIO_BLK_F_CONFIG_WCE 11 /* Writeback mode available in config */
> #ifndef __KERNEL__
> -/* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
> -#define VIRTIO_BLK_F_FLUSH VIRTIO_BLK_F_WCE
> +/* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */
> +#define VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH
> #endif
> #endif /* !VIRTIO_BLK_NO_LEGACY */
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 6ca3549..28cff0d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -477,8 +477,13 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
> err = virtio_cread_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE,
> struct virtio_blk_config, wce,
> &writeback);
> +
> + /*
> + * If WCE is not configurable and flush is not available,
> + * assume no writeback cache is in use.
> + */
> if (err)
> - writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
> + writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH);
>
> return writeback;
> }
> @@ -833,14 +838,14 @@ static const struct virtio_device_id id_table[] = {
> static unsigned int features_legacy[] = {
> VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
> VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
> - VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> + VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> VIRTIO_BLK_F_MQ,
> }
> ;
> static unsigned int features[] = {
> VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
> VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
> - VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> + VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> VIRTIO_BLK_F_MQ,
> };
>
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply
* [PATCH v2] virtio_blk: VIRTIO_BLK_F_WCE->VIRTIO_BLK_F_FLUSH
From: Michael S. Tsirkin @ 2016-02-24 21:58 UTC (permalink / raw)
To: linux-kernel; +Cc: Paolo Bonzini, virtualization
Latest virtio spec says the feature bit name is VIRTIO_BLK_F_FLUSH,
VIRTIO_BLK_F_WCE is the legacy name. virtio blk header says exactly the
reverse - fix that and update driver code to match.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changs from v1:
comments updated
include/uapi/linux/virtio_blk.h | 6 +++---
drivers/block/virtio_blk.c | 11 ++++++++---
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 19c66fc..9ebe4d9 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -43,11 +43,11 @@
#ifndef VIRTIO_BLK_NO_LEGACY
#define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? */
#define VIRTIO_BLK_F_SCSI 7 /* Supports scsi command passthru */
-#define VIRTIO_BLK_F_WCE 9 /* Writeback mode enabled after reset */
+#define VIRTIO_BLK_F_FLUSH 9 /* Flush command supported */
#define VIRTIO_BLK_F_CONFIG_WCE 11 /* Writeback mode available in config */
#ifndef __KERNEL__
-/* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
-#define VIRTIO_BLK_F_FLUSH VIRTIO_BLK_F_WCE
+/* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */
+#define VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH
#endif
#endif /* !VIRTIO_BLK_NO_LEGACY */
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6ca3549..28cff0d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -477,8 +477,13 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE,
struct virtio_blk_config, wce,
&writeback);
+
+ /*
+ * If WCE is not configurable and flush is not available,
+ * assume no writeback cache is in use.
+ */
if (err)
- writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
+ writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH);
return writeback;
}
@@ -833,14 +838,14 @@ static const struct virtio_device_id id_table[] = {
static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
- VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
}
;
static unsigned int features[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
- VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
};
--
MST
^ permalink raw reply related
* Re: [PATCH] virtio_blk: VIRTIO_BLK_F_WCE->VIRTIO_BLK_F_FLUSH
From: Paolo Bonzini @ 2016-02-24 15:16 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel; +Cc: virtualization
In-Reply-To: <1456326631-8478-1-git-send-email-mst@redhat.com>
On 24/02/2016 16:11, Michael S. Tsirkin wrote:
> Latest virtio spec says the feature bit name is VIRTIO_BLK_F_FLUSH,
> VIRTIO_BLK_F_WCE is the legacy name. virtio blk header says exactly the
> reverse - fix that and update driver code to match.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> I'm inclined to merge it for the current kernel -
> safe and avoids confusion.
Since you are at it, also change this:
> +#define VIRTIO_BLK_F_FLUSH 9 /* Writeback mode enabled after reset */
to /* Flush command supported */
> #define VIRTIO_BLK_F_CONFIG_WCE 11 /* Writeback mode available in config */
> #ifndef __KERNEL__
> -/* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
> -#define VIRTIO_BLK_F_FLUSH VIRTIO_BLK_F_WCE
> +/* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */
> +#define VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH
> #endif
> #endif /* !VIRTIO_BLK_NO_LEGACY */
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 6ca3549..dbaebb0 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -478,7 +478,7 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
> struct virtio_blk_config, wce,
> &writeback);
and add a comment here:
/*
* If WCE is not configurable and flush is not available,
* assume no writeback cache is in use.
*/
> if (err)
> - writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
> + writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH);
>
> return writeback;
> }
> @@ -833,14 +833,14 @@ static const struct virtio_device_id id_table[] = {
> static unsigned int features_legacy[] = {
> VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
> VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
> - VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> + VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> VIRTIO_BLK_F_MQ,
> }
> ;
> static unsigned int features[] = {
> VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
> VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
> - VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> + VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
> VIRTIO_BLK_F_MQ,
> };
>
>
^ permalink raw reply
* [PATCH] virtio_blk: VIRTIO_BLK_F_WCE->VIRTIO_BLK_F_FLUSH
From: Michael S. Tsirkin @ 2016-02-24 15:11 UTC (permalink / raw)
To: linux-kernel; +Cc: pbonzini, virtualization
Latest virtio spec says the feature bit name is VIRTIO_BLK_F_FLUSH,
VIRTIO_BLK_F_WCE is the legacy name. virtio blk header says exactly the
reverse - fix that and update driver code to match.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I'm inclined to merge it for the current kernel -
safe and avoids confusion.
include/uapi/linux/virtio_blk.h | 6 +++---
drivers/block/virtio_blk.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 19c66fc..c3f98df 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -43,11 +43,11 @@
#ifndef VIRTIO_BLK_NO_LEGACY
#define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? */
#define VIRTIO_BLK_F_SCSI 7 /* Supports scsi command passthru */
-#define VIRTIO_BLK_F_WCE 9 /* Writeback mode enabled after reset */
+#define VIRTIO_BLK_F_FLUSH 9 /* Writeback mode enabled after reset */
#define VIRTIO_BLK_F_CONFIG_WCE 11 /* Writeback mode available in config */
#ifndef __KERNEL__
-/* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
-#define VIRTIO_BLK_F_FLUSH VIRTIO_BLK_F_WCE
+/* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */
+#define VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH
#endif
#endif /* !VIRTIO_BLK_NO_LEGACY */
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6ca3549..dbaebb0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -478,7 +478,7 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
struct virtio_blk_config, wce,
&writeback);
if (err)
- writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
+ writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH);
return writeback;
}
@@ -833,14 +833,14 @@ static const struct virtio_device_id id_table[] = {
static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
- VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
}
;
static unsigned int features[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
- VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
};
--
MST
^ permalink raw reply related
* Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics
From: Denis V. Lunev @ 2016-02-23 16:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Igor Redko, Andrew Morton, linux-kernel, virtualization
In-Reply-To: <20160223174952-mutt-send-email-mst@redhat.com>
On 02/23/2016 06:53 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 23, 2016 at 06:26:47PM +0300, Denis V. Lunev wrote:
>> On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
>>> On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
>>>> From: Igor Redko <redkoi@virtuozzo.com>
>>>>
>>>> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
>>>> statistics protocol, corresponding to 'Available' in /proc/meminfo.
>>>>
>>>> It indicates to the hypervisor how big the balloon can be inflated
>>>> without pushing the guest system to swap.
>>>>
>>>> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
>>>> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>> Oops - I missed the fact that this affects host/guest ABI.
>>>
>>> Can you please submit ABI update proposal to virtio tc?
>>> Spec patch would be even better.
>>>
>>> This is important to ensure there are no conflicts
>>> with other features being developed in parallel.
>> hmmm
>>
>> From my point of view ABI remains untouched.
> Anything exposed by guest to host is ABI.
> Once we add stuff there, we never can remove it
> as some host might rely on it.
>
>> The guest can send any amount of <tag>;<value>
>> pairs and unknown tags are properly ignored
>> by the host.
>>
>> That is why I think that this change is safe.
> What happens if someone uses the tag you
> used for VIRTIO_BALLOON_S_AVAIL, for some
> other purpose?
> Any tools using VIRTIO_BALLOON_S_AVAIL will be confused.
actually this constant resides in QEMU only,
values are reported above using JSON and
string tags.
> Really, it's not hard to get a tag number from virtio TC,
> so please just do this.
>
ok. So do you propose to negotiate maximum allowed
tag to send at the driver start time?
we will have to guard this exchange with proper flag
in feature space then. This could be done but from my
point of view this looks like serious over-complication.
Do we have somebody who can judge?
Den
^ permalink raw reply
* Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics
From: Michael S. Tsirkin @ 2016-02-23 15:53 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Igor Redko, Andrew Morton, linux-kernel, virtualization
In-Reply-To: <56CC7A37.2030807@openvz.org>
On Tue, Feb 23, 2016 at 06:26:47PM +0300, Denis V. Lunev wrote:
> On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
> >On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
> >>From: Igor Redko <redkoi@virtuozzo.com>
> >>
> >>Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
> >>statistics protocol, corresponding to 'Available' in /proc/meminfo.
> >>
> >>It indicates to the hypervisor how big the balloon can be inflated
> >>without pushing the guest system to swap.
> >>
> >>Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> >>Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
> >>Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>CC: Michael S. Tsirkin <mst@redhat.com>
> >>CC: Andrew Morton <akpm@linux-foundation.org>
> >Oops - I missed the fact that this affects host/guest ABI.
> >
> >Can you please submit ABI update proposal to virtio tc?
> >Spec patch would be even better.
> >
> >This is important to ensure there are no conflicts
> >with other features being developed in parallel.
>
> hmmm
>
> From my point of view ABI remains untouched.
Anything exposed by guest to host is ABI.
Once we add stuff there, we never can remove it
as some host might rely on it.
> The guest can send any amount of <tag>;<value>
> pairs and unknown tags are properly ignored
> by the host.
>
> That is why I think that this change is safe.
What happens if someone uses the tag you
used for VIRTIO_BALLOON_S_AVAIL, for some
other purpose?
Any tools using VIRTIO_BALLOON_S_AVAIL will be confused.
Really, it's not hard to get a tag number from virtio TC,
so please just do this.
--
MST
^ permalink raw reply
* Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics
From: Denis V. Lunev @ 2016-02-23 15:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Igor Redko, Andrew Morton, linux-kernel, virtualization
In-Reply-To: <20160223170828-mutt-send-email-mst@redhat.com>
On 02/23/2016 06:10 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
>> From: Igor Redko <redkoi@virtuozzo.com>
>>
>> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
>> statistics protocol, corresponding to 'Available' in /proc/meminfo.
>>
>> It indicates to the hypervisor how big the balloon can be inflated
>> without pushing the guest system to swap.
>>
>> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
>> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> CC: Andrew Morton <akpm@linux-foundation.org>
> Oops - I missed the fact that this affects host/guest ABI.
>
> Can you please submit ABI update proposal to virtio tc?
> Spec patch would be even better.
>
> This is important to ensure there are no conflicts
> with other features being developed in parallel.
hmmm
From my point of view ABI remains untouched.
The guest can send any amount of <tag>;<value>
pairs and unknown tags are properly ignored
by the host.
That is why I think that this change is safe.
^ permalink raw reply
* Re: [PATCH 2/2] virtio_balloon: export 'available' memory to balloon statistics
From: Michael S. Tsirkin @ 2016-02-23 15:10 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: Igor Redko, Andrew Morton, linux-kernel, virtualization
In-Reply-To: <1455637852-7323-3-git-send-email-den@openvz.org>
On Tue, Feb 16, 2016 at 06:50:52PM +0300, Denis V. Lunev wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
>
> Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
> statistics protocol, corresponding to 'Available' in /proc/meminfo.
>
> It indicates to the hypervisor how big the balloon can be inflated
> without pushing the guest system to swap.
>
> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
Oops - I missed the fact that this affects host/guest ABI.
Can you please submit ABI update proposal to virtio tc?
Spec patch would be even better.
This is important to ensure there are no conflicts
with other features being developed in parallel.
> ---
> drivers/virtio/virtio_balloon.c | 6 ++++++
> include/uapi/linux/virtio_balloon.h | 3 ++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 0c3691f..f2b77de 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -30,6 +30,7 @@
> #include <linux/balloon_compaction.h>
> #include <linux/oom.h>
> #include <linux/wait.h>
> +#include <linux/mm.h>
>
> /*
> * Balloon device works in 4K page units. So each page is pointed to by
> @@ -229,10 +230,13 @@ static void update_balloon_stats(struct virtio_balloon *vb)
> unsigned long events[NR_VM_EVENT_ITEMS];
> struct sysinfo i;
> int idx = 0;
> + long available;
>
> all_vm_events(events);
> si_meminfo(&i);
>
> + available = si_mem_available();
> +
> update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
> pages_to_bytes(events[PSWPIN]));
> update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT,
> @@ -243,6 +247,8 @@ static void update_balloon_stats(struct virtio_balloon *vb)
> pages_to_bytes(i.freeram));
> update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT,
> pages_to_bytes(i.totalram));
> + update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
> + pages_to_bytes(available));
> }
>
> /*
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index d7f1cbc..343d7dd 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -51,7 +51,8 @@ struct virtio_balloon_config {
> #define VIRTIO_BALLOON_S_MINFLT 3 /* Number of minor faults */
> #define VIRTIO_BALLOON_S_MEMFREE 4 /* Total amount of free memory */
> #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> -#define VIRTIO_BALLOON_S_NR 6
> +#define VIRTIO_BALLOON_S_AVAIL 6 /* Available memory as in /proc */
> +#define VIRTIO_BALLOON_S_NR 7
>
> /*
> * Memory statistics structure.
> --
> 2.5.0
^ permalink raw reply
* Re: [PATCH] virtio_net: switch to build_skb for mrg_rxbuf
From: Michael S. Tsirkin @ 2016-02-21 18:41 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, virtualization
In-Reply-To: <1456079998-5240-1-git-send-email-mst@redhat.com>
On Sun, Feb 21, 2016 at 08:40:59PM +0200, Michael S. Tsirkin wrote:
> For small packets data copy was observed to
> take up about 15% CPU time. Switch to build_skb
> and avoid the copy when using mergeable rx buffers.
>
> As a bonus, medium-size skbs that fit in a page will be
> completely linear.
>
> Of course, we now need to lower the lower bound on packet size,
> to make sure a sane number of skbs fits in rx socket buffer.
> By how much? I don't know yet.
>
> It might also be useful to prefetch the packet buffer since
> net stack will likely use it soon.
>
> Lightly tested, in particular, I didn't yet test what this
> actually does to performance - sending this out for early
> feedback/flames.
>
> TODO: it appears that Linux won't handle correctly the case of first
> buffer being very small (or consisting exclusively of virtio header).
> This is already the case for current code, so don't bother
> testing yet.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Oops. This should have been "RFC PATCH".
Please don't merge it yet, it's just a preview.
> ---
> drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..20f8dda 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -347,6 +347,26 @@ err:
> return NULL;
> }
>
> +#define VNET_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
> +#define VNET_SKB_BUG (VNET_SKB_PAD < sizeof(struct virtio_net_hdr_mrg_rxbuf))
> +#define VNET_SKB_LEN(len) ((len) - sizeof(struct virtio_net_hdr_mrg_rxbuf))
> +#define VNET_SKB_OFF VNET_SKB_LEN(VNET_SKB_PAD)
> +
> +static struct sk_buff *vnet_build_skb(struct virtnet_info *vi,
> + void *buf,
> + unsigned int len, unsigned int truesize)
> +{
> + struct sk_buff *skb = build_skb(buf, truesize);
> +
> + if (!skb)
> + return NULL;
> +
> + skb_reserve(skb, VNET_SKB_PAD);
> + skb_put(skb, VNET_SKB_LEN(len));
> +
> + return skb;
> +}
> +
> static struct sk_buff *receive_mergeable(struct net_device *dev,
> struct virtnet_info *vi,
> struct receive_queue *rq,
> @@ -354,14 +374,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> unsigned int len)
> {
> void *buf = mergeable_ctx_to_buf_address(ctx);
> - struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
> + struct virtio_net_hdr_mrg_rxbuf *hdr = buf + VNET_SKB_OFF;
> u16 num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> struct page *page = virt_to_head_page(buf);
> - int offset = buf - page_address(page);
> unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
> + int offset;
>
> - struct sk_buff *head_skb = page_to_skb(vi, rq, page, offset, len,
> - truesize);
> + struct sk_buff *head_skb = vnet_build_skb(vi, buf, len, truesize);
> struct sk_buff *curr_skb = head_skb;
>
> if (unlikely(!curr_skb))
> @@ -606,11 +625,14 @@ static int add_recvbuf_big(struct virtnet_info *vi, struct receive_queue *rq,
>
> static unsigned int get_mergeable_buf_len(struct ewma_pkt_len *avg_pkt_len)
> {
> - const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> + unsigned int hdr;
> unsigned int len;
>
> - len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
> - GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
> + hdr = ALIGN(VNET_SKB_PAD + sizeof(struct skb_shared_info),
> + MERGEABLE_BUFFER_ALIGN);
> +
> + len = hdr + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
> + 500 /* TODO */, PAGE_SIZE - hdr);
> return ALIGN(len, MERGEABLE_BUFFER_ALIGN);
> }
>
> @@ -626,7 +648,10 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> if (unlikely(!skb_page_frag_refill(len, alloc_frag, gfp)))
> return -ENOMEM;
>
> - buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> + BUILD_BUG_ON(VNET_SKB_BUG);
> +
> + buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset +
> + VNET_SKB_OFF;
> ctx = mergeable_buf_to_ctx(buf, len);
> get_page(alloc_frag->page);
> alloc_frag->offset += len;
> @@ -641,7 +666,8 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> alloc_frag->offset += hole;
> }
>
> - sg_init_one(rq->sg, buf, len);
> + sg_init_one(rq->sg, buf,
> + len - VNET_SKB_OFF - sizeof(struct skb_shared_info));
> err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, (void *)ctx, gfp);
> if (err < 0)
> put_page(virt_to_head_page(buf));
> --
> MST
^ permalink raw reply
* [PATCH] virtio_net: switch to build_skb for mrg_rxbuf
From: Michael S. Tsirkin @ 2016-02-21 18:40 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, virtualization
For small packets data copy was observed to
take up about 15% CPU time. Switch to build_skb
and avoid the copy when using mergeable rx buffers.
As a bonus, medium-size skbs that fit in a page will be
completely linear.
Of course, we now need to lower the lower bound on packet size,
to make sure a sane number of skbs fits in rx socket buffer.
By how much? I don't know yet.
It might also be useful to prefetch the packet buffer since
net stack will likely use it soon.
Lightly tested, in particular, I didn't yet test what this
actually does to performance - sending this out for early
feedback/flames.
TODO: it appears that Linux won't handle correctly the case of first
buffer being very small (or consisting exclusively of virtio header).
This is already the case for current code, so don't bother
testing yet.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 44 +++++++++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 767ab11..20f8dda 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -347,6 +347,26 @@ err:
return NULL;
}
+#define VNET_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
+#define VNET_SKB_BUG (VNET_SKB_PAD < sizeof(struct virtio_net_hdr_mrg_rxbuf))
+#define VNET_SKB_LEN(len) ((len) - sizeof(struct virtio_net_hdr_mrg_rxbuf))
+#define VNET_SKB_OFF VNET_SKB_LEN(VNET_SKB_PAD)
+
+static struct sk_buff *vnet_build_skb(struct virtnet_info *vi,
+ void *buf,
+ unsigned int len, unsigned int truesize)
+{
+ struct sk_buff *skb = build_skb(buf, truesize);
+
+ if (!skb)
+ return NULL;
+
+ skb_reserve(skb, VNET_SKB_PAD);
+ skb_put(skb, VNET_SKB_LEN(len));
+
+ return skb;
+}
+
static struct sk_buff *receive_mergeable(struct net_device *dev,
struct virtnet_info *vi,
struct receive_queue *rq,
@@ -354,14 +374,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
unsigned int len)
{
void *buf = mergeable_ctx_to_buf_address(ctx);
- struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
+ struct virtio_net_hdr_mrg_rxbuf *hdr = buf + VNET_SKB_OFF;
u16 num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
struct page *page = virt_to_head_page(buf);
- int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
+ int offset;
- struct sk_buff *head_skb = page_to_skb(vi, rq, page, offset, len,
- truesize);
+ struct sk_buff *head_skb = vnet_build_skb(vi, buf, len, truesize);
struct sk_buff *curr_skb = head_skb;
if (unlikely(!curr_skb))
@@ -606,11 +625,14 @@ static int add_recvbuf_big(struct virtnet_info *vi, struct receive_queue *rq,
static unsigned int get_mergeable_buf_len(struct ewma_pkt_len *avg_pkt_len)
{
- const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+ unsigned int hdr;
unsigned int len;
- len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
- GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
+ hdr = ALIGN(VNET_SKB_PAD + sizeof(struct skb_shared_info),
+ MERGEABLE_BUFFER_ALIGN);
+
+ len = hdr + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
+ 500 /* TODO */, PAGE_SIZE - hdr);
return ALIGN(len, MERGEABLE_BUFFER_ALIGN);
}
@@ -626,7 +648,10 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
if (unlikely(!skb_page_frag_refill(len, alloc_frag, gfp)))
return -ENOMEM;
- buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
+ BUILD_BUG_ON(VNET_SKB_BUG);
+
+ buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset +
+ VNET_SKB_OFF;
ctx = mergeable_buf_to_ctx(buf, len);
get_page(alloc_frag->page);
alloc_frag->offset += len;
@@ -641,7 +666,8 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
alloc_frag->offset += hole;
}
- sg_init_one(rq->sg, buf, len);
+ sg_init_one(rq->sg, buf,
+ len - VNET_SKB_OFF - sizeof(struct skb_shared_info));
err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, (void *)ctx, gfp);
if (err < 0)
put_page(virt_to_head_page(buf));
--
MST
^ permalink raw reply related
* Re: [PATCH 09/16] drm/shmobile: removed optional dummy crtc mode_fixup function.
From: Laurent Pinchart @ 2016-02-17 15:24 UTC (permalink / raw)
To: Carlos Palminha
Cc: nicolas.pitre, boris.brezillon, jianwei.wang.chn, airlied,
daniel.vetter, alison.wang, patrik.r.jakobsson, virtualization,
linux-renesas-soc, jani.nikula, dri-devel, benjamin.gaignard,
vincent.abriou, sudipm.mukherjee
In-Reply-To: <c1652b5b8d84d857f8036a37d1d3e6123970e1a0.1455630967.git.palminha@synopsys.com>
Hi Carlos,
Thank you for the patch.
On Tuesday 16 February 2016 14:18:40 Carlos Palminha wrote:
> This patch set nukes all the dummy crtc mode_fixup implementations.
> (made on top of Daniel topic/drm-misc branch)
>
> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index 27342fd..88643ab 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
> @@ -359,13 +359,6 @@ static void shmob_drm_crtc_dpms(struct drm_crtc *crtc,
> int mode) scrtc->dpms = mode;
> }
>
> -static bool shmob_drm_crtc_mode_fixup(struct drm_crtc *crtc,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - return true;
> -}
> -
> static void shmob_drm_crtc_mode_prepare(struct drm_crtc *crtc)
> {
> shmob_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
> @@ -431,7 +424,6 @@ static int shmob_drm_crtc_mode_set_base(struct drm_crtc
> *crtc, int x, int y,
>
> static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
> .dpms = shmob_drm_crtc_dpms,
> - .mode_fixup = shmob_drm_crtc_mode_fixup,
> .prepare = shmob_drm_crtc_mode_prepare,
> .commit = shmob_drm_crtc_mode_commit,
> .mode_set = shmob_drm_crtc_mode_set,
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 07/16] drm/omapdrm: removed optional dummy crtc mode_fixup function.
From: Laurent Pinchart @ 2016-02-17 15:24 UTC (permalink / raw)
To: Carlos Palminha
Cc: nicolas.pitre, boris.brezillon, jianwei.wang.chn, airlied,
daniel.vetter, alison.wang, patrik.r.jakobsson, virtualization,
linux-renesas-soc, jani.nikula, dri-devel, benjamin.gaignard,
vincent.abriou, sudipm.mukherjee
In-Reply-To: <101f043d5fa747291c09ae765bac4d55c6e39988.1455630967.git.palminha@synopsys.com>
Hi Carlos,
Thank you for the patch.
On Tuesday 16 February 2016 14:18:12 Carlos Palminha wrote:
> This patch set nukes all the dummy crtc mode_fixup implementations.
> (made on top of Daniel topic/drm-misc branch)
>
> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index d38fcbc..483acdb 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -330,13 +330,6 @@ static void omap_crtc_destroy(struct drm_crtc *crtc)
> kfree(omap_crtc);
> }
>
> -static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - return true;
> -}
> -
> static void omap_crtc_enable(struct drm_crtc *crtc)
> {
> struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> @@ -451,7 +444,6 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
> };
>
> static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
> - .mode_fixup = omap_crtc_mode_fixup,
> .mode_set_nofb = omap_crtc_mode_set_nofb,
> .disable = omap_crtc_disable,
> .enable = omap_crtc_enable,
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 06/16] drm/rcar-du: removed optional dummy crtc mode_fixup function.
From: Laurent Pinchart @ 2016-02-17 15:23 UTC (permalink / raw)
To: Carlos Palminha
Cc: nicolas.pitre, boris.brezillon, jianwei.wang.chn, airlied,
daniel.vetter, alison.wang, patrik.r.jakobsson, virtualization,
linux-renesas-soc, jani.nikula, dri-devel, benjamin.gaignard,
vincent.abriou, sudipm.mukherjee
In-Reply-To: <becab4ff666eca77162e5cd978087f2d3fb3e308.1455630967.git.palminha@synopsys.com>
Hi Carlos,
Thank you for the patch.
On Tuesday 16 February 2016 14:18:00 Carlos Palminha wrote:
> This patch set nukes all the dummy crtc mode_fixup implementations.
> (made on top of Daniel topic/drm-misc branch)
>
> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 4ec80ae..627abc80 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -468,14 +468,6 @@ static void rcar_du_crtc_disable(struct drm_crtc *crtc)
> rcrtc->outputs = 0;
> }
>
> -static bool rcar_du_crtc_mode_fixup(struct drm_crtc *crtc,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - /* TODO Fixup modes */
> - return true;
> -}
> -
> static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> @@ -502,7 +494,6 @@ static void rcar_du_crtc_atomic_flush(struct drm_crtc
> *crtc, }
>
> static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
> - .mode_fixup = rcar_du_crtc_mode_fixup,
> .disable = rcar_du_crtc_disable,
> .enable = rcar_du_crtc_enable,
> .atomic_begin = rcar_du_crtc_atomic_begin,
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 11/16] drm/atmel-hldcd: removed optional dummy crtc mode_fixup function.
From: Daniel Vetter @ 2016-02-17 13:08 UTC (permalink / raw)
To: Carlos Palminha
Cc: nicolas.pitre, Boris Brezillon, benjamin.gaignard,
jianwei.wang.chn, airlied, daniel.vetter, alison.wang,
patrik.r.jakobsson, virtualization, linux-renesas-soc,
jani.nikula, dri-devel, Daniel Vetter, vincent.abriou,
sudipm.mukherjee, laurent.pinchart
In-Reply-To: <56C43734.9050708@synopsys.com>
On Wed, Feb 17, 2016 at 09:02:44AM +0000, Carlos Palminha wrote:
> Thanks Boris.
>
> @Daniel, do you want me to resend this patch or will you fix it directly in mode-fixup git branch?
I can fix the typos, but I'm meh on the whitespace change ;-) Imo that
doesn't need a resend.
-Daniel
>
> On 16-02-2016 16:58, Boris Brezillon wrote:
> > On Tue, 16 Feb 2016 14:19:06 +0000
> > Carlos Palminha <CARLOS.PALMINHA@synopsys.com> wrote:
> >
> >> This patch set nukes all the dummy crtc mode_fixup implementations.
> >> (made on top of Daniel topic/drm-misc branch)
> >
> > There's 2 typos in the subject line (s/hldcd/hlcdc/ and
> > s/removed/remove/), and you're removing an empty line after
> > atmel_hlcdc_crtc_create() definition (which is correct, but I'm not
> > sure it should be part of the same patch).
> > Otherwise it looks good to me.
> > Once you've fixed those 2 things, you can add my
> >
> > Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> >
> >>
> >> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
> >> ---
> >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 9 ---------
> >> 1 file changed, 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> index 9863291..58c4f78 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> @@ -121,13 +121,6 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
> >> cfg);
> >> }
> >>
> >> -static bool atmel_hlcdc_crtc_mode_fixup(struct drm_crtc *crtc,
> >> - const struct drm_display_mode *mode,
> >> - struct drm_display_mode *adjusted_mode)
> >> -{
> >> - return true;
> >> -}
> >> -
> >> static void atmel_hlcdc_crtc_disable(struct drm_crtc *c)
> >> {
> >> struct drm_device *dev = c->dev;
> >> @@ -261,7 +254,6 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
> >> }
> >>
> >> static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
> >> - .mode_fixup = atmel_hlcdc_crtc_mode_fixup,
> >> .mode_set = drm_helper_crtc_mode_set,
> >> .mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
> >> .mode_set_base = drm_helper_crtc_mode_set_base,
> >> @@ -349,4 +341,3 @@ fail:
> >> atmel_hlcdc_crtc_destroy(&crtc->base);
> >> return ret;
> >> }
> >> -
> >
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
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