* Re: [Pv-drivers] [PATCH 08/12] VMCI: resource object implementation.
From: Greg KH @ 2012-10-30 15:51 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030052016.GF32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 10:20:16PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 29, 2012 at 07:29:05PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote:
> > > +static struct vmci_resource *vmci_resource_lookup(struct vmci_handle handle)
> > > +{
> > > + struct vmci_resource *r, *resource = NULL;
> > > + struct hlist_node *node;
> > > + unsigned int idx = vmci_resource_hash(handle);
> > > +
> > > + BUG_ON(VMCI_HANDLE_EQUAL(handle, VMCI_INVALID_HANDLE));
> >
> > You just crashed a machine, with no chance for recovery. Not a good
> > idea. Never a good idea. Customers just lost data, and now they are
> > mad. Make sure you at least print out your email address so they know
> > who to blame :)
> >
> > Seriously, never BUG() in a driver, warn, sure, but this just looks like
> > a debugging assert(). Please remove all of these, they are sprinkled
> > all over the driver code here, I'm only responding to one of them here.
> >
> > Even better yet, properly handle the error and keep on going, that's
> > what the rest of the kernel does. Or should :)
>
> For public APIs it certainly makes sense to check and handle erroneous input;
It's not "public", it's an in-kernel api. See the static up there? :)
> internally it often makes sense to simply enforce invariants, because if
> we managed to get into that state that we consider impossible we can't really
> trust anything.
Then error out, don't crash the box. Again, this really looks like an
ASSERT() you are trying to catch, which you know how well we like those
in kernel code...
> FWIW:
> [dtor@dtor-ws kernel]$ grep -r BUG_ON . | wc -l
> 11269
I'm not saying that those are acceptable either, I just don't want to
add any more to the kernel.
greg k-h
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 05/12] VMCI: event handling implementation.
From: Greg KH @ 2012-10-30 15:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030050152.GE32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 10:01:52PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 29, 2012 at 07:26:05PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
> > > +static void event_signal_destroy(struct kref *kref)
> > > +{
> > > + struct vmci_subscription *entry =
> > > + container_of(kref, struct vmci_subscription, kref);
> > > +
> > > + complete(&entry->done);
> > > +}
> >
> > Didn't you just leak memory here? What frees the structure up?
>
> event_unregister_subscription() waits for that completion and frees the
> structure. We want event_unregister_subscription() to wait until all
> fired callbacks completed before unregister is complete.
So all calls to this can just sit and spin waiting for others to clean
up? Odd, but ok.
greg k-h
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 05/12] VMCI: event handling implementation.
From: Greg KH @ 2012-10-30 15:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030045821.GD32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 09:58:21PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 29, 2012 at 07:24:46PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
> > > +/*
> > > + * Releases the given VMCISubscription.
> > > + * Fires the destroy event if the reference count has gone to zero.
> > > + */
> > > +static void event_release(struct vmci_subscription *entry)
> > > +{
> > > + kref_put(&entry->kref, event_signal_destroy);
> > > +}
> >
> > Same question as before with the kref_put() call, what is handling the
> > locking here? It looks like a race to me.
>
> The reference is taken only if event is on the list (which managed by
> RCU and a mutex), so it is not possible to go from 0->1 for that
> refcount.
Ok, as long as you have audited this.
greg k-h
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 04/12] VMCI: device driver implementaton.
From: Greg KH @ 2012-10-30 15:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030041522.GC32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 09:15:22PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 29, 2012 at 07:23:47PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:04:15PM -0700, George Zhang wrote:
> > > +static int __init vmci_core_init(void)
> > > +{
> > > + int result;
> > > +
> > > + result = vmci_ctx_init();
> > > + if (result < VMCI_SUCCESS) {
> > > + pr_err("Failed to initialize VMCIContext (result=%d).\n",
> > > + result);
> >
> > If you are going to use pr_* functions, it's usually a good idea to
> > define pr_fmt in a consistant way so that it shows up the same across
> > all of your .c files. See other drivers for examples of how to do this
> > properly.
>
> pr_fmt() is defined in drivers/misc/vmw_vmci/vmci_common_int.h
I think that is too late in the #include file chain, shouldn't it be set
before any #include files are called?
> > > + return -EINVAL;
> > > + }
> > > +
> > > + result = vmci_datagram_init();
> > > + if (result < VMCI_SUCCESS) {
> > > + pr_err("Failed to initialize VMCIDatagram (result=%d).\n",
> > > + result);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + result = vmci_event_init();
> > > + if (result < VMCI_SUCCESS) {
> > > + pr_err("Failed to initialize VMCIEvent (result=%d).\n", result);
> > > + return -EINVAL;
> > > + }
> >
> > You don't free and unwind things properly if things go wrong here, why
> > not?
>
> We do, the above calls do not need to be cleaned up.
Ah, that's helpful :)
> > > + return 0;
> > > +}
> > > +
> > > +static void __exit vmci_core_exit(void)
> > > +{
> > > + vmci_event_exit();
> > > +}
> > > +
> > > +static int __init vmci_drv_init(void)
> > > +{
> > > + int error;
> > > +
> > > + error = vmci_core_init();
> > > + if (error)
> > > + return error;
> > > +
> > > + if (!vmci_disable_guest) {
> > > + error = vmci_guest_init();
> > > + if (error) {
> > > + pr_warn("Failed to initialize guest personality (err=%d).\n",
> > > + error);
> > > + } else {
> > > + vmci_guest_personality_initialized = true;
> > > + pr_info("Guest personality initialized and is %s\n",
> > > + vmci_guest_code_active() ?
> > > + "active" : "inactive");
> > > + }
> > > + }
> > > +
> > > + if (!vmci_disable_host) {
> > > + error = vmci_host_init();
> > > + if (error) {
> > > + pr_warn("Unable to initialize host personality (err=%d).\n",
> > > + error);
> > > + } else {
> > > + vmci_host_personality_initialized = true;
> > > + pr_info("Initialized host personality\n");
> > > + }
> > > + }
> > > +
> > > + if (!vmci_guest_personality_initialized &&
> > > + !vmci_host_personality_initialized) {
> > > + vmci_core_exit();
> > > + return -ENODEV;
> > > + }
> > > +
> > > + pr_debug("Module is initialized\n");
> >
> > Really? You need this message still?
>
> These are debug messages so do not show up in logs by default.
I know, but even if you do enable debugging, are such things really
needed? They look like left-over tracing function calls to me.
thanks,
greg k-h
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming
From: Greg KH @ 2012-10-30 15:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030040744.GB32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 09:07:44PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
>
> On Mon, Oct 29, 2012 at 07:19:38PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:03:28PM -0700, George Zhang wrote:
> > > drivers/misc/Kconfig | 1
> > > drivers/misc/Makefile | 2
> > > drivers/misc/vmw_vmci/Kconfig | 16
> > > drivers/misc/vmw_vmci/Makefile | 43
> >
> > Meta comment here, why drivers/misc/? The other hypervisor
> > infrastructures all have their own directory under drivers/ Should we
> > be moving everything to drivers/hyperv/ somehow?
>
> drivers/hyperv is not the best name for obvious reasons...
Sorry, yes :)
> I think that even if we had a special directory for vmci having network
> drivers in Dave's realm and pvscsi in James's is best option, so the new
> directory would contain vmci and the balloon driver (vsock will go into
> net/). Given that balloon is already in drivers/misc it looked like
> obvious place for VMCI as well.
I agree that the individual drivers should go in the subsystem area,
it's this "hypervisor bus core" type code that I'm questioning. Right
now every hypervisor is putting that logic in a different place in the
kernel, having some consistency here would be nice.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next 8/8] vhost-net: reduce vq polling on tx zerocopy
From: Vlad Yasevich @ 2012-10-30 15:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, Ian Campbell, kvm, netdev, linux-kernel,
virtualization, Eric Dumazet, Andrew Morton, David S. Miller
In-Reply-To: <3e57e2cde71a4270a8fef395c31f5f0e8a130c28.1351524502.git.mst@redhat.com>
On 10/29/2012 11:49 AM, Michael S. Tsirkin wrote:
> It seems that to avoid deadlocks it is enough to poll vq before
> we are going to use the last buffer. This should be faster than
> c70aa540c7a9f67add11ad3161096fb95233aa2e.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 8e9de79..3967f82 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -197,8 +197,16 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, int status)
> {
> struct vhost_ubuf_ref *ubufs = ubuf->ctx;
> struct vhost_virtqueue *vq = ubufs->vq;
> -
> - vhost_poll_queue(&vq->poll);
> + int cnt = atomic_read(&ubufs->kref.refcount);
> +
> + /*
> + * Trigger polling thread if guest stopped submitting new buffers:
> + * in this case, the refcount after decrement will eventually reach 1
> + * so here it is 2.
> + * We also trigger polling periodically after each 16 packets.
> + */
> + if (cnt <= 2 || !(cnt % 16))
Why 16? Does it make sense to make it configurable?
-vlad
> + vhost_poll_queue(&vq->poll);
> /* set len to mark this desc buffers done DMA */
> vq->heads[ubuf->desc].len = status ?
> VHOST_DMA_FAILED_LEN : VHOST_DMA_DONE_LEN;
>
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 01/12] VMCI: context implementation.
From: Greg KH @ 2012-10-30 15:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030040139.GA32055@dtor-ws.eng.vmware.com>
On Mon, Oct 29, 2012 at 09:01:40PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
>
> On Mon, Oct 29, 2012 at 07:10:58PM -0700, Greg KH wrote:
> > On Mon, Oct 29, 2012 at 06:03:42PM -0700, George Zhang wrote:
> > > +/*
> > > + * Releases the VMCI context. If this is the last reference to
> > > + * the context it will be deallocated. A context is created with
> > > + * a reference count of one, and on destroy, it is removed from
> > > + * the context list before its reference count is
> > > + * decremented. Thus, if we reach zero, we are sure that nobody
> > > + * else are about to increment it (they need the entry in the
> > > + * context list for that). This function musn't be called with a
> > > + * lock held.
> > > + */
> > > +void vmci_ctx_release(struct vmci_ctx *context)
> > > +{
> > > + ASSERT(context);
> > > + kref_put(&context->kref, ctx_free_ctx);
> > > +}
> > > +
> >
> > Hm, are you _sure_ you should be calling this without a lock held?
> > That's usually kref-101, you MUST hold a lock when calling put,
> > otherwise you can race a kref_get() call, and all hell can break loose.
> >
> > Because of this, some saner people (like Al Viro), have suggested that I
> > force the kref_put() and kref_get() calls pass in a spinlock just to
> > enforce this.
> >
> > So, tell me what I'm missing here, and why you put the comment here
> > saying that it really is supposed to be called without a lock held? How
> > is that safe?
> >
>
> Contexts are created/registered in vmci_ctx_init_ctx() and unregistered in
> vmci_ctx_release_ctx() and these operations are protected by
> ctx_list.lock spinlock. Context lookup (vmci_ctx_get) also uses spinlock
> to traverse list of registered contexts and then grabs reference to the
> [valid] context. The use of kref_put() without additional locking in
> vmci_ctx_release() is fine as there is no chance of another thread
> bumping count from 0 to 1.
As I didn't see all callers of this holding that spinlock, it was
confusing. You should put this type of description somewhere so that
other reviewers don't have the same questions.
> I believe the comment should actually read that the function should not
> be called from atomic contexts.
That might be nice to document, but could it ever happen?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next 2/8] skb: api to report errors for zero copy skbs
From: Vlad Yasevich @ 2012-10-30 15:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, Ian Campbell, kvm, netdev, linux-kernel,
virtualization, Eric Dumazet, Andrew Morton, David S. Miller
In-Reply-To: <ee85ad5053a929c90e0347b6ba33778d96690e9b.1351524501.git.mst@redhat.com>
On 10/29/2012 11:49 AM, Michael S. Tsirkin wrote:
> Orphaning frags for zero copy skbs needs to allocate data in atomic
> context so is has a chance to fail. If it does we currently discard
> the skb which is safe, but we don't report anything to the caller,
> so it can not recover by e.g. disabling zero copy.
>
> Add an API to free skb reporting such errors: this is used
> by tun in case orphaning frags fails.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/linux/skbuff.h | 1 +
> net/core/skbuff.c | 19 +++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 8bac11b..0644432 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -568,6 +568,7 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
> }
>
> extern void kfree_skb(struct sk_buff *skb);
> +extern void skb_tx_error(struct sk_buff *skb, int err);
> extern void consume_skb(struct sk_buff *skb);
> extern void __kfree_skb(struct sk_buff *skb);
> extern struct kmem_cache *skbuff_head_cache;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index eb31f6e..ad99c64 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -635,6 +635,25 @@ void kfree_skb(struct sk_buff *skb)
> EXPORT_SYMBOL(kfree_skb);
>
> /**
> + * kfree_skb_on_error - report an sk_buff xmit error
> + * @skb: buffer that triggered an error
> + *
> + * Report xmit error if a device callback is tracking this skb.
> + */
Nit: Comment doesn't match new function.
-vlad
> +void skb_tx_error(struct sk_buff *skb, int err)
> +{
> + if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
> + struct ubuf_info *uarg;
> +
> + uarg = skb_shinfo(skb)->destructor_arg;
> + if (uarg->callback)
> + uarg->callback(uarg, err);
> + skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
> + }
> +}
> +EXPORT_SYMBOL(skb_tx_error);
> +
> +/**
> * consume_skb - free an skbuff
> * @skb: buffer to free
> *
>
^ permalink raw reply
* Re: [PATCH] Add a page cache-backed balloon device driver.
From: Michael S. Tsirkin @ 2012-10-30 15:29 UTC (permalink / raw)
To: Mike Waychison
Cc: Frank Swiderski, Andrea Arcangeli, Rik van Riel, kvm,
linux-kernel@vger.kernel.org, virtualization
In-Reply-To: <20120910211018.GB21484@redhat.com>
On Tue, Sep 11, 2012 at 12:10:18AM +0300, Michael S. Tsirkin wrote:
> > On the plus side, having an exit taken here on each page turns out to
> > be relatively cheap, as the vmexit from the page fault should be
> > faster to process as it is fully handled within the host kernel.
> >
> > Perhaps some combination of both methods is required? I'm not sure :\
>
> Perhaps some benchmarking is in order :)
> Can you try telling host, potentially MADV_WILL_NEED
> in that case like qemu does, then run your proprietary test
> and see if things work well enough?
Ping. Had a chance to try that?
--
MST
^ permalink raw reply
* [rfc net-next v6 3/3] virtio-net: change the number of queues through ethtool
From: Jason Wang @ 2012-10-30 10:03 UTC (permalink / raw)
To: mst, davem, virtualization, linux-kernel, netdev, rusty, krkumar2; +Cc: kvm
In-Reply-To: <1351591403-23065-1-git-send-email-jasowang@redhat.com>
This patch implement the {set|get}_channels method of ethool to allow user to
change the number of queues dymaically when the device is running. This would
let the user to tune the device for specific applications.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8cc43e5..66fc129 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1616,10 +1616,53 @@ static struct virtio_driver virtio_net_driver = {
#endif
};
+/* TODO: Eliminate OOO packets during switching */
+static int virtnet_set_channels(struct net_device *dev,
+ struct ethtool_channels *channels)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ u16 queue_pairs = channels->combined_count;
+
+ /* Only two modes were support currently */
+ if (queue_pairs == 0)
+ return -EINVAL;
+ if (queue_pairs != vi->total_queue_pairs - 1 && queue_pairs != 1)
+ return -EINVAL;
+
+ vi->num_queue_pairs = queue_pairs > 1 ? queue_pairs + 1 : 1;
+ BUG_ON(virtnet_set_queues(vi));
+
+ netif_set_real_num_tx_queues(dev, vi->num_queue_pairs);
+ netif_set_real_num_rx_queues(dev, vi->num_queue_pairs);
+
+ return 0;
+}
+
+static void virtnet_get_channels(struct net_device *dev,
+ struct ethtool_channels *channels)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ if (vi->total_queue_pairs != 1) {
+ channels->combined_count = vi->num_queue_pairs;
+ channels->max_combined = vi->total_queue_pairs - 1;
+ } else {
+ channels->combined_count = 1;
+ channels->max_combined = 1;
+ }
+
+ channels->max_other = 0;
+ channels->rx_count = 0;
+ channels->tx_count = 0;
+ channels->other_count = 0;
+}
+
static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
+ .set_channels = virtnet_set_channels,
+ .get_channels = virtnet_get_channels,
};
static int __init init(void)
--
1.7.1
^ permalink raw reply related
* [rfc net-next v6 2/3] virtio_net: multiqueue support
From: Jason Wang @ 2012-10-30 10:03 UTC (permalink / raw)
To: mst, davem, virtualization, linux-kernel, netdev, rusty, krkumar2; +Cc: kvm
In-Reply-To: <1351591403-23065-1-git-send-email-jasowang@redhat.com>
This addes multiqueue support to virtio_net driver. There's two mode supported:
single queue pair mode and multiple queue pairs mode. An obvious difference
compared with a physical mq card is that virtio-net reserve first two virtqueues
when it is working in multiqueue mode, this is used for implementing adaptive
mode switching in the future. The virtqueues that were in both mq and sq mode
were initialized and only one queue pair (single queue mode) were used at
default. User could use ethtool -L to switch to multiqueue mode withe the next
patch.
In multiple queue modes, the driver expects the number of queue paris is equal
to the number of vcpus. In the case, each vcpu would have its private virtqueue
pairs through:
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
To make sure a single vcpu is handling the packets from the same flow, driver
would let the device (usually tuntap) to use the 'rx follows tx' packet steering
rules when working in multiqueue mode. In this mode, the device would select the
rxq based on the last queue where the packet of the flow were sent.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 747 ++++++++++++++++++++++++++++-----------
include/uapi/linux/virtio_net.h | 18 +
2 files changed, 550 insertions(+), 215 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cbf8b06..8cc43e5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,7 @@
#include <linux/scatterlist.h>
#include <linux/if_vlan.h>
#include <linux/slab.h>
+#include <linux/interrupt.h>
static int napi_weight = 128;
module_param(napi_weight, int, 0444);
@@ -51,43 +52,70 @@ struct virtnet_stats {
u64 rx_packets;
};
-struct virtnet_info {
- struct virtio_device *vdev;
- struct virtqueue *rvq, *svq, *cvq;
- struct net_device *dev;
+/* Internal representation of a send virtqueue */
+struct send_queue {
+ /* Virtqueue associated with this send _queue */
+ struct virtqueue *vq;
+
+ /* TX: fragments + linear part + virtio header */
+ struct scatterlist sg[MAX_SKB_FRAGS + 2];
+};
+
+/* Internal representation of a receive virtqueue */
+struct receive_queue {
+ /* Virtqueue associated with this receive_queue */
+ struct virtqueue *vq;
+
+ /* Back pointer to the virtnet_info */
+ struct virtnet_info *vi;
+
struct napi_struct napi;
- unsigned int status;
/* Number of input buffers, and max we've ever had. */
unsigned int num, max;
+ /* Work struct for refilling if we run low on memory. */
+ struct delayed_work refill;
+
+ /* Chain pages by the private ptr. */
+ struct page *pages;
+
+ /* RX: fragments + linear part + virtio header */
+ struct scatterlist sg[MAX_SKB_FRAGS + 2];
+};
+
+struct virtnet_info {
+ u16 num_queue_pairs; /* # of RX/TX vq pairs */
+ u16 total_queue_pairs;
+
+ struct send_queue *sq;
+ struct receive_queue *rq;
+ struct virtqueue *cvq;
+
+ struct virtio_device *vdev;
+ struct net_device *dev;
+ unsigned int status;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
/* Active statistics */
struct virtnet_stats __percpu *stats;
- /* Work struct for refilling if we run low on memory. */
- struct delayed_work refill;
-
/* Work struct for config space updates */
struct work_struct config_work;
/* Lock for config space updates */
struct mutex config_lock;
-
- /* Chain pages by the private ptr. */
- struct page *pages;
-
- /* fragments + linear part + virtio header */
- struct scatterlist rx_sg[MAX_SKB_FRAGS + 2];
- struct scatterlist tx_sg[MAX_SKB_FRAGS + 2];
};
struct skb_vnet_hdr {
@@ -108,6 +136,30 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+static inline int vq2txq(struct virtqueue *vq)
+{
+ int index = virtqueue_get_queue_index(vq);
+ return index == 1 ? 0 : (index - 3) / 2;
+}
+
+static inline int txq2vq(int txq)
+{
+ return txq ? 2 * txq + 3 : 1;
+}
+
+static inline int vq2rxq(struct virtqueue *vq)
+{
+ int index = virtqueue_get_queue_index(vq);
+ return index ? (index - 2) / 2 : 0;
+}
+
+static inline int rxq2vq(int rxq)
+{
+ return rxq ? 2 * rxq + 2 : 0;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb->cb;
@@ -117,22 +169,22 @@ static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
* private is used to chain pages for big packets, put the whole
* most recent used list in the beginning for reuse
*/
-static void give_pages(struct virtnet_info *vi, struct page *page)
+static void give_pages(struct receive_queue *rq, struct page *page)
{
struct page *end;
/* Find end of list, sew whole thing into vi->pages. */
for (end = page; end->private; end = (struct page *)end->private);
- end->private = (unsigned long)vi->pages;
- vi->pages = page;
+ end->private = (unsigned long)rq->pages;
+ rq->pages = page;
}
-static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
+static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
{
- struct page *p = vi->pages;
+ struct page *p = rq->pages;
if (p) {
- vi->pages = (struct page *)p->private;
+ rq->pages = (struct page *)p->private;
/* clear private here, it is used to chain pages */
p->private = 0;
} else
@@ -140,15 +192,15 @@ static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
return p;
}
-static void skb_xmit_done(struct virtqueue *svq)
+static void skb_xmit_done(struct virtqueue *vq)
{
- struct virtnet_info *vi = svq->vdev->priv;
+ struct virtnet_info *vi = vq->vdev->priv;
/* Suppress further interrupts. */
- virtqueue_disable_cb(svq);
+ virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi->dev);
+ netif_wake_subqueue(vi->dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -167,9 +219,10 @@ static void set_skb_frag(struct sk_buff *skb, struct page *page,
}
/* Called from bottom half context */
-static struct sk_buff *page_to_skb(struct virtnet_info *vi,
+static struct sk_buff *page_to_skb(struct receive_queue *rq,
struct page *page, unsigned int len)
{
+ struct virtnet_info *vi = rq->vi;
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
unsigned int copy, hdr_len, offset;
@@ -225,12 +278,12 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
}
if (page)
- give_pages(vi, page);
+ give_pages(rq, page);
return skb;
}
-static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
+static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb)
{
struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
struct page *page;
@@ -244,7 +297,7 @@ static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
skb->dev->stats.rx_length_errors++;
return -EINVAL;
}
- page = virtqueue_get_buf(vi->rvq, &len);
+ page = virtqueue_get_buf(rq->vq, &len);
if (!page) {
pr_debug("%s: rx error: %d buffers missing\n",
skb->dev->name, hdr->mhdr.num_buffers);
@@ -257,13 +310,14 @@ static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
set_skb_frag(skb, page, 0, &len);
- --vi->num;
+ --rq->num;
}
return 0;
}
-static void receive_buf(struct net_device *dev, void *buf, unsigned int len)
+static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
{
+ struct net_device *dev = rq->vi->dev;
struct virtnet_info *vi = netdev_priv(dev);
struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
struct sk_buff *skb;
@@ -274,7 +328,7 @@ static void receive_buf(struct net_device *dev, void *buf, unsigned int len)
pr_debug("%s: short packet %i\n", dev->name, len);
dev->stats.rx_length_errors++;
if (vi->mergeable_rx_bufs || vi->big_packets)
- give_pages(vi, buf);
+ give_pages(rq, buf);
else
dev_kfree_skb(buf);
return;
@@ -286,14 +340,14 @@ static void receive_buf(struct net_device *dev, void *buf, unsigned int len)
skb_trim(skb, len);
} else {
page = buf;
- skb = page_to_skb(vi, page, len);
+ skb = page_to_skb(rq, page, len);
if (unlikely(!skb)) {
dev->stats.rx_dropped++;
- give_pages(vi, page);
+ give_pages(rq, page);
return;
}
if (vi->mergeable_rx_bufs)
- if (receive_mergeable(vi, skb)) {
+ if (receive_mergeable(rq, skb)) {
dev_kfree_skb(skb);
return;
}
@@ -363,90 +417,91 @@ frame_err:
dev_kfree_skb(skb);
}
-static int add_recvbuf_small(struct virtnet_info *vi, gfp_t gfp)
+static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
{
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
int err;
- skb = __netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN, gfp);
+ skb = __netdev_alloc_skb_ip_align(rq->vi->dev, MAX_PACKET_LEN, gfp);
if (unlikely(!skb))
return -ENOMEM;
skb_put(skb, MAX_PACKET_LEN);
hdr = skb_vnet_hdr(skb);
- sg_set_buf(vi->rx_sg, &hdr->hdr, sizeof hdr->hdr);
+ sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr);
+
+ skb_to_sgvec(skb, rq->sg + 1, 0, skb->len);
- skb_to_sgvec(skb, vi->rx_sg + 1, 0, skb->len);
+ err = virtqueue_add_buf(rq->vq, rq->sg, 0, 2, skb, gfp);
- err = virtqueue_add_buf(vi->rvq, vi->rx_sg, 0, 2, skb, gfp);
if (err < 0)
dev_kfree_skb(skb);
return err;
}
-static int add_recvbuf_big(struct virtnet_info *vi, gfp_t gfp)
+static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
{
struct page *first, *list = NULL;
char *p;
int i, err, offset;
- /* page in vi->rx_sg[MAX_SKB_FRAGS + 1] is list tail */
+ /* page in rq->sg[MAX_SKB_FRAGS + 1] is list tail */
for (i = MAX_SKB_FRAGS + 1; i > 1; --i) {
- first = get_a_page(vi, gfp);
+ first = get_a_page(rq, gfp);
if (!first) {
if (list)
- give_pages(vi, list);
+ give_pages(rq, list);
return -ENOMEM;
}
- sg_set_buf(&vi->rx_sg[i], page_address(first), PAGE_SIZE);
+ sg_set_buf(&rq->sg[i], page_address(first), PAGE_SIZE);
/* chain new page in list head to match sg */
first->private = (unsigned long)list;
list = first;
}
- first = get_a_page(vi, gfp);
+ first = get_a_page(rq, gfp);
if (!first) {
- give_pages(vi, list);
+ give_pages(rq, list);
return -ENOMEM;
}
p = page_address(first);
- /* vi->rx_sg[0], vi->rx_sg[1] share the same page */
- /* a separated vi->rx_sg[0] for virtio_net_hdr only due to QEMU bug */
- sg_set_buf(&vi->rx_sg[0], p, sizeof(struct virtio_net_hdr));
+ /* rq->sg[0], rq->sg[1] share the same page */
+ /* a separated rq->sg[0] for virtio_net_hdr only due to QEMU bug */
+ sg_set_buf(&rq->sg[0], p, sizeof(struct virtio_net_hdr));
- /* vi->rx_sg[1] for data packet, from offset */
+ /* rq->sg[1] for data packet, from offset */
offset = sizeof(struct padded_vnet_hdr);
- sg_set_buf(&vi->rx_sg[1], p + offset, PAGE_SIZE - offset);
+ sg_set_buf(&rq->sg[1], p + offset, PAGE_SIZE - offset);
/* chain first in list head */
first->private = (unsigned long)list;
- err = virtqueue_add_buf(vi->rvq, vi->rx_sg, 0, MAX_SKB_FRAGS + 2,
+ err = virtqueue_add_buf(rq->vq, rq->sg, 0, MAX_SKB_FRAGS + 2,
first, gfp);
if (err < 0)
- give_pages(vi, first);
+ give_pages(rq, first);
return err;
}
-static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
+static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
{
struct page *page;
int err;
- page = get_a_page(vi, gfp);
+ page = get_a_page(rq, gfp);
if (!page)
return -ENOMEM;
- sg_init_one(vi->rx_sg, page_address(page), PAGE_SIZE);
+ sg_init_one(rq->sg, page_address(page), PAGE_SIZE);
- err = virtqueue_add_buf(vi->rvq, vi->rx_sg, 0, 1, page, gfp);
+ err = virtqueue_add_buf(rq->vq, rq->sg, 0, 1, page, gfp);
if (err < 0)
- give_pages(vi, page);
+ give_pages(rq, page);
return err;
}
@@ -458,97 +513,104 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
* before we're receiving packets, or from refill_work which is
* careful to disable receiving (using napi_disable).
*/
-static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
+static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
{
+ struct virtnet_info *vi = rq->vi;
int err;
bool oom;
do {
if (vi->mergeable_rx_bufs)
- err = add_recvbuf_mergeable(vi, gfp);
+ err = add_recvbuf_mergeable(rq, gfp);
else if (vi->big_packets)
- err = add_recvbuf_big(vi, gfp);
+ err = add_recvbuf_big(rq, gfp);
else
- err = add_recvbuf_small(vi, gfp);
+ err = add_recvbuf_small(rq, gfp);
oom = err == -ENOMEM;
if (err < 0)
break;
- ++vi->num;
+ ++rq->num;
} while (err > 0);
- if (unlikely(vi->num > vi->max))
- vi->max = vi->num;
- virtqueue_kick(vi->rvq);
+ if (unlikely(rq->num > rq->max))
+ rq->max = rq->num;
+ virtqueue_kick(rq->vq);
return !oom;
}
-static void skb_recv_done(struct virtqueue *rvq)
+static void skb_recv_done(struct virtqueue *vq)
{
- struct virtnet_info *vi = rvq->vdev->priv;
+ struct virtnet_info *vi = vq->vdev->priv;
+ struct napi_struct *napi = &vi->rq[vq2rxq(vq)].napi;
+
/* Schedule NAPI, Suppress further interrupts if successful. */
- if (napi_schedule_prep(&vi->napi)) {
- virtqueue_disable_cb(rvq);
- __napi_schedule(&vi->napi);
+ if (napi_schedule_prep(napi)) {
+ virtqueue_disable_cb(vq);
+ __napi_schedule(napi);
}
}
-static void virtnet_napi_enable(struct virtnet_info *vi)
+static void virtnet_napi_enable(struct receive_queue *rq)
{
- napi_enable(&vi->napi);
+ napi_enable(&rq->napi);
/* If all buffers were filled by other side before we napi_enabled, we
* won't get another interrupt, so process any outstanding packets
* now. virtnet_poll wants re-enable the queue, so we disable here.
* We synchronize against interrupts via NAPI_STATE_SCHED */
- if (napi_schedule_prep(&vi->napi)) {
- virtqueue_disable_cb(vi->rvq);
+ if (napi_schedule_prep(&rq->napi)) {
+ virtqueue_disable_cb(rq->vq);
local_bh_disable();
- __napi_schedule(&vi->napi);
+ __napi_schedule(&rq->napi);
local_bh_enable();
}
}
static void refill_work(struct work_struct *work)
{
- struct virtnet_info *vi;
+ struct napi_struct *napi;
+ struct receive_queue *rq;
bool still_empty;
- vi = container_of(work, struct virtnet_info, refill.work);
- napi_disable(&vi->napi);
- still_empty = !try_fill_recv(vi, GFP_KERNEL);
- virtnet_napi_enable(vi);
+ rq = container_of(work, struct receive_queue, refill.work);
+ napi = &rq->napi;
+
+ napi_disable(napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
/* In theory, this can happen: if we don't get any buffers in
* we will *never* try to fill again. */
if (still_empty)
- schedule_delayed_work(&vi->refill, HZ/2);
+ schedule_delayed_work(&rq->refill, HZ/2);
}
static int virtnet_poll(struct napi_struct *napi, int budget)
{
- struct virtnet_info *vi = container_of(napi, struct virtnet_info, napi);
+ struct receive_queue *rq = container_of(napi, struct receive_queue,
+ napi);
void *buf;
unsigned int len, received = 0;
again:
while (received < budget &&
- (buf = virtqueue_get_buf(vi->rvq, &len)) != NULL) {
- receive_buf(vi->dev, buf, len);
- --vi->num;
+ (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
+ receive_buf(rq, buf, len);
+ --rq->num;
received++;
}
- if (vi->num < vi->max / 2) {
- if (!try_fill_recv(vi, GFP_ATOMIC))
- schedule_delayed_work(&vi->refill, 0);
+ if (rq->num < rq->max / 2) {
+ if (!try_fill_recv(rq, GFP_ATOMIC))
+ schedule_delayed_work(&rq->refill, 0);
}
/* Out of packets? */
if (received < budget) {
napi_complete(napi);
- if (unlikely(!virtqueue_enable_cb(vi->rvq)) &&
+ if (unlikely(!virtqueue_enable_cb(rq->vq)) &&
napi_schedule_prep(napi)) {
- virtqueue_disable_cb(vi->rvq);
+ virtqueue_disable_cb(rq->vq);
__napi_schedule(napi);
goto again;
}
@@ -557,13 +619,14 @@ again:
return received;
}
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static unsigned int free_old_xmit_skbs(struct virtnet_info *vi,
+ struct virtqueue *vq)
{
struct sk_buff *skb;
unsigned int len, tot_sgs = 0;
struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
- while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+ while ((skb = virtqueue_get_buf(vq, &len)) != NULL) {
pr_debug("Sent skb %p\n", skb);
u64_stats_update_begin(&stats->tx_syncp);
@@ -577,7 +640,8 @@ static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
return tot_sgs;
}
-static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
+static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb,
+ struct virtqueue *vq, struct scatterlist *sg)
{
struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
@@ -615,44 +679,47 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
/* Encode metadata header at front. */
if (vi->mergeable_rx_bufs)
- sg_set_buf(vi->tx_sg, &hdr->mhdr, sizeof hdr->mhdr);
+ sg_set_buf(sg, &hdr->mhdr, sizeof hdr->mhdr);
else
- sg_set_buf(vi->tx_sg, &hdr->hdr, sizeof hdr->hdr);
+ sg_set_buf(sg, &hdr->hdr, sizeof hdr->hdr);
- hdr->num_sg = skb_to_sgvec(skb, vi->tx_sg + 1, 0, skb->len) + 1;
- return virtqueue_add_buf(vi->svq, vi->tx_sg, hdr->num_sg,
+ hdr->num_sg = skb_to_sgvec(skb, sg + 1, 0, skb->len) + 1;
+ return virtqueue_add_buf(vq, sg, hdr->num_sg,
0, skb, GFP_ATOMIC);
}
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
+ int qnum = skb_get_queue_mapping(skb);
+ struct virtqueue *vq = vi->sq[qnum].vq;
int capacity;
/* Free up any pending old buffers before queueing new ones. */
- free_old_xmit_skbs(vi);
+ free_old_xmit_skbs(vi, vq);
/* Try to transmit */
- capacity = xmit_skb(vi, skb);
+ capacity = xmit_skb(vi, skb, vq, vi->sq[qnum].sg);
/* This can happen with OOM and indirect buffers. */
if (unlikely(capacity < 0)) {
if (likely(capacity == -ENOMEM)) {
if (net_ratelimit())
dev_warn(&dev->dev,
- "TX queue failure: out of memory\n");
+ "TXQ (%d) failure: out of memory\n",
+ qnum);
} else {
dev->stats.tx_fifo_errors++;
if (net_ratelimit())
dev_warn(&dev->dev,
- "Unexpected TX queue failure: %d\n",
- capacity);
+ "Unexpected TXQ (%d) failure: %d\n",
+ qnum, capacity);
}
dev->stats.tx_dropped++;
kfree_skb(skb);
return NETDEV_TX_OK;
}
- virtqueue_kick(vi->svq);
+ virtqueue_kick(vq);
/* Don't wait up for transmitted skbs to be freed. */
skb_orphan(skb);
@@ -661,13 +728,13 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
/* Apparently nice girls don't return TX_BUSY; stop the queue
* before it gets out of hand. Naturally, this wastes entries. */
if (capacity < 2+MAX_SKB_FRAGS) {
- netif_stop_queue(dev);
- if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
+ netif_stop_subqueue(dev, qnum);
+ if (unlikely(!virtqueue_enable_cb_delayed(vq))) {
/* More just got used, free them then recheck. */
- capacity += free_old_xmit_skbs(vi);
+ capacity += free_old_xmit_skbs(vi, vq);
if (capacity >= 2+MAX_SKB_FRAGS) {
- netif_start_queue(dev);
- virtqueue_disable_cb(vi->svq);
+ netif_start_subqueue(dev, qnum);
+ virtqueue_disable_cb(vq);
}
}
}
@@ -700,7 +767,8 @@ static struct rtnl_link_stats64 *virtnet_stats(struct net_device *dev,
unsigned int start;
for_each_possible_cpu(cpu) {
- struct virtnet_stats *stats = per_cpu_ptr(vi->stats, cpu);
+ struct virtnet_stats __percpu *stats
+ = per_cpu_ptr(vi->stats, cpu);
u64 tpackets, tbytes, rpackets, rbytes;
do {
@@ -734,23 +802,13 @@ static struct rtnl_link_stats64 *virtnet_stats(struct net_device *dev,
static void virtnet_netpoll(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
+ int i;
- napi_schedule(&vi->napi);
+ for (i = 0; i < vi->num_queue_pairs; i++)
+ napi_schedule(&vi->rq[i].napi);
}
#endif
-static int virtnet_open(struct net_device *dev)
-{
- struct virtnet_info *vi = netdev_priv(dev);
-
- /* Make sure we have some buffers: if oom use wq. */
- if (!try_fill_recv(vi, GFP_KERNEL))
- schedule_delayed_work(&vi->refill, 0);
-
- virtnet_napi_enable(vi);
- return 0;
-}
-
/*
* Send command via the control virtqueue and check status. Commands
* supported by the hypervisor, as indicated by feature bits, should
@@ -806,13 +864,63 @@ static void virtnet_ack_link_announce(struct virtnet_info *vi)
rtnl_unlock();
}
+static int virtnet_set_queues(struct virtnet_info *vi)
+{
+ struct scatterlist sg;
+ struct virtio_net_ctrl_steering s;
+ struct net_device *dev = vi->dev;
+
+ if (vi->num_queue_pairs == 1) {
+ s.current_steering_rule = VIRTIO_NET_CTRL_STEERING_SINGLE;
+ s.current_steering_param = 1;
+ } else {
+ s.current_steering_rule =
+ VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX;
+ s.current_steering_param = vi->num_queue_pairs;
+ }
+ sg_init_one(&sg, &s, sizeof(s));
+
+ if (!vi->has_cvq)
+ return -EINVAL;
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_STEERING,
+ VIRTIO_NET_CTRL_STEERING_SET, &sg, 1, 0)){
+ dev_warn(&dev->dev, "Fail to set the number of queue pairs to"
+ " %d\n", vi->num_queue_pairs);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int virtnet_open(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ int i;
+
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ /* Make sure we have some buffers: if oom use wq. */
+ if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
+ schedule_delayed_work(&vi->rq[i].refill, 0);
+ virtnet_napi_enable(&vi->rq[i]);
+ }
+
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_MULTIQUEUE))
+ virtnet_set_queues(vi);
+
+ return 0;
+}
+
static int virtnet_close(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
+ int i;
/* Make sure refill_work doesn't re-enable napi! */
- cancel_delayed_work_sync(&vi->refill);
- napi_disable(&vi->napi);
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ cancel_delayed_work_sync(&vi->rq[i].refill);
+ napi_disable(&vi->rq[i].napi);
+ }
return 0;
}
@@ -924,11 +1032,10 @@ static void virtnet_get_ringparam(struct net_device *dev,
{
struct virtnet_info *vi = netdev_priv(dev);
- ring->rx_max_pending = virtqueue_get_vring_size(vi->rvq);
- ring->tx_max_pending = virtqueue_get_vring_size(vi->svq);
+ ring->rx_max_pending = virtqueue_get_vring_size(vi->rq[0].vq);
+ ring->tx_max_pending = virtqueue_get_vring_size(vi->sq[0].vq);
ring->rx_pending = ring->rx_max_pending;
ring->tx_pending = ring->tx_max_pending;
-
}
@@ -944,12 +1051,6 @@ static void virtnet_get_drvinfo(struct net_device *dev,
}
-static const struct ethtool_ops virtnet_ethtool_ops = {
- .get_drvinfo = virtnet_get_drvinfo,
- .get_link = ethtool_op_get_link,
- .get_ringparam = virtnet_get_ringparam,
-};
-
#define MIN_MTU 68
#define MAX_MTU 65535
@@ -961,6 +1062,22 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
return 0;
}
+/* To avoid contending a lock hold by a vcpu who would exit to host, select the
+ * txq based on the processor id.
+ */
+static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+ int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
+ smp_processor_id();
+ int limit = dev->real_num_tx_queues - 1;
+
+ while (unlikely(txq >= limit))
+ txq -= limit;
+
+ /* queue 0 is reserved for single queue mode */
+ return txq + 1;
+}
+
static const struct net_device_ops virtnet_netdev = {
.ndo_open = virtnet_open,
.ndo_stop = virtnet_close,
@@ -972,6 +1089,7 @@ static const struct net_device_ops virtnet_netdev = {
.ndo_get_stats64 = virtnet_stats,
.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
+ .ndo_select_queue = virtnet_select_queue,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = virtnet_netpoll,
#endif
@@ -1007,10 +1125,10 @@ static void virtnet_config_changed_work(struct work_struct *work)
if (vi->status & VIRTIO_NET_S_LINK_UP) {
netif_carrier_on(vi->dev);
- netif_wake_queue(vi->dev);
+ netif_tx_wake_all_queues(vi->dev);
} else {
netif_carrier_off(vi->dev);
- netif_stop_queue(vi->dev);
+ netif_tx_stop_all_queues(vi->dev);
}
done:
mutex_unlock(&vi->config_lock);
@@ -1023,41 +1141,216 @@ static void virtnet_config_changed(struct virtio_device *vdev)
schedule_work(&vi->config_work);
}
-static int init_vqs(struct virtnet_info *vi)
+static void free_receive_bufs(struct virtnet_info *vi)
{
- struct virtqueue *vqs[3];
- vq_callback_t *callbacks[] = { skb_recv_done, skb_xmit_done, NULL};
- const char *names[] = { "input", "output", "control" };
- int nvqs, err;
+ int i;
- /* We expect two virtqueues, receive then send,
- * and optionally control. */
- nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ while (vi->rq[i].pages)
+ __free_pages(get_a_page(&vi->rq[i], GFP_KERNEL), 0);
+ }
+}
- err = vi->vdev->config->find_vqs(vi->vdev, nvqs, vqs, callbacks, names);
- if (err)
- return err;
+/* Free memory allocated for send and receive queues */
+static void virtnet_free_queues(struct virtnet_info *vi)
+{
+ kfree(vi->rq);
+ vi->rq = NULL;
+ kfree(vi->sq);
+ vi->sq = NULL;
+}
+
+static void free_unused_bufs(struct virtnet_info *vi)
+{
+ void *buf;
+ int i;
+
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ struct virtqueue *vq = vi->sq[i].vq;
+
+ while ((buf = virtqueue_detach_unused_buf(vq)) != NULL)
+ dev_kfree_skb(buf);
+ }
+
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ struct virtqueue *vq = vi->rq[i].vq;
+
+ while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
+ if (vi->mergeable_rx_bufs || vi->big_packets)
+ give_pages(&vi->rq[i], buf);
+ else
+ dev_kfree_skb(buf);
+ --vi->rq[i].num;
+ }
+ BUG_ON(vi->rq[i].num != 0);
+ }
+}
+
+static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
+{
+ int i;
+
+ for (i = 1; i < vi->total_queue_pairs; i++) {
+ int cpu = set ? i - 1 : -1;
+ virtqueue_set_affinity(vi->rq[i].vq, cpu);
+ virtqueue_set_affinity(vi->sq[i].vq, cpu);
+ }
+}
+
+static void virtnet_del_vqs(struct virtnet_info *vi)
+{
+ struct virtio_device *vdev = vi->vdev;
+
+ virtnet_set_affinity(vi, false);
+
+ vdev->config->del_vqs(vdev);
+
+ virtnet_free_queues(vi);
+}
+
+static int virtnet_find_vqs(struct virtnet_info *vi)
+{
+ vq_callback_t **callbacks;
+ struct virtqueue **vqs;
+ int ret = -ENOMEM;
+ int i, total_vqs;
+ char **names;
- vi->rvq = vqs[0];
- vi->svq = vqs[1];
+ /*
+ * We expect 1 RX virtqueue followed by 1 TX virtqueue, followd by
+ * possible control virtqueue, followed by 1 reserved vq, followed
+ * by RX/TX queue pairs used in multiqueue mode.
+ */
+ if (vi->total_queue_pairs == 1)
+ total_vqs = 2 + virtio_has_feature(vi->vdev,
+ VIRTIO_NET_F_CTRL_VQ);
+ else
+ total_vqs = 2 * vi->total_queue_pairs + 2;
+
+ /* Allocate space for find_vqs parameters */
+ vqs = kzalloc(total_vqs * sizeof(*vqs), GFP_KERNEL);
+ callbacks = kzalloc(total_vqs * sizeof(*callbacks), GFP_KERNEL);
+ if (!vqs || !callbacks)
+ goto err_mem;
+ names = kzalloc(total_vqs * sizeof(*names), GFP_KERNEL);
+ if (!names)
+ goto err_mem;
+
+ /* Parameters for control virtqueue, if any */
+ if (vi->has_cvq) {
+ callbacks[2] = NULL;
+ names[2] = kasprintf(GFP_KERNEL, "control");
+ }
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
+ /* Allocate/initialize parameters for send/receive virtqueues */
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ callbacks[rxq2vq(i)] = skb_recv_done;
+ callbacks[txq2vq(i)] = skb_xmit_done;
+ names[rxq2vq(i)] = kasprintf(GFP_KERNEL, "input.%d", i);
+ names[txq2vq(i)] = kasprintf(GFP_KERNEL, "output.%d", i);
+ }
+
+ ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
+ (const char **)names);
+ if (ret)
+ goto err_names;
+
+ if (vi->has_cvq)
vi->cvq = vqs[2];
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
- vi->dev->features |= NETIF_F_HW_VLAN_FILTER;
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ vi->rq[i].vq = vqs[rxq2vq(i)];
+ vi->sq[i].vq = vqs[txq2vq(i)];
}
+
+ kfree(callbacks);
+ kfree(vqs);
+
return 0;
+
+err_names:
+ for (i = 0; i < total_vqs * 2; i ++)
+ kfree(names[i]);
+ kfree(names);
+
+err_mem:
+ kfree(callbacks);
+ kfree(vqs);
+
+ return ret;
+}
+
+static int virtnet_alloc_queues(struct virtnet_info *vi)
+{
+ int i;
+
+ vi->sq = kzalloc(sizeof(vi->sq[0]) * vi->total_queue_pairs, GFP_KERNEL);
+ vi->rq = kzalloc(sizeof(vi->rq[0]) * vi->total_queue_pairs, GFP_KERNEL);
+ if (!vi->rq || !vi->sq)
+ goto err;
+
+ /* setup initial receive and send queue parameters */
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ vi->rq[i].vi = vi;
+ vi->rq[i].pages = NULL;
+ INIT_DELAYED_WORK(&vi->rq[i].refill, refill_work);
+ netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll,
+ napi_weight);
+
+ sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
+ sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
+ }
+
+
+ return 0;
+
+err:
+ virtnet_free_queues(vi);
+ return -ENOMEM;
+}
+
+static int virtnet_setup_vqs(struct virtnet_info *vi)
+{
+ int ret;
+
+ /* Allocate send & receive queues */
+ ret = virtnet_alloc_queues(vi);
+ if (ret)
+ goto err;
+
+ ret = virtnet_find_vqs(vi);
+ if (ret)
+ goto err_free;
+
+ virtnet_set_affinity(vi, true);
+ return 0;
+
+err_free:
+ virtnet_free_queues(vi);
+err:
+ return ret;
}
static int virtnet_probe(struct virtio_device *vdev)
{
- int err;
+ int i, err;
struct net_device *dev;
struct virtnet_info *vi;
+ u16 num_queue_pairs;
+
+ /* Find if host supports multiqueue virtio_net device */
+ err = virtio_config_val(vdev, VIRTIO_NET_F_MULTIQUEUE,
+ offsetof(struct virtio_net_config,
+ max_virtqueue_pairs), &num_queue_pairs);
+
+ /* We need at least 2 queue's */
+ if (err)
+ num_queue_pairs = 1;
+ if (num_queue_pairs > 1)
+ num_queue_pairs ++;
/* Allocate ourselves a network device with room for our info */
- dev = alloc_etherdev(sizeof(struct virtnet_info));
+ dev = alloc_etherdev_mq(sizeof(struct virtnet_info), num_queue_pairs);
if (!dev)
return -ENOMEM;
@@ -1103,22 +1396,17 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Set up our device-specific information */
vi = netdev_priv(dev);
- netif_napi_add(dev, &vi->napi, virtnet_poll, napi_weight);
vi->dev = dev;
vi->vdev = vdev;
vdev->priv = vi;
- vi->pages = NULL;
vi->stats = alloc_percpu(struct virtnet_stats);
err = -ENOMEM;
if (vi->stats == NULL)
- goto free;
+ goto free_netdev;
- INIT_DELAYED_WORK(&vi->refill, refill_work);
mutex_init(&vi->config_lock);
vi->config_enable = true;
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
- sg_init_table(vi->rx_sg, ARRAY_SIZE(vi->rx_sg));
- sg_init_table(vi->tx_sg, ARRAY_SIZE(vi->tx_sg));
/* If we can receive ANY GSO packets, we must allocate large ones. */
if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
@@ -1129,10 +1417,25 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
vi->mergeable_rx_bufs = true;
- err = init_vqs(vi);
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
+ vi->has_cvq = true;
+
+ /* Use single tx/rx queue pair as default */
+ vi->num_queue_pairs = 1;
+ vi->total_queue_pairs = num_queue_pairs;
+
+ /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
+ err = virtnet_setup_vqs(vi);
if (err)
goto free_stats;
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) &&
+ virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
+ dev->features |= NETIF_F_HW_VLAN_FILTER;
+
+ netif_set_real_num_tx_queues(dev, 1);
+ netif_set_real_num_rx_queues(dev, 1);
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -1140,12 +1443,15 @@ static int virtnet_probe(struct virtio_device *vdev)
}
/* Last of all, set up some receive buffers. */
- try_fill_recv(vi, GFP_KERNEL);
-
- /* If we didn't even get one input buffer, we're useless. */
- if (vi->num == 0) {
- err = -ENOMEM;
- goto unregister;
+ for (i = 0; i < vi->total_queue_pairs; i++) {
+ try_fill_recv(&vi->rq[i], GFP_KERNEL);
+
+ /* If we didn't even get one input buffer, we're useless. */
+ if (vi->rq[i].num == 0) {
+ free_unused_bufs(vi);
+ err = -ENOMEM;
+ goto free_recv_bufs;
+ }
}
/* Assume link up if device can't report link status,
@@ -1158,42 +1464,28 @@ static int virtnet_probe(struct virtio_device *vdev)
netif_carrier_on(dev);
}
- pr_debug("virtnet: registered device %s\n", dev->name);
+ pr_debug("virtnet: registered device %s with %d RX and TX vq's\n",
+ dev->name, num_queue_pairs);
+
return 0;
-unregister:
+free_recv_bufs:
+ free_receive_bufs(vi);
unregister_netdev(dev);
+
free_vqs:
- vdev->config->del_vqs(vdev);
+ for (i = 0; i <num_queue_pairs; i++)
+ cancel_delayed_work_sync(&vi->rq[i].refill);
+ virtnet_del_vqs(vi);
+
free_stats:
free_percpu(vi->stats);
-free:
+
+free_netdev:
free_netdev(dev);
return err;
}
-static void free_unused_bufs(struct virtnet_info *vi)
-{
- void *buf;
- while (1) {
- buf = virtqueue_detach_unused_buf(vi->svq);
- if (!buf)
- break;
- dev_kfree_skb(buf);
- }
- while (1) {
- buf = virtqueue_detach_unused_buf(vi->rvq);
- if (!buf)
- break;
- if (vi->mergeable_rx_bufs || vi->big_packets)
- give_pages(vi, buf);
- else
- dev_kfree_skb(buf);
- --vi->num;
- }
- BUG_ON(vi->num != 0);
-}
-
static void remove_vq_common(struct virtnet_info *vi)
{
vi->vdev->config->reset(vi->vdev);
@@ -1201,10 +1493,9 @@ static void remove_vq_common(struct virtnet_info *vi)
/* Free unused buffers in both send and recv, if any. */
free_unused_bufs(vi);
- vi->vdev->config->del_vqs(vi->vdev);
+ free_receive_bufs(vi);
- while (vi->pages)
- __free_pages(get_a_page(vi, GFP_KERNEL), 0);
+ virtnet_del_vqs(vi);
}
static void __devexit virtnet_remove(struct virtio_device *vdev)
@@ -1226,10 +1517,9 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}
-#ifdef CONFIG_PM
-static int virtnet_freeze(struct virtio_device *vdev)
+static void virtnet_stop(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
+ int i;
/* Prevent config work handler from accessing the device */
mutex_lock(&vi->config_lock);
@@ -1237,34 +1527,32 @@ static int virtnet_freeze(struct virtio_device *vdev)
mutex_unlock(&vi->config_lock);
netif_device_detach(vi->dev);
- cancel_delayed_work_sync(&vi->refill);
+ for (i = 0; i < vi->total_queue_pairs; i++)
+ cancel_delayed_work_sync(&vi->rq[i].refill);
if (netif_running(vi->dev))
- napi_disable(&vi->napi);
-
- remove_vq_common(vi);
+ for (i = 0; i < vi->total_queue_pairs; i++)
+ napi_disable(&vi->rq[i].napi);
- flush_work(&vi->config_work);
-
- return 0;
}
-static int virtnet_restore(struct virtio_device *vdev)
+static int virtnet_start(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
- int err;
+ int err, i;
- err = init_vqs(vi);
+ err = virtnet_setup_vqs(vi);
if (err)
return err;
if (netif_running(vi->dev))
- virtnet_napi_enable(vi);
+ for (i = 0; i < vi->total_queue_pairs; i++)
+ virtnet_napi_enable(&vi->rq[i]);
netif_device_attach(vi->dev);
- if (!try_fill_recv(vi, GFP_KERNEL))
- schedule_delayed_work(&vi->refill, 0);
+ for (i = 0; i < vi->total_queue_pairs; i++)
+ if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
+ schedule_delayed_work(&vi->rq[i].refill, 0);
mutex_lock(&vi->config_lock);
vi->config_enable = true;
@@ -1272,6 +1560,29 @@ static int virtnet_restore(struct virtio_device *vdev)
return 0;
}
+
+#ifdef CONFIG_PM
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ virtnet_stop(vi);
+
+ remove_vq_common(vi);
+
+ flush_work(&vi->config_work);
+
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ virtnet_start(vi);
+
+ return 0;
+}
#endif
static struct virtio_device_id id_table[] = {
@@ -1287,7 +1598,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
- VIRTIO_NET_F_GUEST_ANNOUNCE,
+ VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MULTIQUEUE,
};
static struct virtio_driver virtio_net_driver = {
@@ -1305,6 +1616,12 @@ static struct virtio_driver virtio_net_driver = {
#endif
};
+static const struct ethtool_ops virtnet_ethtool_ops = {
+ .get_drvinfo = virtnet_get_drvinfo,
+ .get_link = ethtool_op_get_link,
+ .get_ringparam = virtnet_get_ringparam,
+};
+
static int __init init(void)
{
return register_virtio_driver(&virtio_net_driver);
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 1bc7e30..a43caeb 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -61,6 +61,8 @@ struct virtio_net_config {
__u8 mac[6];
/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
__u16 status;
+ /* Total number of RX/TX queues */
+ __u16 max_virtqueue_pairs;
} __attribute__((packed));
/* This is the first element of the scatter-gather list. If you don't
@@ -167,4 +169,20 @@ struct virtio_net_ctrl_mac {
#define VIRTIO_NET_CTRL_ANNOUNCE 3
#define VIRTIO_NET_CTRL_ANNOUNCE_ACK 0
+/*
+ * Control multiqueue
+ *
+ */
+#define VIRTIO_NET_CTRL_STEERING 4
+ #define VIRTIO_NET_CTRL_STEERING_SET 0
+
+#define VIRTIO_NET_CTRL_STEERING_SINGLE 0
+#define VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX 1
+
+struct virtio_net_ctrl_steering {
+ u8 current_steering_rule;
+ u8 reserved;
+ u16 current_steering_param;
+};
+
#endif /* _LINUX_VIRTIO_NET_H */
--
1.7.1
^ permalink raw reply related
* [rfc net-next v6 1/3] virtio_net: Introduce VIRTIO_NET_F_MULTIQUEUE
From: Jason Wang @ 2012-10-30 10:03 UTC (permalink / raw)
To: mst, davem, virtualization, linux-kernel, netdev, rusty, krkumar2; +Cc: kvm
In-Reply-To: <1351591403-23065-1-git-send-email-jasowang@redhat.com>
From: Krishna Kumar <krkumar2@in.ibm.com>
Introduce VIRTIO_NET_F_MULTIQUEUE.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/uapi/linux/virtio_net.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 2470f54..1bc7e30 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -51,6 +51,7 @@
#define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support */
#define VIRTIO_NET_F_GUEST_ANNOUNCE 21 /* Guest can announce device on the
* network */
+#define VIRTIO_NET_F_MULTIQUEUE 22 /* Device supports multiple TXQ/RXQ */
#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
--
1.7.1
^ permalink raw reply related
* [rfc net-next v6 0/3] Multiqueue virtio-net
From: Jason Wang @ 2012-10-30 10:03 UTC (permalink / raw)
To: mst, davem, virtualization, linux-kernel, netdev, rusty, krkumar2; +Cc: kvm
Hi all:
This series is an update version of multiqueue virtio-net driver based on
Krishna Kumar's work to let virtio-net use multiple rx/tx queues to do the
packets reception and transmission. Please review and comments.
Changes from v5:
- Align the implementation with the RFC spec update v4
- Switch the mode between single mode and multiqueue mode without reset
- Remove the 256 limitation of queues
- Use helpers to do the mapping between virtqueues and tx/rx queues
- Use commbined channels instead of separated rx/tx queus when do the queue
number configuartion
- Other coding style comments from Michael
Reference:
- A protype implementation of qemu-kvm support could by found in
git://github.com/jasowang/qemu-kvm-mq.git
- V5 could be found at http://lwn.net/Articles/505388/
- V4 could be found at https://lkml.org/lkml/2012/6/25/120
- V2 could be found at http://lwn.net/Articles/467283/
- Michael virtio-spec: http://www.spinics.net/lists/netdev/msg209986.html
Perf Numbers:
- Pktgen test shows the receiving capability of the multiqueue virtio-net were
dramatically improved.
- Netperf result shows latency were greately improved according to the test
result. The throughput were kept or improved when transfter with large
packets. But we get regression with small packet (<1500)
transmission/receiving. According to the satistics, TCP tends batch less when mq
is enabled which means much more but smaller pakcets were sent/received whcih
lead much higher cpu utilization and degradate the throughput. In the future,
either tuning of TCP or automatic switch bettwen mq and sq is needed.
Test environment:
- Intel(R) Xeon(R) CPU E5620 @ 2.40GHz, 8 cores 2 numa nodes
- Two directed connected 82599
- Host/Guest kenrel: net-next with the mq virtio-net patches and mq tuntap
patches
Pktgen test:
- Local host generate 64 byte UDP packet to guest.
- average of 20 runs
20 runs
#q #vcpu kpps +improvement
1q 1vcpu: 264kpps +0%
2q 2vcpu: 451kpps +70%
3q 3vcpu: 661kpps +150%
4q 4vcpu: 941kpps +250%
Netperf Local VM to VM test:
- VM1 and its vcpu/vhost thread in numa node 0
- VM2 and its vcpu/vhost thread in numa node 1
- a script is used to lauch the netperf with demo mode and do the postprocessing
to measure the aggreagte result with the help of timestamp
- average of 3 runs
TCP_RR:
size/session/+lat%/+normalize%
1/ 1/ 0%/ 0%
1/ 10/ +52%/ +6%
1/ 20/ +27%/ +5%
64/ 1/ 0%/ 0%
64/ 10/ +45%/ +4%
64/ 20/ +28%/ +7%
256/ 1/ -1%/ 0%
256/ 10/ +38%/ +2%
256/ 20/ +27%/ +6%
TCP_CRR:
size/session/+lat%/+normalize%
1/ 1/ -7%/ -12%
1/ 10/ +34%/ +3%
1/ 20/ +3%/ -8%
64/ 1/ -7%/ -3%
64/ 10/ +32%/ +1%
64/ 20/ +4%/ -7%
256/ 1/ -6%/ -18%
256/ 10/ +33%/ 0%
256/ 20/ +4%/ -8%
STREAM:
size/session/+thu%/+normalize%
1/ 1/ -3%/ 0%
1/ 2/ -1%/ 0%
1/ 4/ -2%/ 0%
64/ 1/ 0%/ +1%
64/ 2/ -6%/ -6%
64/ 4/ -8%/ -14%
256/ 1/ 0%/ 0%
256/ 2/ -48%/ -52%
256/ 4/ -50%/ -55%
512/ 1/ +4%/ +5%
512/ 2/ -29%/ -33%
512/ 4/ -37%/ -49%
1024/ 1/ +6%/ +7%
1024/ 2/ -46%/ -51%
1024/ 4/ -15%/ -17%
4096/ 1/ +1%/ +1%
4096/ 2/ +16%/ -2%
4096/ 4/ +31%/ -10%
16384/ 1/ 0%/ 0%
16384/ 2/ +16%/ +9%
16384/ 4/ +17%/ -9%
Netperf test between external host and guest over 10gb(ixgbe):
- VM thread and vhost threads were pinned int the node 0
- a script is used to lauch the netperf with demo mode and do the postprocessing
to measure the aggreagte result with the help of timestamp
- average of 3 runs
TCP_RR:
size/session/+lat%/+normalize%
1/ 1/ 0%/ +6%
1/ 10/ +41%/ +2%
1/ 20/ +10%/ -3%
64/ 1/ 0%/ -10%
64/ 10/ +39%/ +1%
64/ 20/ +22%/ +2%
256/ 1/ 0%/ +2%
256/ 10/ +26%/ -17%
256/ 20/ +24%/ +10%
TCP_CRR:
size/session/+lat%/+normalize%
1/ 1/ -3%/ -3%
1/ 10/ +34%/ -3%
1/ 20/ 0%/ -15%
64/ 1/ -3%/ -3%
64/ 10/ +34%/ -3%
64/ 20/ -1%/ -16%
256/ 1/ -1%/ -3%
256/ 10/ +38%/ -2%
256/ 20/ -2%/ -17%
TCP_STREAM:(guest receiving)
size/session/+thu%/+normalize%
1/ 1/ +1%/ +14%
1/ 2/ 0%/ +4%
1/ 4/ -2%/ -24%
64/ 1/ -6%/ +1%
64/ 2/ +1%/ +1%
64/ 4/ -1%/ -11%
256/ 1/ +3%/ +4%
256/ 2/ 0%/ -1%
256/ 4/ 0%/ -15%
512/ 1/ +4%/ 0%
512/ 2/ -10%/ -12%
512/ 4/ 0%/ -11%
1024/ 1/ -5%/ 0%
1024/ 2/ -11%/ -16%
1024/ 4/ +3%/ -11%
4096/ 1/ +27%/ +6%
4096/ 2/ 0%/ -12%
4096/ 4/ 0%/ -20%
16384/ 1/ 0%/ -2%
16384/ 2/ 0%/ -9%
16384/ 4/ +10%/ -2%
TCP_MAERTS:(guest sending)
1/ 1/ -1%/ 0%
1/ 2/ 0%/ 0%
1/ 4/ -5%/ 0%
64/ 1/ 0%/ 0%
64/ 2/ -7%/ -8%
64/ 4/ -7%/ -8%
256/ 1/ 0%/ 0%
256/ 2/ -28%/ -28%
256/ 4/ -28%/ -29%
512/ 1/ 0%/ 0%
512/ 2/ -15%/ -13%
512/ 4/ -53%/ -59%
1024/ 1/ +4%/ +13%
1024/ 2/ -7%/ -18%
1024/ 4/ +1%/ -18%
4096/ 1/ +2%/ 0%
4096/ 2/ +3%/ -19%
4096/ 4/ -1%/ -19%
16384/ 1/ -3%/ -1%
16384/ 2/ 0%/ -12%
16384/ 4/ 0%/ -10%
Jason Wang (2):
virtio_net: multiqueue support
virtio-net: change the number of queues through ethtool
Krishna Kumar (1):
virtio_net: Introduce VIRTIO_NET_F_MULTIQUEUE
drivers/net/virtio_net.c | 790 ++++++++++++++++++++++++++++-----------
include/uapi/linux/virtio_net.h | 19 +
2 files changed, 594 insertions(+), 215 deletions(-)
^ permalink raw reply
* [PATCH v8 3/3] virtio_console: Remove buffers from out_vq at port removal
From: Sjur Brændeland @ 2012-10-30 8:51 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Linus Walleij, linux-kernel, virtualization,
Masami Hiramatsu, Amit Shah, Sjur Brændeland
In-Reply-To: <1351587113-2566-1-git-send-email-sjur@brendeland.net>
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Remove buffers from the out-queue when a rproc_serial device
is removed. Rproc_serial communicates with remote processors
that may crash and leave buffers in the out-queue. But the
virtio_console device is not supposed to leave buffers in the
out-queue when a port is removed, so in this case throw a
warning.
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
drivers/char/virtio_console.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9ebadcb..3fa036a 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1521,6 +1521,16 @@ static void remove_port_data(struct port *port)
/* Remove buffers we queued up for the Host to send us data in. */
while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
free_buf(buf, true);
+
+ /*
+ * Check the out-queue for buffers. For VIRTIO_CONSOLE it is a
+ * bug if this happens. But for RPROC_SERIAL the remote processor
+ * may have crashed, leaving buffers hanging in the out-queue.
+ */
+ while ((buf = virtqueue_detach_unused_buf(port->out_vq))) {
+ WARN_ON_ONCE(!is_rproc_serial(port->portdev->vdev));
+ free_buf(buf, true);
+ }
}
/*
--
1.7.9.5
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH v8 2/3] virtio_console: Add support for remoteproc serial
From: Sjur Brændeland @ 2012-10-30 8:51 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Linus Walleij, linux-kernel, virtualization,
Masami Hiramatsu, Amit Shah, Sjur Brændeland
In-Reply-To: <1351587113-2566-1-git-send-email-sjur@brendeland.net>
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Add a simple serial connection driver called
VIRTIO_ID_RPROC_SERIAL (11) for communicating with a
remote processor in an asymmetric multi-processing
configuration.
This implementation reuses the existing virtio_console
implementation, and adds support for DMA allocation
of data buffers and disables use of tty console and
the virtio control queue.
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
Acked-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 190 ++++++++++++++++++++++++++++++++++-----
include/uapi/linux/virtio_ids.h | 1 +
2 files changed, 169 insertions(+), 22 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b84390f..9ebadcb 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -37,8 +37,12 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/module.h>
+#include <linux/dma-mapping.h>
+#include <linux/kconfig.h>
#include "../tty/hvc/hvc_console.h"
+#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
+
/*
* This is a global struct for storing common data for all the devices
* this driver handles.
@@ -112,6 +116,15 @@ struct port_buffer {
/* offset in the buf from which to consume data */
size_t offset;
+ /* DMA address of buffer */
+ dma_addr_t dma;
+
+ /* Device we got DMA memory from */
+ struct device *dev;
+
+ /* List of pending dma buffers to free */
+ struct list_head list;
+
/* If sgpages == 0 then buf is used */
unsigned int sgpages;
@@ -331,6 +344,11 @@ static bool is_console_port(struct port *port)
return false;
}
+static bool is_rproc_serial(const struct virtio_device *vdev)
+{
+ return is_rproc_enabled && vdev->id.device == VIRTIO_ID_RPROC_SERIAL;
+}
+
static inline bool use_multiport(struct ports_device *portdev)
{
/*
@@ -342,11 +360,13 @@ static inline bool use_multiport(struct ports_device *portdev)
return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
}
-static void free_buf(struct port_buffer *buf)
+static DEFINE_SPINLOCK(dma_bufs_lock);
+static LIST_HEAD(pending_free_dma_bufs);
+
+static void free_buf(struct port_buffer *buf, bool can_sleep)
{
unsigned int i;
- kfree(buf->buf);
for (i = 0; i < buf->sgpages; i++) {
struct page *page = sg_page(&buf->sg[i]);
if (!page)
@@ -354,14 +374,57 @@ static void free_buf(struct port_buffer *buf)
put_page(page);
}
+ if (!buf->dev) {
+ kfree(buf->buf);
+ } else if (is_rproc_enabled) {
+ unsigned long flags;
+
+ /* dma_free_coherent requires interrupts to be enabled. */
+ if (!can_sleep) {
+ /* queue up dma-buffers to be freed later */
+ spin_lock_irqsave(&dma_bufs_lock, flags);
+ list_add_tail(&buf->list, &pending_free_dma_bufs);
+ spin_unlock_irqrestore(&dma_bufs_lock, flags);
+ return;
+ }
+ dma_free_coherent(buf->dev, buf->size, buf->buf, buf->dma);
+
+ /* Release device refcnt and allow it to be freed */
+ put_device(buf->dev);
+ }
+
kfree(buf);
}
+static void reclaim_dma_bufs(void)
+{
+ unsigned long flags;
+ struct port_buffer *buf, *tmp;
+ LIST_HEAD(tmp_list);
+
+ if (list_empty(&pending_free_dma_bufs))
+ return;
+
+ /* Create a copy of the pending_free_dma_bufs while holding the lock */
+ spin_lock_irqsave(&dma_bufs_lock, flags);
+ list_cut_position(&tmp_list, &pending_free_dma_bufs,
+ pending_free_dma_bufs.prev);
+ spin_unlock_irqrestore(&dma_bufs_lock, flags);
+
+ /* Release the dma buffers, without irqs enabled */
+ list_for_each_entry_safe(buf, tmp, &tmp_list, list) {
+ list_del(&buf->list);
+ free_buf(buf, true);
+ }
+}
+
static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
int pages)
{
struct port_buffer *buf;
+ reclaim_dma_bufs();
+
/*
* Allocate buffer and the sg list. The sg list array is allocated
* directly after the port_buffer struct.
@@ -373,11 +436,34 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
buf->sgpages = pages;
if (pages > 0) {
+ buf->dev = NULL;
buf->buf = NULL;
return buf;
}
- buf->buf = kmalloc(buf_size, GFP_KERNEL);
+ if (is_rproc_serial(vq->vdev)) {
+ /*
+ * Allocate DMA memory from ancestor. When a virtio
+ * device is created by remoteproc, the DMA memory is
+ * associated with the grandparent device:
+ * vdev => rproc => platform-dev.
+ * The code here would have been less quirky if
+ * DMA_MEMORY_INCLUDES_CHILDREN had been supported
+ * in dma-coherent.c
+ */
+ if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
+ goto free_buf;
+ buf->dev = vq->vdev->dev.parent->parent;
+
+ /* Increase device refcnt to avoid freeing it */
+ get_device(buf->dev);
+ buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
+ GFP_KERNEL);
+ } else {
+ buf->dev = NULL;
+ buf->buf = kmalloc(buf_size, GFP_KERNEL);
+ }
+
if (!buf->buf)
goto free_buf;
buf->len = 0;
@@ -444,7 +530,7 @@ static void discard_port_data(struct port *port)
port->stats.bytes_discarded += buf->len - buf->offset;
if (add_inbuf(port->in_vq, buf) < 0) {
err++;
- free_buf(buf);
+ free_buf(buf, false);
}
port->inbuf = NULL;
buf = get_inbuf(port);
@@ -516,7 +602,7 @@ static void reclaim_consumed_buffers(struct port *port)
return;
}
while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
- free_buf(buf);
+ free_buf(buf, false);
port->outvq_full = false;
}
}
@@ -763,7 +849,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
goto out;
free_buf:
- free_buf(buf);
+ free_buf(buf, true);
out:
return ret;
}
@@ -837,6 +923,15 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
.u.data = &sgl,
};
+ /*
+ * Rproc_serial does not yet support splice. To support splice
+ * pipe_to_sg() must allocate dma-buffers and copy content from
+ * regular pages to dma pages. And alloc_buf and free_buf must
+ * support allocating and freeing such a list of dma-buffers.
+ */
+ if (is_rproc_serial(port->out_vq->vdev))
+ return -EINVAL;
+
ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
if (ret < 0)
return ret;
@@ -855,7 +950,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
if (unlikely(ret <= 0))
- kfree(sgl.sg);
+ free_buf(buf, true);
return ret;
}
@@ -904,6 +999,7 @@ static int port_fops_release(struct inode *inode, struct file *filp)
reclaim_consumed_buffers(port);
spin_unlock_irq(&port->outvq_lock);
+ reclaim_dma_bufs();
/*
* Locks aren't necessary here as a port can't be opened after
* unplug, and if a port isn't unplugged, a kref would already
@@ -1055,7 +1151,10 @@ static void resize_console(struct port *port)
return;
vdev = port->portdev->vdev;
- if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
+
+ /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
+ if (!is_rproc_serial(vdev) &&
+ virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
hvc_resize(port->cons.hvc, port->cons.ws);
}
@@ -1247,7 +1346,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
ret = add_inbuf(vq, buf);
if (ret < 0) {
spin_unlock_irq(lock);
- free_buf(buf);
+ free_buf(buf, true);
break;
}
nr_added_bufs++;
@@ -1335,10 +1434,18 @@ static int add_port(struct ports_device *portdev, u32 id)
goto free_device;
}
- /*
- * If we're not using multiport support, this has to be a console port
- */
- if (!use_multiport(port->portdev)) {
+ if (is_rproc_serial(port->portdev->vdev))
+ /*
+ * For rproc_serial assume remote processor is connected.
+ * rproc_serial does not want the console port, only
+ * the generic port implementation.
+ */
+ port->host_connected = true;
+ else if (!use_multiport(port->portdev)) {
+ /*
+ * If we're not using multiport support,
+ * this has to be a console port.
+ */
err = init_port_console(port);
if (err)
goto free_inbufs;
@@ -1371,7 +1478,7 @@ static int add_port(struct ports_device *portdev, u32 id)
free_inbufs:
while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
- free_buf(buf);
+ free_buf(buf, true);
free_device:
device_destroy(pdrvdata.class, port->dev->devt);
free_cdev:
@@ -1413,7 +1520,7 @@ static void remove_port_data(struct port *port)
/* Remove buffers we queued up for the Host to send us data in. */
while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
- free_buf(buf);
+ free_buf(buf, true);
}
/*
@@ -1615,7 +1722,7 @@ static void control_work_handler(struct work_struct *work)
if (add_inbuf(portdev->c_ivq, buf) < 0) {
dev_warn(&portdev->vdev->dev,
"Error adding buffer to queue\n");
- free_buf(buf);
+ free_buf(buf, false);
}
}
spin_unlock(&portdev->cvq_lock);
@@ -1811,10 +1918,10 @@ static void remove_controlq_data(struct ports_device *portdev)
return;
while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
- free_buf(buf);
+ free_buf(buf, true);
while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
- free_buf(buf);
+ free_buf(buf, true);
}
/*
@@ -1861,11 +1968,15 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
multiport = false;
portdev->config.max_nr_ports = 1;
- if (virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
- offsetof(struct virtio_console_config,
- max_nr_ports),
- &portdev->config.max_nr_ports) == 0)
+
+ /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
+ if (!is_rproc_serial(vdev) &&
+ virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+ offsetof(struct virtio_console_config,
+ max_nr_ports),
+ &portdev->config.max_nr_ports) == 0) {
multiport = true;
+ }
err = init_vqs(portdev);
if (err < 0) {
@@ -1975,6 +2086,16 @@ static unsigned int features[] = {
VIRTIO_CONSOLE_F_MULTIPORT,
};
+static struct virtio_device_id rproc_serial_id_table[] = {
+#if IS_ENABLED(CONFIG_REMOTEPROC)
+ { VIRTIO_ID_RPROC_SERIAL, VIRTIO_DEV_ANY_ID },
+#endif
+ { 0 },
+};
+
+static unsigned int rproc_serial_features[] = {
+};
+
#ifdef CONFIG_PM
static int virtcons_freeze(struct virtio_device *vdev)
{
@@ -2059,6 +2180,20 @@ static struct virtio_driver virtio_console = {
#endif
};
+/*
+ * virtio_rproc_serial refers to __devinit function which causes
+ * section mismatch warnings. So use __refdata to silence warnings.
+ */
+static struct virtio_driver __refdata virtio_rproc_serial = {
+ .feature_table = rproc_serial_features,
+ .feature_table_size = ARRAY_SIZE(rproc_serial_features),
+ .driver.name = "virtio_rproc_serial",
+ .driver.owner = THIS_MODULE,
+ .id_table = rproc_serial_id_table,
+ .probe = virtcons_probe,
+ .remove = virtcons_remove,
+};
+
static int __init init(void)
{
int err;
@@ -2083,7 +2218,15 @@ static int __init init(void)
pr_err("Error %d registering virtio driver\n", err);
goto free;
}
+ err = register_virtio_driver(&virtio_rproc_serial);
+ if (err < 0) {
+ pr_err("Error %d registering virtio rproc serial driver\n",
+ err);
+ goto unregister;
+ }
return 0;
+unregister:
+ unregister_virtio_driver(&virtio_console);
free:
if (pdrvdata.debugfs_dir)
debugfs_remove_recursive(pdrvdata.debugfs_dir);
@@ -2093,7 +2236,10 @@ free:
static void __exit fini(void)
{
+ reclaim_dma_bufs();
+
unregister_virtio_driver(&virtio_console);
+ unregister_virtio_driver(&virtio_rproc_serial);
class_destroy(pdrvdata.class);
if (pdrvdata.debugfs_dir)
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 270fb22..a7630d0 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -37,5 +37,6 @@
#define VIRTIO_ID_RPMSG 7 /* virtio remote processor messaging */
#define VIRTIO_ID_SCSI 8 /* virtio scsi */
#define VIRTIO_ID_9P 9 /* 9p virtio console */
+#define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */
#endif /* _LINUX_VIRTIO_IDS_H */
--
1.7.9.5
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH v8 1/3] virtio_console: Merge struct buffer_token into struct port_buffer
From: Sjur Brændeland @ 2012-10-30 8:51 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Linus Walleij, linux-kernel, virtualization,
Masami Hiramatsu, Amit Shah, Sjur Brændeland
In-Reply-To: <1351587113-2566-1-git-send-email-sjur@brendeland.net>
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Refactoring the splice functionality by unifying the approach for
sending scatter-lists and regular buffers. This simplifies
buffer handling and reduces code size. Splice will now allocate
a port_buffer and send_buf() and free_buf() can always be used
for any buffer.
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
Acked-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 129 +++++++++++++++++------------------------
1 file changed, 53 insertions(+), 76 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 4ad8aca..b84390f 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -111,6 +111,12 @@ struct port_buffer {
size_t len;
/* offset in the buf from which to consume data */
size_t offset;
+
+ /* If sgpages == 0 then buf is used */
+ unsigned int sgpages;
+
+ /* sg is used if spages > 0. sg must be the last in is struct */
+ struct scatterlist sg[0];
};
/*
@@ -338,17 +344,39 @@ static inline bool use_multiport(struct ports_device *portdev)
static void free_buf(struct port_buffer *buf)
{
+ unsigned int i;
+
kfree(buf->buf);
+ for (i = 0; i < buf->sgpages; i++) {
+ struct page *page = sg_page(&buf->sg[i]);
+ if (!page)
+ break;
+ put_page(page);
+ }
+
kfree(buf);
}
-static struct port_buffer *alloc_buf(size_t buf_size)
+static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
+ int pages)
{
struct port_buffer *buf;
- buf = kmalloc(sizeof(*buf), GFP_KERNEL);
+ /*
+ * Allocate buffer and the sg list. The sg list array is allocated
+ * directly after the port_buffer struct.
+ */
+ buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages,
+ GFP_KERNEL);
if (!buf)
goto fail;
+
+ buf->sgpages = pages;
+ if (pages > 0) {
+ buf->buf = NULL;
+ return buf;
+ }
+
buf->buf = kmalloc(buf_size, GFP_KERNEL);
if (!buf->buf)
goto free_buf;
@@ -476,52 +504,26 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
return 0;
}
-struct buffer_token {
- union {
- void *buf;
- struct scatterlist *sg;
- } u;
- /* If sgpages == 0 then buf is used, else sg is used */
- unsigned int sgpages;
-};
-
-static void reclaim_sg_pages(struct scatterlist *sg, unsigned int nrpages)
-{
- int i;
- struct page *page;
-
- for (i = 0; i < nrpages; i++) {
- page = sg_page(&sg[i]);
- if (!page)
- break;
- put_page(page);
- }
- kfree(sg);
-}
/* Callers must take the port->outvq_lock */
static void reclaim_consumed_buffers(struct port *port)
{
- struct buffer_token *tok;
+ struct port_buffer *buf;
unsigned int len;
if (!port->portdev) {
/* Device has been unplugged. vqs are already gone. */
return;
}
- while ((tok = virtqueue_get_buf(port->out_vq, &len))) {
- if (tok->sgpages)
- reclaim_sg_pages(tok->u.sg, tok->sgpages);
- else
- kfree(tok->u.buf);
- kfree(tok);
+ while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
+ free_buf(buf);
port->outvq_full = false;
}
}
static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
int nents, size_t in_count,
- struct buffer_token *tok, bool nonblock)
+ void *data, bool nonblock)
{
struct virtqueue *out_vq;
int err;
@@ -534,7 +536,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
reclaim_consumed_buffers(port);
- err = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC);
+ err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC);
/* Tell Host to go! */
virtqueue_kick(out_vq);
@@ -572,37 +574,6 @@ done:
return in_count;
}
-static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count,
- bool nonblock)
-{
- struct scatterlist sg[1];
- struct buffer_token *tok;
-
- tok = kmalloc(sizeof(*tok), GFP_ATOMIC);
- if (!tok)
- return -ENOMEM;
- tok->sgpages = 0;
- tok->u.buf = in_buf;
-
- sg_init_one(sg, in_buf, in_count);
-
- return __send_to_port(port, sg, 1, in_count, tok, nonblock);
-}
-
-static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents,
- size_t in_count, bool nonblock)
-{
- struct buffer_token *tok;
-
- tok = kmalloc(sizeof(*tok), GFP_ATOMIC);
- if (!tok)
- return -ENOMEM;
- tok->sgpages = nents;
- tok->u.sg = sg;
-
- return __send_to_port(port, sg, nents, in_count, tok, nonblock);
-}
-
/*
* Give out the data that's requested from the buffer that we have
* queued up.
@@ -748,9 +719,10 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
size_t count, loff_t *offp)
{
struct port *port;
- char *buf;
+ struct port_buffer *buf;
ssize_t ret;
bool nonblock;
+ struct scatterlist sg[1];
/* Userspace could be out to fool us */
if (!count)
@@ -766,11 +738,11 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
count = min((size_t)(32 * 1024), count);
- buf = kmalloc(count, GFP_KERNEL);
+ buf = alloc_buf(port->out_vq, count, 0);
if (!buf)
return -ENOMEM;
- ret = copy_from_user(buf, ubuf, count);
+ ret = copy_from_user(buf->buf, ubuf, count);
if (ret) {
ret = -EFAULT;
goto free_buf;
@@ -784,13 +756,14 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
* through to the host.
*/
nonblock = true;
- ret = send_buf(port, buf, count, nonblock);
+ sg_init_one(sg, buf->buf, count);
+ ret = __send_to_port(port, sg, 1, count, buf, nonblock);
if (nonblock && ret > 0)
goto out;
free_buf:
- kfree(buf);
+ free_buf(buf);
out:
return ret;
}
@@ -856,6 +829,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
struct port *port = filp->private_data;
struct sg_list sgl;
ssize_t ret;
+ struct port_buffer *buf;
struct splice_desc sd = {
.total_len = len,
.flags = flags,
@@ -867,17 +841,18 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
if (ret < 0)
return ret;
+ buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
+ if (!buf)
+ return -ENOMEM;
+
sgl.n = 0;
sgl.len = 0;
sgl.size = pipe->nrbufs;
- sgl.sg = kmalloc(sizeof(struct scatterlist) * sgl.size, GFP_KERNEL);
- if (unlikely(!sgl.sg))
- return -ENOMEM;
-
+ sgl.sg = buf->sg;
sg_init_table(sgl.sg, sgl.size);
ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
if (likely(ret > 0))
- ret = send_pages(port, sgl.sg, sgl.n, sgl.len, true);
+ ret = __send_to_port(port, buf->sg, sgl.n, sgl.len, buf, true);
if (unlikely(ret <= 0))
kfree(sgl.sg);
@@ -1033,6 +1008,7 @@ static const struct file_operations port_fops = {
static int put_chars(u32 vtermno, const char *buf, int count)
{
struct port *port;
+ struct scatterlist sg[1];
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
@@ -1041,7 +1017,8 @@ static int put_chars(u32 vtermno, const char *buf, int count)
if (!port)
return -EPIPE;
- return send_buf(port, (void *)buf, count, false);
+ sg_init_one(sg, buf, count);
+ return __send_to_port(port, sg, 1, count, (void *)buf, false);
}
/*
@@ -1262,7 +1239,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
nr_added_bufs = 0;
do {
- buf = alloc_buf(PAGE_SIZE);
+ buf = alloc_buf(vq, PAGE_SIZE, 0);
if (!buf)
break;
--
1.7.9.5
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCHv8 0/3]virtio_console: Add rproc_serial driver
From: Sjur Brændeland @ 2012-10-30 8:51 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Linus Walleij, linux-kernel, virtualization,
Masami Hiramatsu, Amit Shah, Sjur Brændeland
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
This patch-set introduces a new virtio type "rproc_serial" for communicating
with remote processors over shared memory. The driver depends on the
the remoteproc framework. As preparation for introducing "rproc_serial"
I've done a refactoring of the transmit buffer handling.
Changes since v7:
- Rebased to virtio-next branch.
- Removed extra added lines.
- Removed superfluous checks before calling reclaim_dma_bufs().
- Rusty raised some question regarding garbage collection of the
out-vq, so I moved this into a separate patch.
Thanks,
Sjur
Sjur Brændeland (3):
virtio_console: Merge struct buffer_token into struct port_buffer
virtio_console: Add support for remoteproc serial
virtio_console: Remove buffers from out_vq at port removal
drivers/char/virtio_console.c | 325 +++++++++++++++++++++++++++------------
include/uapi/linux/virtio_ids.h | 1 +
2 files changed, 230 insertions(+), 96 deletions(-)
--
1.7.9.5
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 12/12] VMCI: Some header and config files.
From: Dmitry Torokhov @ 2012-10-30 5:22 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030023255.GJ1920@kroah.com>
On Mon, Oct 29, 2012 at 07:32:55PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:05:38PM -0700, George Zhang wrote:
> > --- /dev/null
> > +++ b/drivers/misc/vmw_vmci/Makefile
> > @@ -0,0 +1,43 @@
> > +################################################################################
> > +#
> > +# Linux driver for VMware's VMCI device.
> > +#
> > +# Copyright (C) 2007-2012, VMware, Inc. All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or modify it
> > +# under the terms of the GNU General Public License as published by the
> > +# Free Software Foundation; version 2 of the License and no later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, but
> > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> > +# NON INFRINGEMENT. See the GNU General Public License for more
> > +# details.
> > +#
> > +# Maintained by: Andrew Stiegmann <pv-drivers@vmware.com>
> > +#
> > +################################################################################
>
> That's a big boiler-plate for a makefile :)
>
> Wait, what's Andrew's name doing here, and yet it isn't on any of the
> signed-off-by: or From: lines of the driver? Surely you aren't the only
> contributor here?
>
> > +#
> > +# Makefile for the VMware VMCI
> > +#
>
> That's the only needed comment for this file, if even that.
>
> > +
> > +obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci.o
> > +
> > +vmw_vmci-y += vmci_context.o
> > +vmw_vmci-y += vmci_datagram.o
> > +vmw_vmci-y += vmci_doorbell.o
> > +vmw_vmci-y += vmci_driver.o
> > +vmw_vmci-y += vmci_event.o
> > +vmw_vmci-y += vmci_guest.o
> > +vmw_vmci-y += vmci_handle_array.o
> > +vmw_vmci-y += vmci_host.o
> > +vmw_vmci-y += vmci_queue_pair.o
> > +vmw_vmci-y += vmci_resource.o
> > +vmw_vmci-y += vmci_route.o
>
> You can do this cleaner with multiple .o objects on the same line...
>
> > +vmci:
> > + $(MAKE) -C ../../.. SUBDIRS=$$PWD CONFIG_VMWARE_VMCI=m modules
> > +
> > +clean:
> > + $(MAKE) -C ../../.. SUBDIRS=$$PWD CONFIG_VMWARE_VMCI=m clean
>
> What are these two last targets for? I'm guessing this is from when you
> ported it from a stand-along module? Please remove them.
We'll clean it all up.
Thanks for going over the code.
--
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 08/12] VMCI: resource object implementation.
From: Dmitry Torokhov @ 2012-10-30 5:21 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030022946.GI1920@kroah.com>
On Mon, Oct 29, 2012 at 07:29:46PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote:
> > VMCI resource tracks all used resources within the vmci code.
>
> Same "kref_put() with no lock seen" question in this file, prove me
> wrong please.
Same proof as with others, the reference can't be taken unless the
resource is in hast table (which protected by RCU/spinlock) so no fear
of bouncing off 0.
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 08/12] VMCI: resource object implementation.
From: Dmitry Torokhov @ 2012-10-30 5:20 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030022905.GH1920@kroah.com>
On Mon, Oct 29, 2012 at 07:29:05PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:58PM -0700, George Zhang wrote:
> > +static struct vmci_resource *vmci_resource_lookup(struct vmci_handle handle)
> > +{
> > + struct vmci_resource *r, *resource = NULL;
> > + struct hlist_node *node;
> > + unsigned int idx = vmci_resource_hash(handle);
> > +
> > + BUG_ON(VMCI_HANDLE_EQUAL(handle, VMCI_INVALID_HANDLE));
>
> You just crashed a machine, with no chance for recovery. Not a good
> idea. Never a good idea. Customers just lost data, and now they are
> mad. Make sure you at least print out your email address so they know
> who to blame :)
>
> Seriously, never BUG() in a driver, warn, sure, but this just looks like
> a debugging assert(). Please remove all of these, they are sprinkled
> all over the driver code here, I'm only responding to one of them here.
>
> Even better yet, properly handle the error and keep on going, that's
> what the rest of the kernel does. Or should :)
For public APIs it certainly makes sense to check and handle erroneous input;
internally it often makes sense to simply enforce invariants, because if
we managed to get into that state that we consider impossible we can't really
trust anything.
FWIW:
[dtor@dtor-ws kernel]$ grep -r BUG_ON . | wc -l
11269
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 05/12] VMCI: event handling implementation.
From: Dmitry Torokhov @ 2012-10-30 5:01 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030022605.GG1920@kroah.com>
On Mon, Oct 29, 2012 at 07:26:05PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
> > +static void event_signal_destroy(struct kref *kref)
> > +{
> > + struct vmci_subscription *entry =
> > + container_of(kref, struct vmci_subscription, kref);
> > +
> > + complete(&entry->done);
> > +}
>
> Didn't you just leak memory here? What frees the structure up?
event_unregister_subscription() waits for that completion and frees the
structure. We want event_unregister_subscription() to wait until all
fired callbacks completed before unregister is complete.
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 05/12] VMCI: event handling implementation.
From: Dmitry Torokhov @ 2012-10-30 4:58 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030022446.GF1920@kroah.com>
On Mon, Oct 29, 2012 at 07:24:46PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:27PM -0700, George Zhang wrote:
> > +/*
> > + * Releases the given VMCISubscription.
> > + * Fires the destroy event if the reference count has gone to zero.
> > + */
> > +static void event_release(struct vmci_subscription *entry)
> > +{
> > + kref_put(&entry->kref, event_signal_destroy);
> > +}
>
> Same question as before with the kref_put() call, what is handling the
> locking here? It looks like a race to me.
The reference is taken only if event is on the list (which managed by
RCU and a mutex), so it is not possible to go from 0->1 for that
refcount.
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 04/12] VMCI: device driver implementaton.
From: Dmitry Torokhov @ 2012-10-30 4:15 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030022347.GE1920@kroah.com>
On Mon, Oct 29, 2012 at 07:23:47PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:04:15PM -0700, George Zhang wrote:
> > +static int __init vmci_core_init(void)
> > +{
> > + int result;
> > +
> > + result = vmci_ctx_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIContext (result=%d).\n",
> > + result);
>
> If you are going to use pr_* functions, it's usually a good idea to
> define pr_fmt in a consistant way so that it shows up the same across
> all of your .c files. See other drivers for examples of how to do this
> properly.
pr_fmt() is defined in drivers/misc/vmw_vmci/vmci_common_int.h
>
> > + return -EINVAL;
> > + }
> > +
> > + result = vmci_datagram_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIDatagram (result=%d).\n",
> > + result);
> > + return -EINVAL;
> > + }
> > +
> > + result = vmci_event_init();
> > + if (result < VMCI_SUCCESS) {
> > + pr_err("Failed to initialize VMCIEvent (result=%d).\n", result);
> > + return -EINVAL;
> > + }
>
> You don't free and unwind things properly if things go wrong here, why
> not?
We do, the above calls do not need to be cleaned up.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit vmci_core_exit(void)
> > +{
> > + vmci_event_exit();
> > +}
> > +
> > +static int __init vmci_drv_init(void)
> > +{
> > + int error;
> > +
> > + error = vmci_core_init();
> > + if (error)
> > + return error;
> > +
> > + if (!vmci_disable_guest) {
> > + error = vmci_guest_init();
> > + if (error) {
> > + pr_warn("Failed to initialize guest personality (err=%d).\n",
> > + error);
> > + } else {
> > + vmci_guest_personality_initialized = true;
> > + pr_info("Guest personality initialized and is %s\n",
> > + vmci_guest_code_active() ?
> > + "active" : "inactive");
> > + }
> > + }
> > +
> > + if (!vmci_disable_host) {
> > + error = vmci_host_init();
> > + if (error) {
> > + pr_warn("Unable to initialize host personality (err=%d).\n",
> > + error);
> > + } else {
> > + vmci_host_personality_initialized = true;
> > + pr_info("Initialized host personality\n");
> > + }
> > + }
> > +
> > + if (!vmci_guest_personality_initialized &&
> > + !vmci_host_personality_initialized) {
> > + vmci_core_exit();
> > + return -ENODEV;
> > + }
> > +
> > + pr_debug("Module is initialized\n");
>
> Really? You need this message still?
These are debug messages so do not show up in logs by default.
>
> > + return 0;
> > +}
> > +module_init(vmci_drv_init);
> > +
> > +static void __exit vmci_drv_exit(void)
> > +{
> > + if (vmci_guest_personality_initialized)
> > + vmci_guest_exit();
> > +
> > + if (vmci_host_personality_initialized)
> > + vmci_host_exit();
> > +
> > + vmci_core_exit();
> > + pr_debug("Module is unloaded\n");
>
> Same here, is this really needed?
>
Still debug so no littering in logs.
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 00/12] VMCI for Linux upstreaming
From: Dmitry Torokhov @ 2012-10-30 4:07 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030021938.GC1920@kroah.com>
Hi Greg,
On Mon, Oct 29, 2012 at 07:19:38PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:03:28PM -0700, George Zhang wrote:
> > drivers/misc/Kconfig | 1
> > drivers/misc/Makefile | 2
> > drivers/misc/vmw_vmci/Kconfig | 16
> > drivers/misc/vmw_vmci/Makefile | 43
>
> Meta comment here, why drivers/misc/? The other hypervisor
> infrastructures all have their own directory under drivers/ Should we
> be moving everything to drivers/hyperv/ somehow?
drivers/hyperv is not the best name for obvious reasons...
I think that even if we had a special directory for vmci having network
drivers in Dave's realm and pvscsi in James's is best option, so the new
directory would contain vmci and the balloon driver (vsock will go into
net/). Given that balloon is already in drivers/misc it looked like
obvious place for VMCI as well.
Thanks,
Dmitry
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 01/12] VMCI: context implementation.
From: Dmitry Torokhov @ 2012-10-30 4:01 UTC (permalink / raw)
To: Greg KH; +Cc: pv-drivers, linux-kernel, George Zhang, virtualization
In-Reply-To: <20121030021058.GB1920@kroah.com>
Hi Greg,
On Mon, Oct 29, 2012 at 07:10:58PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:03:42PM -0700, George Zhang wrote:
> > +/*
> > + * Releases the VMCI context. If this is the last reference to
> > + * the context it will be deallocated. A context is created with
> > + * a reference count of one, and on destroy, it is removed from
> > + * the context list before its reference count is
> > + * decremented. Thus, if we reach zero, we are sure that nobody
> > + * else are about to increment it (they need the entry in the
> > + * context list for that). This function musn't be called with a
> > + * lock held.
> > + */
> > +void vmci_ctx_release(struct vmci_ctx *context)
> > +{
> > + ASSERT(context);
> > + kref_put(&context->kref, ctx_free_ctx);
> > +}
> > +
>
> Hm, are you _sure_ you should be calling this without a lock held?
> That's usually kref-101, you MUST hold a lock when calling put,
> otherwise you can race a kref_get() call, and all hell can break loose.
>
> Because of this, some saner people (like Al Viro), have suggested that I
> force the kref_put() and kref_get() calls pass in a spinlock just to
> enforce this.
>
> So, tell me what I'm missing here, and why you put the comment here
> saying that it really is supposed to be called without a lock held? How
> is that safe?
>
Contexts are created/registered in vmci_ctx_init_ctx() and unregistered in
vmci_ctx_release_ctx() and these operations are protected by
ctx_list.lock spinlock. Context lookup (vmci_ctx_get) also uses spinlock
to traverse list of registered contexts and then grabs reference to the
[valid] context. The use of kref_put() without additional locking in
vmci_ctx_release() is fine as there is no chance of another thread
bumping count from 0 to 1.
I believe the comment should actually read that the function should not
be called from atomic contexts.
Thanks,
Dmitry
^ 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