* Re: [virtio-dev] Re: [PATCH v7 net-next 2/4] net: Introduce generic failover module
From: Michael S. Tsirkin @ 2018-04-20 16:03 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, Netdev,
David Miller
In-Reply-To: <CAKgT0UeQTx7zJPK3K3eM9xxHfVyHXwJ-G_b8eqGn0bWAyt9aAg@mail.gmail.com>
On Fri, Apr 20, 2018 at 08:56:57AM -0700, Alexander Duyck wrote:
> On Fri, Apr 20, 2018 at 8:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Apr 20, 2018 at 08:21:00AM -0700, Samudrala, Sridhar wrote:
> >> > > + finfo = netdev_priv(failover_dev);
> >> > > +
> >> > > + primary_dev = rtnl_dereference(finfo->primary_dev);
> >> > > + standby_dev = rtnl_dereference(finfo->standby_dev);
> >> > > +
> >> > > + if (slave_dev != primary_dev && slave_dev != standby_dev)
> >> > > + goto done;
> >> > > +
> >> > > + if ((primary_dev && failover_xmit_ready(primary_dev)) ||
> >> > > + (standby_dev && failover_xmit_ready(standby_dev))) {
> >> > > + netif_carrier_on(failover_dev);
> >> > > + netif_tx_wake_all_queues(failover_dev);
> >> > > + } else {
> >> > > + netif_carrier_off(failover_dev);
> >> > > + netif_tx_stop_all_queues(failover_dev);
> >> > And I think it's a good idea to get stats from device here too.
> >>
> >> Not sure why we need to get stats from lower devs here?
> >
> > link down is often indication of a hardware problem.
> > lower dev might stop responding down the road.
> >
> >> > > +static const struct net_device_ops failover_dev_ops = {
> >> > > + .ndo_open = failover_open,
> >> > > + .ndo_stop = failover_close,
> >> > > + .ndo_start_xmit = failover_start_xmit,
> >> > > + .ndo_select_queue = failover_select_queue,
> >> > > + .ndo_get_stats64 = failover_get_stats,
> >> > > + .ndo_change_mtu = failover_change_mtu,
> >> > > + .ndo_set_rx_mode = failover_set_rx_mode,
> >> > > + .ndo_validate_addr = eth_validate_addr,
> >> > > + .ndo_features_check = passthru_features_check,
> >> > xdp support?
> >>
> >> I think it should be possible to add it be calling the lower dev ndo_xdp routines
> >> with proper checks. can we add this later?
> >
> > I'd be concerned that if you don't xdp userspace will keep poking
> > at lower devs. Then it will stop working if you add this later.
>
> The failover device is better off not providing in-driver XDP since
> there are already skbs allocated by the time that we see the packet
> here anyway. As such generic XDP is the preferred way to handle this
> since it will work regardless of what lower devices are present.
>
> The only advantage of having XDP down at the virtio or VF level would
> be that it performs better, but at the cost of complexity since we
> would need to rebind the eBPF program any time a device is hotplugged
> out and then back in. For now the current approach is in keeping with
> how bonding and other similar drivers are currently handling this.
>
> Thanks.
>
> - Alex
OK fair enough.
--
MST
^ permalink raw reply
* virtio remoteproc device
From: Michael S. Tsirkin @ 2018-04-20 16:53 UTC (permalink / raw)
To: linux-remoteproc; +Cc: Ohad Ben-Cohen, virtualization, Bjorn Andersson
Hello!
I note the following in the serial console:
if (is_rproc_serial(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.
*/
if (!vdev->dev.parent || !vdev->dev.parent->parent)
goto free_buf;
buf->dev = 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);
}
Added here:
commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Fri Dec 14 14:40:51 2012 +1030
virtio_console: Add support for remoteproc serial
I am not familiar with rproc so I have a question:
why is it required to use coherent memory here,
and why through a grandparent device?
Would it work to instead change vring_use_dma_api
to whitelist rproc (like we do for xen)?
I can sent a patch for your testing.
Thanks!
--
MST
^ permalink raw reply
* [PATCH 0/6] virtio-console: spec compliance fixes
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
Turns out virtio console tries to take a buffer out of an active vq.
Works by sheer luck, and is explicitly forbidden by spec. And while
going over it I saw that error handling is also broken -
failure is easy to trigger if I force allocations to fail.
Lightly tested.
Michael S. Tsirkin (6):
virtio_console: don't tie bufs to a vq
virtio: add ability to iterate over vqs
virtio_console: free buffers after reset
virtio_console: drop custom control queue cleanup
virtio_console: move removal code
virtio_console: reset on out of memory
drivers/char/virtio_console.c | 155 ++++++++++++++++++++----------------------
include/linux/virtio.h | 3 +
2 files changed, 75 insertions(+), 83 deletions(-)
--
MST
^ permalink raw reply
* [PATCH 1/6] virtio_console: don't tie bufs to a vq
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
an allocated buffer doesn't need to be tied to a vq -
only vq->vdev is ever used. Pass the function the
just what it needs - the vdev.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 468f061..3e56f32 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -422,7 +422,7 @@ static void reclaim_dma_bufs(void)
}
}
-static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
+static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size,
int pages)
{
struct port_buffer *buf;
@@ -445,16 +445,16 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
return buf;
}
- if (is_rproc_serial(vq->vdev)) {
+ if (is_rproc_serial(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.
*/
- if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
+ if (!vdev->dev.parent || !vdev->dev.parent->parent)
goto free_buf;
- buf->dev = vq->vdev->dev.parent->parent;
+ buf->dev = vdev->dev.parent->parent;
/* Increase device refcnt to avoid freeing it */
get_device(buf->dev);
@@ -838,7 +838,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
count = min((size_t)(32 * 1024), count);
- buf = alloc_buf(port->out_vq, count, 0);
+ buf = alloc_buf(port->portdev->vdev, count, 0);
if (!buf)
return -ENOMEM;
@@ -957,7 +957,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
if (ret < 0)
goto error_out;
- buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
+ buf = alloc_buf(port->portdev->vdev, 0, pipe->nrbufs);
if (!buf) {
ret = -ENOMEM;
goto error_out;
@@ -1374,7 +1374,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
nr_added_bufs = 0;
do {
- buf = alloc_buf(vq, PAGE_SIZE, 0);
+ buf = alloc_buf(vq->vdev, PAGE_SIZE, 0);
if (!buf)
break;
--
MST
^ permalink raw reply related
* [PATCH 3/6] virtio_console: free buffers after reset
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization, stable
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
Console driver is out of spec. The spec says:
A driver MUST NOT decrement the available idx on a live
virtqueue (ie. there is no way to “unexpose” buffers).
and it does exactly that by trying to detach unused buffers
without doing a device reset first.
Defer detaching the buffers until device unplug.
Of course this means we might get an interrupt for
a vq without an attached port now. Handle that by
discarding the consumed buffer.
Reported-by: Tiwei Bie <tiwei.bie@intel.com>
Fixes: b3258ff1d6 ("virtio: Decrement avail idx on buffer detach")
CC: stable@kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 49 +++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3e56f32..26a66ff 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1402,7 +1402,6 @@ static int add_port(struct ports_device *portdev, u32 id)
{
char debugfs_name[16];
struct port *port;
- struct port_buffer *buf;
dev_t devt;
unsigned int nr_added_bufs;
int err;
@@ -1513,8 +1512,6 @@ static int add_port(struct ports_device *portdev, u32 id)
return 0;
free_inbufs:
- while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
- free_buf(buf, true);
free_device:
device_destroy(pdrvdata.class, port->dev->devt);
free_cdev:
@@ -1539,34 +1536,14 @@ static void remove_port(struct kref *kref)
static void remove_port_data(struct port *port)
{
- struct port_buffer *buf;
-
spin_lock_irq(&port->inbuf_lock);
/* Remove unused data this port might have received. */
discard_port_data(port);
spin_unlock_irq(&port->inbuf_lock);
- /* Remove buffers we queued up for the Host to send us data in. */
- do {
- spin_lock_irq(&port->inbuf_lock);
- buf = virtqueue_detach_unused_buf(port->in_vq);
- spin_unlock_irq(&port->inbuf_lock);
- if (buf)
- free_buf(buf, true);
- } while (buf);
-
spin_lock_irq(&port->outvq_lock);
reclaim_consumed_buffers(port);
spin_unlock_irq(&port->outvq_lock);
-
- /* Free pending buffers from the out-queue. */
- do {
- spin_lock_irq(&port->outvq_lock);
- buf = virtqueue_detach_unused_buf(port->out_vq);
- spin_unlock_irq(&port->outvq_lock);
- if (buf)
- free_buf(buf, true);
- } while (buf);
}
/*
@@ -1791,13 +1768,24 @@ static void control_work_handler(struct work_struct *work)
spin_unlock(&portdev->c_ivq_lock);
}
+static void flush_bufs(struct virtqueue *vq, bool can_sleep)
+{
+ struct port_buffer *buf;
+ unsigned int len;
+
+ while ((buf = virtqueue_get_buf(vq, &len)))
+ free_buf(buf, can_sleep);
+}
+
static void out_intr(struct virtqueue *vq)
{
struct port *port;
port = find_port_by_vq(vq->vdev->priv, vq);
- if (!port)
+ if (!port) {
+ flush_bufs(vq, false);
return;
+ }
wake_up_interruptible(&port->waitqueue);
}
@@ -1808,8 +1796,10 @@ static void in_intr(struct virtqueue *vq)
unsigned long flags;
port = find_port_by_vq(vq->vdev->priv, vq);
- if (!port)
+ if (!port) {
+ flush_bufs(vq, false);
return;
+ }
spin_lock_irqsave(&port->inbuf_lock, flags);
port->inbuf = get_inbuf(port);
@@ -1984,6 +1974,15 @@ static const struct file_operations portdev_fops = {
static void remove_vqs(struct ports_device *portdev)
{
+ struct virtqueue *vq;
+
+ virtio_device_for_each_vq(portdev->vdev, vq) {
+ struct port_buffer *buf;
+
+ flush_bufs(vq, true);
+ while ((buf = virtqueue_detach_unused_buf(vq)))
+ free_buf(buf, true);
+ }
portdev->vdev->config->del_vqs(portdev->vdev);
kfree(portdev->in_vqs);
kfree(portdev->out_vqs);
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH 2/6] virtio: add ability to iterate over vqs
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
For cleanup it's helpful to be able to simply scan all vqs and discard
all data. Add an iterator to do that.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 988c735..fa1b5da 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -157,6 +157,9 @@ int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
#endif
+#define virtio_device_for_each_vq(vdev, vq) \
+ list_for_each_entry(vq, &vdev->vqs, list)
+
/**
* virtio_driver - operations for a virtio I/O driver
* @driver: underlying device driver (populate name and owner).
--
MST
^ permalink raw reply related
* [PATCH 4/6] virtio_console: drop custom control queue cleanup
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
We now cleanup all VQs on device removal - no need
to handle the control VQ specially.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 26a66ff..2d87ce5 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1988,21 +1988,6 @@ static void remove_vqs(struct ports_device *portdev)
kfree(portdev->out_vqs);
}
-static void remove_controlq_data(struct ports_device *portdev)
-{
- struct port_buffer *buf;
- unsigned int len;
-
- if (!use_multiport(portdev))
- return;
-
- while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
- free_buf(buf, true);
-
- while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
- free_buf(buf, true);
-}
-
/*
* Once we're further in boot, we get probed like any other virtio
* device.
@@ -2163,7 +2148,6 @@ static void virtcons_remove(struct virtio_device *vdev)
* have to just stop using the port, as the vqs are going
* away.
*/
- remove_controlq_data(portdev);
remove_vqs(portdev);
kfree(portdev);
}
@@ -2208,7 +2192,6 @@ static int virtcons_freeze(struct virtio_device *vdev)
*/
if (use_multiport(portdev))
virtqueue_disable_cb(portdev->c_ivq);
- remove_controlq_data(portdev);
list_for_each_entry(port, &portdev->ports, list) {
virtqueue_disable_cb(port->in_vq);
--
MST
^ permalink raw reply related
* [PATCH 5/6] virtio_console: move removal code
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
Will make it reusable for error handling.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 72 +++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 2d87ce5..e8480fe 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1988,6 +1988,42 @@ static void remove_vqs(struct ports_device *portdev)
kfree(portdev->out_vqs);
}
+static void virtcons_remove(struct virtio_device *vdev)
+{
+ struct ports_device *portdev;
+ struct port *port, *port2;
+
+ portdev = vdev->priv;
+
+ spin_lock_irq(&pdrvdata_lock);
+ list_del(&portdev->list);
+ spin_unlock_irq(&pdrvdata_lock);
+
+ /* Disable interrupts for vqs */
+ vdev->config->reset(vdev);
+ /* Finish up work that's lined up */
+ if (use_multiport(portdev))
+ cancel_work_sync(&portdev->control_work);
+ else
+ cancel_work_sync(&portdev->config_work);
+
+ list_for_each_entry_safe(port, port2, &portdev->ports, list)
+ unplug_port(port);
+
+ unregister_chrdev(portdev->chr_major, "virtio-portsdev");
+
+ /*
+ * When yanking out a device, we immediately lose the
+ * (device-side) queues. So there's no point in keeping the
+ * guest side around till we drop our final reference. This
+ * also means that any ports which are in an open state will
+ * have to just stop using the port, as the vqs are going
+ * away.
+ */
+ remove_vqs(portdev);
+ kfree(portdev);
+}
+
/*
* Once we're further in boot, we get probed like any other virtio
* device.
@@ -2116,42 +2152,6 @@ static int virtcons_probe(struct virtio_device *vdev)
return err;
}
-static void virtcons_remove(struct virtio_device *vdev)
-{
- struct ports_device *portdev;
- struct port *port, *port2;
-
- portdev = vdev->priv;
-
- spin_lock_irq(&pdrvdata_lock);
- list_del(&portdev->list);
- spin_unlock_irq(&pdrvdata_lock);
-
- /* Disable interrupts for vqs */
- vdev->config->reset(vdev);
- /* Finish up work that's lined up */
- if (use_multiport(portdev))
- cancel_work_sync(&portdev->control_work);
- else
- cancel_work_sync(&portdev->config_work);
-
- list_for_each_entry_safe(port, port2, &portdev->ports, list)
- unplug_port(port);
-
- unregister_chrdev(portdev->chr_major, "virtio-portsdev");
-
- /*
- * When yanking out a device, we immediately lose the
- * (device-side) queues. So there's no point in keeping the
- * guest side around till we drop our final reference. This
- * also means that any ports which are in an open state will
- * have to just stop using the port, as the vqs are going
- * away.
- */
- remove_vqs(portdev);
- kfree(portdev);
-}
-
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_CONSOLE, VIRTIO_DEV_ANY_ID },
{ 0 },
--
MST
^ permalink raw reply related
* [PATCH 6/6] virtio_console: reset on out of memory
From: Michael S. Tsirkin @ 2018-04-20 18:18 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Amit Shah, Greg Kroah-Hartman, stable,
virtualization
In-Reply-To: <1524248223-393618-1-git-send-email-mst@redhat.com>
When out of memory and we can't add ctrl vq buffers,
probe fails. Unfortunately the error handling is
out of spec: it calls del_vqs without bothering
to reset the device first.
To fix, call the full cleanup function in this case.
Cc: stable@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e8480fe..2108551 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2090,6 +2090,7 @@ static int virtcons_probe(struct virtio_device *vdev)
spin_lock_init(&portdev->ports_lock);
INIT_LIST_HEAD(&portdev->ports);
+ INIT_LIST_HEAD(&portdev->list);
virtio_device_ready(portdev->vdev);
@@ -2107,8 +2108,15 @@ static int virtcons_probe(struct virtio_device *vdev)
if (!nr_added_bufs) {
dev_err(&vdev->dev,
"Error allocating buffers for control queue\n");
- err = -ENOMEM;
- goto free_vqs;
+ /*
+ * The host might want to notify mgmt sw about device
+ * add failure.
+ */
+ __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
+ VIRTIO_CONSOLE_DEVICE_READY, 0);
+ /* Device was functional: we need full cleanup. */
+ virtcons_remove(vdev);
+ return -ENOMEM;
}
} else {
/*
@@ -2139,11 +2147,6 @@ static int virtcons_probe(struct virtio_device *vdev)
return 0;
-free_vqs:
- /* The host might want to notify mgmt sw about device add failure */
- __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
- VIRTIO_CONSOLE_DEVICE_READY, 0);
- remove_vqs(portdev);
free_chrdev:
unregister_chrdev(portdev->chr_major, "virtio-portsdev");
free:
--
MST
^ permalink raw reply related
* Re: [PATCH net-next 0/5] virtio-net: Add SCTP checksum offload support
From: Michael S. Tsirkin @ 2018-04-20 18:32 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: nhorman, netdev, Vladislav Yasevich, virtualization, linux-sctp
In-Reply-To: <20180420172219.GR4716@localhost.localdomain>
On Fri, Apr 20, 2018 at 02:22:19PM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Apr 18, 2018 at 05:06:46PM +0300, Michael S. Tsirkin wrote:
> > On Tue, Apr 17, 2018 at 04:35:18PM -0400, Vlad Yasevich wrote:
> > > On 04/02/2018 10:47 AM, Marcelo Ricardo Leitner wrote:
> > > > On Mon, Apr 02, 2018 at 09:40:01AM -0400, Vladislav Yasevich wrote:
> > > >> Now that we have SCTP offload capabilities in the kernel, we can add
> > > >> them to virtio as well. First step is SCTP checksum.
> > > >
> > > > Thanks.
> > > >
> > > >> As for GSO, the way sctp GSO is currently implemented buys us nothing
> > > >> in added support to virtio. To add true GSO, would require a lot of
> > > >> re-work inside of SCTP and would require extensions to the virtio
> > > >> net header to carry extra sctp data.
> > > >
> > > > Can you please elaborate more on this? Is this because SCTP GSO relies
> > > > on the gso skb format for knowing how to segment it instead of having
> > > > a list of sizes?
> > > >
> > >
> > > it's mainly because all the true segmentation, placing data into chunks,
> > > has already happened. All that GSO does is allow for higher bundling
> > > rate between VMs. If that is all SCTP GSO ever going to do, that fine,
> > > but the goal is to do real GSO eventually and potentially reduce the
> > > amount of memory copying we are doing.
> > > If we do that, any current attempt at GSO in virtio would have to be
> > > depricated and we'd need GSO2 or something like that.
> >
> > Batching helps virtualization *a lot* though.
>
> Yep. The results posted by Xin in the other email give good insights
> on it.
>
> > Are there actual plans for GSO2? Is it just for SCTP?
>
> No plans. In this context, at least, yes, just for SCTP.
>
> It was a supposition in case we start doing a different GSO for SCTP,
> one more like what we have for TCP.
>
> Currently, as the SCTP GSO code doesn't leave the system, we can
> update it if we want. But by the moment we add support for it in
> virtio, we will have to be backwards compatible if we end up doing
> SCTP GSO differently.
At least for TX you can always just disable the optimization. Won't be
worse than what is there now. RX is trickier - but that's GRO
not GSO.
> But again, I don't think such approach for SCTP GSO would be neither
> feasible or worth. The complexity for it, to work across stream
> schedules and late TSN allocation, would do more harm then good IMO.
>
> >
> > >
> > > This is why, after doing the GSO support, I decided not to include it.
> > >
> > > -vlad
> > > > Marcelo
> > > >
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Mikulas Patocka @ 2018-04-20 20:54 UTC (permalink / raw)
To: Michal Hocko
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, virtualization,
linux-mm, edumazet, bhutchings, Andrew Morton, David Miller,
Vlastimil Babka
In-Reply-To: <20180420130852.GC16083@dhcp22.suse.cz>
On Fri, 20 Apr 2018, Michal Hocko wrote:
> On Thu 19-04-18 12:12:38, Mikulas Patocka wrote:
> [...]
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Subject: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
> >
> > The kvmalloc function tries to use kmalloc and falls back to vmalloc if
> > kmalloc fails.
> >
> > Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
> > uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> > found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> > code.
> >
> > These bugs are hard to reproduce because vmalloc falls back to kmalloc
> > only if memory is fragmented.
> >
> > In order to detect these bugs reliably I submit this patch that changes
> > kvmalloc to always use vmalloc if CONFIG_DEBUG_VM is turned on.
>
> No way. This is just wrong! First of all, you will explode most likely
> on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> enabled quite often.
You're an evil person who doesn't want to fix bugs.
You refused to fix vmalloc(GFP_NOIO) misbehavior a year ago (did you make
some progress with it since that time?) and you refuse to fix kvmalloc
misuses.
I tried this patch on text-only virtual machine and /proc/vmallocinfo
shows 614kB more memory. I tried it on a desktop machine with the chrome
browser open and /proc/vmallocinfo space is increased by 7MB. So no - this
won't exhaust memory and kill the machine.
Arguing that this increases memory consumption is as bogus as arguing that
CONFIG_LOCKDEP increses memory consumption. No one is forcing you to
enable CONFIG_LOCKDEP and no one is forcing you to enable this kvmalloc
test too.
Mikulas
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Mikulas Patocka @ 2018-04-20 20:56 UTC (permalink / raw)
To: Michal Hocko
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
virtualization, linux-mm, edumazet, bhutchings, Andrew Morton,
David Miller, Vlastimil Babka
In-Reply-To: <20180420134901.GB17484@dhcp22.suse.cz>
On Fri, 20 Apr 2018, Michal Hocko wrote:
> On Fri 20-04-18 06:41:36, Matthew Wilcox wrote:
> > On Fri, Apr 20, 2018 at 03:08:52PM +0200, Michal Hocko wrote:
> > > > In order to detect these bugs reliably I submit this patch that changes
> > > > kvmalloc to always use vmalloc if CONFIG_DEBUG_VM is turned on.
> > >
> > > No way. This is just wrong! First of all, you will explode most likely
> > > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > > enabled quite often.
> >
> > I think it'll still suit Mikulas' debugging needs if we always use
> > vmalloc for sizes above PAGE_SIZE?
>
> Even if that was the case then this doesn't sounds like CONFIG_DEBUG_VM
> material. We do not want a completely different behavior when the config
>
> --
> Michal Hocko
> SUSE Labs
I'm not arguing that it must be turned on exactly by CONFIG_DEBUG_VM. It
may be turned on some other option that is enabled in debug kernels
(CONFIG_DEBUG_SG may be a better option, because you'll get meaningful
stracktraces from DMA API then).
> is enabled. If we really need some better fallback testing coverage
> then the fault injection, as suggested by Vlastimil, sounds much more
> reasonable to me
People who test kernels will install the kernel-debug package, reboot to
the debug kernel and run their testsuites. They won't turn on magic
options in debugfs or use some hideous kernel commandline arguments. If
the kvmalloc test isn't in the debug kernel, then the testing crew won't
test it - that's it.
Mikulas
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Matthew Wilcox @ 2018-04-20 21:02 UTC (permalink / raw)
To: Mikulas Patocka
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Michal Hocko,
linux-mm, edumazet, bhutchings, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804201635180.25408@file01.intranet.prod.int.rdu2.redhat.com>
On Fri, Apr 20, 2018 at 04:54:53PM -0400, Mikulas Patocka wrote:
> On Fri, 20 Apr 2018, Michal Hocko wrote:
> > No way. This is just wrong! First of all, you will explode most likely
> > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > enabled quite often.
>
> You're an evil person who doesn't want to fix bugs.
Steady on. There's no need for that. Michal isn't evil. Please
apologise.
> You refused to fix vmalloc(GFP_NOIO) misbehavior a year ago (did you make
> some progress with it since that time?) and you refuse to fix kvmalloc
> misuses.
I understand you're frustrated, but this is not the way to get the problems
fixed.
> I tried this patch on text-only virtual machine and /proc/vmallocinfo
> shows 614kB more memory. I tried it on a desktop machine with the chrome
> browser open and /proc/vmallocinfo space is increased by 7MB. So no - this
> won't exhaust memory and kill the machine.
This is good data, thank you for providing it.
> Arguing that this increases memory consumption is as bogus as arguing that
> CONFIG_LOCKDEP increses memory consumption. No one is forcing you to
> enable CONFIG_LOCKDEP and no one is forcing you to enable this kvmalloc
> test too.
I think there's a real problem which is that CONFIG_DEBUG_VM is too broad.
It inserts code in a *lot* of places, some of which is quite expensive.
We would do better to split it into more granular pieces ... although
an explosion of configuration options isn't great either. Maybe just
CONFIG_DEBUG_VM and CONFIG_DEBUG_VM_EXPENSIVE.
Michal may be wrong, but he's not evil.
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Mikulas Patocka @ 2018-04-20 21:21 UTC (permalink / raw)
To: Matthew Wilcox
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Michal Hocko,
linux-mm, edumazet, bhutchings, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <20180420210200.GH10788@bombadil.infradead.org>
On Fri, 20 Apr 2018, Matthew Wilcox wrote:
> On Fri, Apr 20, 2018 at 04:54:53PM -0400, Mikulas Patocka wrote:
> > On Fri, 20 Apr 2018, Michal Hocko wrote:
> > > No way. This is just wrong! First of all, you will explode most likely
> > > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > > enabled quite often.
> >
> > You're an evil person who doesn't want to fix bugs.
>
> Steady on. There's no need for that. Michal isn't evil. Please
> apologise.
I see this attitude from Michal again and again.
He didn't want to fix vmalloc(GFP_NOIO), he didn't want to fix alloc_pages
sleeping when __GFP_NORETRY is used. So what should I say? Fix them and
you won't be evil :-)
(he could also fix the oom killer, so that it is triggered when
free_memory+cache+free_swap goes beyond a threshold and not when you loop
too long in the allocator)
> > You refused to fix vmalloc(GFP_NOIO) misbehavior a year ago (did you make
> > some progress with it since that time?) and you refuse to fix kvmalloc
> > misuses.
>
> I understand you're frustrated, but this is not the way to get the problems
> fixed.
>
> > I tried this patch on text-only virtual machine and /proc/vmallocinfo
> > shows 614kB more memory. I tried it on a desktop machine with the chrome
> > browser open and /proc/vmallocinfo space is increased by 7MB. So no - this
> > won't exhaust memory and kill the machine.
>
> This is good data, thank you for providing it.
>
> > Arguing that this increases memory consumption is as bogus as arguing that
> > CONFIG_LOCKDEP increses memory consumption. No one is forcing you to
> > enable CONFIG_LOCKDEP and no one is forcing you to enable this kvmalloc
> > test too.
>
> I think there's a real problem which is that CONFIG_DEBUG_VM is too broad.
> It inserts code in a *lot* of places, some of which is quite expensive.
> We would do better to split it into more granular pieces ... although
> an explosion of configuration options isn't great either. Maybe just
> CONFIG_DEBUG_VM and CONFIG_DEBUG_VM_EXPENSIVE.
>
> Michal may be wrong, but he's not evil.
I already said that we can change it from CONFIG_DEBUG_VM to
CONFIG_DEBUG_SG - or to whatever other option you may want, just to make
sure that it is enabled in distro debug kernels by default.
Mikulas
^ permalink raw reply
* Re: [PATCH 1/6] virtio_console: don't tie bufs to a vq
From: Greg Kroah-Hartman @ 2018-04-21 7:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Arnd Bergmann, Amit Shah, linux-kernel, stable, virtualization
In-Reply-To: <1524248223-393618-2-git-send-email-mst@redhat.com>
On Fri, Apr 20, 2018 at 09:18:01PM +0300, Michael S. Tsirkin wrote:
> an allocated buffer doesn't need to be tied to a vq -
> only vq->vdev is ever used. Pass the function the
> just what it needs - the vdev.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/char/virtio_console.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 468f061..3e56f32 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -422,7 +422,7 @@ static void reclaim_dma_bufs(void)
> }
> }
>
> -static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
> +static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size,
> int pages)
> {
> struct port_buffer *buf;
> @@ -445,16 +445,16 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size,
> return buf;
> }
>
> - if (is_rproc_serial(vq->vdev)) {
> + if (is_rproc_serial(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.
> */
> - if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent)
> + if (!vdev->dev.parent || !vdev->dev.parent->parent)
> goto free_buf;
> - buf->dev = vq->vdev->dev.parent->parent;
> + buf->dev = vdev->dev.parent->parent;
>
> /* Increase device refcnt to avoid freeing it */
> get_device(buf->dev);
> @@ -838,7 +838,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
>
> count = min((size_t)(32 * 1024), count);
>
> - buf = alloc_buf(port->out_vq, count, 0);
> + buf = alloc_buf(port->portdev->vdev, count, 0);
> if (!buf)
> return -ENOMEM;
>
> @@ -957,7 +957,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
> if (ret < 0)
> goto error_out;
>
> - buf = alloc_buf(port->out_vq, 0, pipe->nrbufs);
> + buf = alloc_buf(port->portdev->vdev, 0, pipe->nrbufs);
> if (!buf) {
> ret = -ENOMEM;
> goto error_out;
> @@ -1374,7 +1374,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
>
> nr_added_bufs = 0;
> do {
> - buf = alloc_buf(vq, PAGE_SIZE, 0);
> + buf = alloc_buf(vq->vdev, PAGE_SIZE, 0);
> if (!buf)
> break;
>
> --
> MST
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Matthew Wilcox @ 2018-04-21 14:47 UTC (permalink / raw)
To: Mikulas Patocka
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Michal Hocko,
linux-mm, edumazet, bhutchings, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804201704580.25408@file01.intranet.prod.int.rdu2.redhat.com>
On Fri, Apr 20, 2018 at 05:21:26PM -0400, Mikulas Patocka wrote:
> On Fri, 20 Apr 2018, Matthew Wilcox wrote:
> > On Fri, Apr 20, 2018 at 04:54:53PM -0400, Mikulas Patocka wrote:
> > > On Fri, 20 Apr 2018, Michal Hocko wrote:
> > > > No way. This is just wrong! First of all, you will explode most likely
> > > > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > > > enabled quite often.
> > >
> > > You're an evil person who doesn't want to fix bugs.
> >
> > Steady on. There's no need for that. Michal isn't evil. Please
> > apologise.
>
> I see this attitude from Michal again and again.
Fine; then *say that*. I also see Michal saying "No" a lot. Sometimes
I agree with him, sometimes I don't. I think he genuinely wants the best
code in the kernel, and saying "No" is part of it.
> He didn't want to fix vmalloc(GFP_NOIO)
I don't remember that conversation, so I don't know whether I agree with
his reasoning or not. But we are supposed to be moving away from GFP_NOIO
towards marking regions with memalloc_noio_save() / restore. If you do
that, you won't need vmalloc(GFP_NOIO).
> he didn't want to fix alloc_pages sleeping when __GFP_NORETRY is used.
The GFP flags are a mess, still.
> So what should I say? Fix them and
> you won't be evil :-)
No, you should reserve calling somebody evil for truly evil things.
> (he could also fix the oom killer, so that it is triggered when
> free_memory+cache+free_swap goes beyond a threshold and not when you loop
> too long in the allocator)
... that also doesn't make somebody evil.
> I already said that we can change it from CONFIG_DEBUG_VM to
> CONFIG_DEBUG_SG - or to whatever other option you may want, just to make
> sure that it is enabled in distro debug kernels by default.
Yes, and I think that's the right idea. So send a v2 and ignore the
replies that are clearly relating to an earlier version of the patch.
Not everybody reads every mail in the thread before responding to one they
find interesting. Yes, ideally, one would, but sometimes one doesn't.
^ permalink raw reply
* Re: virtio remoteproc device
From: Anup Patel @ 2018-04-22 4:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Ohad Ben-Cohen, virtualization, linux-remoteproc, Bjorn Andersson
In-Reply-To: <20180420194321-mutt-send-email-mst@kernel.org>
On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Hello!
> I note the following in the serial console:
>
> if (is_rproc_serial(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.
> */
> if (!vdev->dev.parent || !vdev->dev.parent->parent)
> goto free_buf;
> buf->dev = 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);
> }
>
> Added here:
> commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> Date: Fri Dec 14 14:40:51 2012 +1030
>
> virtio_console: Add support for remoteproc serial
>
>
> I am not familiar with rproc so I have a question:
> why is it required to use coherent memory here,
> and why through a grandparent device?
I faced similar issue when I tried VirtIO RPMSG bus over
VirtIO MMIO transport.
Here's my fix for VirtIO RPMSG bus driver:
https://patchwork.kernel.org/patch/10155145/
Regards,
Anup
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
From: Michal Hocko @ 2018-04-22 13:03 UTC (permalink / raw)
To: Matthew Wilcox
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, virtualization,
linux-mm, edumazet, Mikulas Patocka, bhutchings, Andrew Morton,
David Miller, Vlastimil Babka
In-Reply-To: <20180421144757.GC14610@bombadil.infradead.org>
On Sat 21-04-18 07:47:57, Matthew Wilcox wrote:
> On Fri, Apr 20, 2018 at 05:21:26PM -0400, Mikulas Patocka wrote:
> > On Fri, 20 Apr 2018, Matthew Wilcox wrote:
> > > On Fri, Apr 20, 2018 at 04:54:53PM -0400, Mikulas Patocka wrote:
> > > > On Fri, 20 Apr 2018, Michal Hocko wrote:
> > > > > No way. This is just wrong! First of all, you will explode most likely
> > > > > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > > > > enabled quite often.
> > > >
> > > > You're an evil person who doesn't want to fix bugs.
> > >
> > > Steady on. There's no need for that. Michal isn't evil. Please
> > > apologise.
> >
> > I see this attitude from Michal again and again.
>
> Fine; then *say that*. I also see Michal saying "No" a lot. Sometimes
> I agree with him, sometimes I don't. I think he genuinely wants the best
> code in the kernel, and saying "No" is part of it.
Exactly! We have a lot of legacy herritage and random semantic because
we were too eager to merge stuff. I think it is time to stop that and
think twice before merging someothing. If you call that evil then I am
fine to be evil.
> > He didn't want to fix vmalloc(GFP_NOIO)
>
> I don't remember that conversation, so I don't know whether I agree with
> his reasoning or not. But we are supposed to be moving away from GFP_NOIO
> towards marking regions with memalloc_noio_save() / restore. If you do
> that, you won't need vmalloc(GFP_NOIO).
It was basically to detect GFP_NOIO context _inside_ vmalloc and use the
scope API to enforce it there. Does it solve potential problems? Yes it
does. Does it solve any existing report, no I am not aware of any. Is
it a good fix longterm? Absolutely no, because the scope API should be
used _at the place_ where the scope starts rather than a random utility
function. If we are going the easier way now, we will never teach users
to use the API properly. And I am willing to risk to keep a broken
code which we have for years rather than allow a random hack that will
seemingly fix it.
Btw. I was pretty much explicit with this reasoning when rejecting the
patch. Do you still call that evil?
> > he didn't want to fix alloc_pages sleeping when __GFP_NORETRY is used.
>
> The GFP flags are a mess, still.
I do not remember that one but __GFP_NORETRY is _allowed_ to sleep. And
yes I do _agree_ gfp flags are a mess which is really hard to get fixed
because they are lacking a good design from the very beginning. Fixing
some of those issues today is a completely PITA.
> > So what should I say? Fix them and
> > you won't be evil :-)
>
> No, you should reserve calling somebody evil for truly evil things.
>
> > (he could also fix the oom killer, so that it is triggered when
> > free_memory+cache+free_swap goes beyond a threshold and not when you loop
> > too long in the allocator)
>
> ... that also doesn't make somebody evil.
And again, it is way much more easier to claim that something will get
fixed when the reality is much more complicated. I've tried to explain
those risks as well.
> > I already said that we can change it from CONFIG_DEBUG_VM to
> > CONFIG_DEBUG_SG - or to whatever other option you may want, just to make
> > sure that it is enabled in distro debug kernels by default.
>
> Yes, and I think that's the right idea. So send a v2 and ignore the
> replies that are clearly relating to an earlier version of the patch.
> Not everybody reads every mail in the thread before responding to one they
> find interesting. Yes, ideally, one would, but sometimes one doesn't.
And look here. This is yet another ad-hoc idea. We have many users of
kvmalloc which have no relation to SG, yet you are going to control
their behavior by CONFIG_DEBUG_SG? No way! (yeah evil again)
Really, we have a fault injection framework and this sounds like
something to hook in there.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: kbuild test robot @ 2018-04-22 15:41 UTC (permalink / raw)
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, kbuild-all, sridhar.samudrala, davem
In-Reply-To: <1524188524-28411-5-git-send-email-sridhar.samudrala@intel.com>
[-- Attachment #1: Type: text/plain, Size: 7695 bytes --]
Hi Sridhar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc1]
[cannot apply to net-next/master next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180422-210557
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from drivers/net/hyperv/netvsc_drv.c:46:0:
include/net/failover.h:67:1: error: expected identifier or '(' before '{' token
{
^
include/net/failover.h:78:16: warning: 'struct pfailover' declared inside parameter list will not be visible outside of this definition or declaration
struct pfailover **pfailover);
^~~~~~~~~
include/net/failover.h:79:1: error: expected identifier or '(' before '{' token
{
^
drivers/net/hyperv/netvsc_drv.c: In function 'netvsc_probe':
>> drivers/net/hyperv/netvsc_drv.c:2020:5: error: passing argument 3 of 'failover_register' from incompatible pointer type [-Werror=incompatible-pointer-types]
&net_device_ctx->failover);
^
In file included from drivers/net/hyperv/netvsc_drv.c:46:0:
include/net/failover.h:77:5: note: expected 'struct pfailover **' but argument is of type 'struct failover **'
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^~~~~~~~~~~~~~~~~
drivers/net/hyperv/netvsc_drv.c: At top level:
include/net/failover.h:65:5: warning: 'failover_create' declared 'static' but never defined [-Wunused-function]
int failover_create(struct net_device *standby_dev,
^~~~~~~~~~~~~~~
>> include/net/failover.h:77:5: warning: 'failover_register' used but never defined
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from drivers/net//hyperv/netvsc_drv.c:46:0:
include/net/failover.h:67:1: error: expected identifier or '(' before '{' token
{
^
include/net/failover.h:78:16: warning: 'struct pfailover' declared inside parameter list will not be visible outside of this definition or declaration
struct pfailover **pfailover);
^~~~~~~~~
include/net/failover.h:79:1: error: expected identifier or '(' before '{' token
{
^
drivers/net//hyperv/netvsc_drv.c: In function 'netvsc_probe':
drivers/net//hyperv/netvsc_drv.c:2020:5: error: passing argument 3 of 'failover_register' from incompatible pointer type [-Werror=incompatible-pointer-types]
&net_device_ctx->failover);
^
In file included from drivers/net//hyperv/netvsc_drv.c:46:0:
include/net/failover.h:77:5: note: expected 'struct pfailover **' but argument is of type 'struct failover **'
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^~~~~~~~~~~~~~~~~
drivers/net//hyperv/netvsc_drv.c: At top level:
include/net/failover.h:65:5: warning: 'failover_create' declared 'static' but never defined [-Wunused-function]
int failover_create(struct net_device *standby_dev,
^~~~~~~~~~~~~~~
>> include/net/failover.h:77:5: warning: 'failover_register' used but never defined
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/failover_register +2020 drivers/net/hyperv/netvsc_drv.c
1929
1930 static int netvsc_probe(struct hv_device *dev,
1931 const struct hv_vmbus_device_id *dev_id)
1932 {
1933 struct net_device *net = NULL;
1934 struct net_device_context *net_device_ctx;
1935 struct netvsc_device_info device_info;
1936 struct netvsc_device *nvdev;
1937 int ret = -ENOMEM;
1938
1939 net = alloc_etherdev_mq(sizeof(struct net_device_context),
1940 VRSS_CHANNEL_MAX);
1941 if (!net)
1942 goto no_net;
1943
1944 netif_carrier_off(net);
1945
1946 netvsc_init_settings(net);
1947
1948 net_device_ctx = netdev_priv(net);
1949 net_device_ctx->device_ctx = dev;
1950 net_device_ctx->msg_enable = netif_msg_init(debug, default_msg);
1951 if (netif_msg_probe(net_device_ctx))
1952 netdev_dbg(net, "netvsc msg_enable: %d\n",
1953 net_device_ctx->msg_enable);
1954
1955 hv_set_drvdata(dev, net);
1956
1957 INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_link_change);
1958
1959 spin_lock_init(&net_device_ctx->lock);
1960 INIT_LIST_HEAD(&net_device_ctx->reconfig_events);
1961 INIT_DELAYED_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup);
1962
1963 net_device_ctx->vf_stats
1964 = netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
1965 if (!net_device_ctx->vf_stats)
1966 goto no_stats;
1967
1968 net->netdev_ops = &device_ops;
1969 net->ethtool_ops = ðtool_ops;
1970 SET_NETDEV_DEV(net, &dev->device);
1971
1972 /* We always need headroom for rndis header */
1973 net->needed_headroom = RNDIS_AND_PPI_SIZE;
1974
1975 /* Initialize the number of queues to be 1, we may change it if more
1976 * channels are offered later.
1977 */
1978 netif_set_real_num_tx_queues(net, 1);
1979 netif_set_real_num_rx_queues(net, 1);
1980
1981 /* Notify the netvsc driver of the new device */
1982 memset(&device_info, 0, sizeof(device_info));
1983 device_info.num_chn = VRSS_CHANNEL_DEFAULT;
1984 device_info.send_sections = NETVSC_DEFAULT_TX;
1985 device_info.send_section_size = NETVSC_SEND_SECTION_SIZE;
1986 device_info.recv_sections = NETVSC_DEFAULT_RX;
1987 device_info.recv_section_size = NETVSC_RECV_SECTION_SIZE;
1988
1989 nvdev = rndis_filter_device_add(dev, &device_info);
1990 if (IS_ERR(nvdev)) {
1991 ret = PTR_ERR(nvdev);
1992 netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
1993 goto rndis_failed;
1994 }
1995
1996 memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
1997
1998 /* hw_features computed in rndis_netdev_set_hwcaps() */
1999 net->features = net->hw_features |
2000 NETIF_F_HIGHDMA | NETIF_F_SG |
2001 NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
2002 net->vlan_features = net->features;
2003
2004 netdev_lockdep_set_classes(net);
2005
2006 /* MTU range: 68 - 1500 or 65521 */
2007 net->min_mtu = NETVSC_MTU_MIN;
2008 if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)
2009 net->max_mtu = NETVSC_MTU - ETH_HLEN;
2010 else
2011 net->max_mtu = ETH_DATA_LEN;
2012
2013 ret = register_netdev(net);
2014 if (ret != 0) {
2015 pr_err("Unable to register netdev.\n");
2016 goto register_failed;
2017 }
2018
2019 ret = failover_register(net, &netvsc_failover_ops,
> 2020 &net_device_ctx->failover);
2021 if (ret != 0)
2022 goto err_failover;
2023
2024 return ret;
2025
2026 err_failover:
2027 unregister_netdev(net);
2028 register_failed:
2029 rndis_filter_device_remove(dev, nvdev);
2030 rndis_failed:
2031 free_percpu(net_device_ctx->vf_stats);
2032 no_stats:
2033 hv_set_drvdata(dev, NULL);
2034 free_netdev(net);
2035 no_net:
2036 return ret;
2037 }
2038
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39371 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v7 net-next 3/4] virtio_net: Extend virtio to use VF datapath when available
From: kbuild test robot @ 2018-04-22 15:41 UTC (permalink / raw)
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, kbuild-all, sridhar.samudrala, davem
In-Reply-To: <1524188524-28411-4-git-send-email-sridhar.samudrala@intel.com>
[-- Attachment #1: Type: text/plain, Size: 3173 bytes --]
Hi Sridhar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc1]
[cannot apply to net-next/master next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180422-210557
config: i386-randconfig-x071-201816 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers//net/virtio_net.c:37:0:
>> include/net/failover.h:67:1: error: expected identifier or '(' before '{' token
{
^
>> include/net/failover.h:78:16: warning: 'struct pfailover' declared inside parameter list will not be visible outside of this definition or declaration
struct pfailover **pfailover);
^~~~~~~~~
include/net/failover.h:79:1: error: expected identifier or '(' before '{' token
{
^
>> include/net/failover.h:65:5: warning: 'failover_create' used but never defined
int failover_create(struct net_device *standby_dev,
^~~~~~~~~~~~~~~
include/net/failover.h:77:5: warning: 'failover_register' declared 'static' but never defined [-Wunused-function]
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^~~~~~~~~~~~~~~~~
vim +67 include/net/failover.h
891cac68 Sridhar Samudrala 2018-04-19 63
891cac68 Sridhar Samudrala 2018-04-19 64 static inline
891cac68 Sridhar Samudrala 2018-04-19 @65 int failover_create(struct net_device *standby_dev,
891cac68 Sridhar Samudrala 2018-04-19 66 struct failover **pfailover);
891cac68 Sridhar Samudrala 2018-04-19 @67 {
891cac68 Sridhar Samudrala 2018-04-19 68 return 0;
891cac68 Sridhar Samudrala 2018-04-19 69 }
891cac68 Sridhar Samudrala 2018-04-19 70
891cac68 Sridhar Samudrala 2018-04-19 71 static inline
891cac68 Sridhar Samudrala 2018-04-19 72 void failover_destroy(struct failover *failover)
891cac68 Sridhar Samudrala 2018-04-19 73 {
891cac68 Sridhar Samudrala 2018-04-19 74 }
891cac68 Sridhar Samudrala 2018-04-19 75
891cac68 Sridhar Samudrala 2018-04-19 76 static inline
891cac68 Sridhar Samudrala 2018-04-19 77 int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
891cac68 Sridhar Samudrala 2018-04-19 @78 struct pfailover **pfailover);
891cac68 Sridhar Samudrala 2018-04-19 79 {
891cac68 Sridhar Samudrala 2018-04-19 80 return 0;
891cac68 Sridhar Samudrala 2018-04-19 81 }
891cac68 Sridhar Samudrala 2018-04-19 82
:::::: The code at line 67 was first introduced by commit
:::::: 891cac68e9283c9561d0097af77e7189b9752719 net: Introduce generic failover module
:::::: TO: Sridhar Samudrala <sridhar.samudrala@intel.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27072 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v7 net-next 3/4] virtio_net: Extend virtio to use VF datapath when available
From: kbuild test robot @ 2018-04-22 15:41 UTC (permalink / raw)
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, kbuild-all, sridhar.samudrala, davem
In-Reply-To: <1524188524-28411-4-git-send-email-sridhar.samudrala@intel.com>
[-- Attachment #1: Type: text/plain, Size: 3204 bytes --]
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on v4.17-rc1]
[cannot apply to net-next/master next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180422-210557
config: i386-randconfig-a1-04221429 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from drivers//net/virtio_net.c:37:0:
include/net/failover.h:67:1: error: expected identifier or '(' before '{' token
{
^
>> include/net/failover.h:78:16: warning: 'struct pfailover' declared inside parameter list
struct pfailover **pfailover);
^
>> include/net/failover.h:78:16: warning: its scope is only this definition or declaration, which is probably not what you want
include/net/failover.h:79:1: error: expected identifier or '(' before '{' token
{
^
include/net/failover.h:65:5: warning: 'failover_create' used but never defined
int failover_create(struct net_device *standby_dev,
^
include/net/failover.h:77:5: warning: 'failover_register' declared 'static' but never defined [-Wunused-function]
int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
^
vim +78 include/net/failover.h
891cac68 Sridhar Samudrala 2018-04-19 63
891cac68 Sridhar Samudrala 2018-04-19 64 static inline
891cac68 Sridhar Samudrala 2018-04-19 65 int failover_create(struct net_device *standby_dev,
891cac68 Sridhar Samudrala 2018-04-19 66 struct failover **pfailover);
891cac68 Sridhar Samudrala 2018-04-19 @67 {
891cac68 Sridhar Samudrala 2018-04-19 68 return 0;
891cac68 Sridhar Samudrala 2018-04-19 69 }
891cac68 Sridhar Samudrala 2018-04-19 70
891cac68 Sridhar Samudrala 2018-04-19 71 static inline
891cac68 Sridhar Samudrala 2018-04-19 72 void failover_destroy(struct failover *failover)
891cac68 Sridhar Samudrala 2018-04-19 73 {
891cac68 Sridhar Samudrala 2018-04-19 74 }
891cac68 Sridhar Samudrala 2018-04-19 75
891cac68 Sridhar Samudrala 2018-04-19 76 static inline
891cac68 Sridhar Samudrala 2018-04-19 77 int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
891cac68 Sridhar Samudrala 2018-04-19 @78 struct pfailover **pfailover);
891cac68 Sridhar Samudrala 2018-04-19 79 {
891cac68 Sridhar Samudrala 2018-04-19 80 return 0;
891cac68 Sridhar Samudrala 2018-04-19 81 }
891cac68 Sridhar Samudrala 2018-04-19 82
:::::: The code at line 78 was first introduced by commit
:::::: 891cac68e9283c9561d0097af77e7189b9752719 net: Introduce generic failover module
:::::: TO: Sridhar Samudrala <sridhar.samudrala@intel.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29503 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v7 net-next 2/4] net: Introduce generic failover module
From: Michael S. Tsirkin @ 2018-04-22 17:06 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: alexander.h.duyck, virtio-dev, jiri, kubakici, netdev,
virtualization, loseweigh, davem
In-Reply-To: <1524188524-28411-3-git-send-email-sridhar.samudrala@intel.com>
On Thu, Apr 19, 2018 at 06:42:02PM -0700, Sridhar Samudrala wrote:
> +#if IS_ENABLED(CONFIG_NET_FAILOVER)
> +
> +int failover_create(struct net_device *standby_dev,
> + struct failover **pfailover);
Should we rename all these structs net_failover?
It's possible to extend the concept to storage I think.
> +void failover_destroy(struct failover *failover);
> +
> +int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
> + struct failover **pfailover);
> +void failover_unregister(struct failover *failover);
> +
> +int failover_slave_unregister(struct net_device *slave_dev);
> +
> +#else
> +
> +static inline
> +int failover_create(struct net_device *standby_dev,
> + struct failover **pfailover);
> +{
> + return 0;
Does this make callers do something sane?
Shouldn't these return an error?
> +}
> +
> +static inline
> +void failover_destroy(struct failover *failover)
> +{
> +}
> +
> +static inline
> +int failover_register(struct net_device *standby_dev, struct failover_ops *ops,
> + struct pfailover **pfailover);
> +{
> + return 0;
> +}
struct pfailover seems like a typo.
> +
> +static inline
> +void failover_unregister(struct failover *failover)
> +{
> +}
> +
> +static inline
> +int failover_slave_unregister(struct net_device *slave_dev)
> +{
> + return 0;
> +}
Does anyone test return value of unregister?
should this be void?
> +
> +#endif
> +
> +#endif /* _NET_FAILOVER_H */
^ permalink raw reply
* Re: [PATCH v7 net-next 2/4] net: Introduce generic failover module
From: kbuild test robot @ 2018-04-22 18:29 UTC (permalink / raw)
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, kbuild-all, sridhar.samudrala, davem
In-Reply-To: <1524188524-28411-3-git-send-email-sridhar.samudrala@intel.com>
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on v4.17-rc1]
[cannot apply to net-next/master next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180422-210557
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/core/failover.c:99:36: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct net_device *dev @@ got struct net_devicestruct net_device *dev @@
net/core/failover.c:99:36: expected struct net_device *dev
net/core/failover.c:99:36: got struct net_device [noderef] <asn:4>*standby_dev
net/core/failover.c:102:36: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct net_device *dev @@ got struct net_devicestruct net_device *dev @@
net/core/failover.c:102:36: expected struct net_device *dev
net/core/failover.c:102:36: got struct net_device [noderef] <asn:4>*primary_dev
>> net/core/failover.c:468:12: sparse: context imbalance in 'failover_select_queue' - wrong count at exit
vim +99 net/core/failover.c
58
59 static int failover_slave_join(struct net_device *slave_dev,
60 struct net_device *failover_dev,
61 struct failover_ops *failover_ops)
62 {
63 struct failover_info *finfo;
64 int err, orig_mtu;
65 bool standby;
66
67 if (failover_ops) {
68 if (!failover_ops->slave_join)
69 return -EINVAL;
70
71 return failover_ops->slave_join(slave_dev, failover_dev);
72 }
73
74 if (netif_running(failover_dev)) {
75 err = dev_open(slave_dev);
76 if (err && (err != -EBUSY)) {
77 netdev_err(failover_dev, "Opening slave %s failed err:%d\n",
78 slave_dev->name, err);
79 goto err_dev_open;
80 }
81 }
82
83 /* Align MTU of slave with failover dev */
84 orig_mtu = slave_dev->mtu;
85 err = dev_set_mtu(slave_dev, failover_dev->mtu);
86 if (err) {
87 netdev_err(failover_dev, "unable to change mtu of %s to %u register failed\n",
88 slave_dev->name, failover_dev->mtu);
89 goto err_set_mtu;
90 }
91
92 finfo = netdev_priv(failover_dev);
93 standby = (slave_dev->dev.parent == failover_dev->dev.parent);
94
95 dev_hold(slave_dev);
96
97 if (standby) {
98 rcu_assign_pointer(finfo->standby_dev, slave_dev);
> 99 dev_get_stats(finfo->standby_dev, &finfo->standby_stats);
100 } else {
101 rcu_assign_pointer(finfo->primary_dev, slave_dev);
102 dev_get_stats(finfo->primary_dev, &finfo->primary_stats);
103 failover_dev->min_mtu = slave_dev->min_mtu;
104 failover_dev->max_mtu = slave_dev->max_mtu;
105 }
106
107 netdev_info(failover_dev, "failover slave:%s joined\n",
108 slave_dev->name);
109
110 return 0;
111
112 err_set_mtu:
113 dev_close(slave_dev);
114 err_dev_open:
115 return err;
116 }
117
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH] drm/virtio: fix vq wait_event condition
From: Dave Airlie @ 2018-04-23 1:24 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, stable, open list:VIRTIO GPU DRIVER,
dri-devel, amagloire
In-Reply-To: <20180420072228.uorb3h7snx4io6xs@sirius.home.kraxel.org>
On 20 April 2018 at 17:22, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 11:59:04AM +0200, Gerd Hoffmann wrote:
>> Wait until we have enough space in the virt queue to actually queue up
>> our request. Avoids the guest spinning in case we have a non-zero
>> amount of free entries but not enough for the request.
>
> Ping airlied, can you please either pick it up or review so I can commit
> myself?
Just in case it got lost from my phone,
Reviewed-by: Dave Airlie <airlied@redhat.com>
>
> thanks,
> Gerd
>
>> Cc: stable@vger.kernel.org
>> Reported-by: Alain Magloire <amagloire@blackberry.com>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> drivers/gpu/drm/virtio/virtgpu_vq.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
>> index 48e4f1df6e..020070d483 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
>> @@ -293,7 +293,7 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
>> ret = virtqueue_add_sgs(vq, sgs, outcnt, incnt, vbuf, GFP_ATOMIC);
>> if (ret == -ENOSPC) {
>> spin_unlock(&vgdev->ctrlq.qlock);
>> - wait_event(vgdev->ctrlq.ack_queue, vq->num_free);
>> + wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= outcnt + incnt);
>> spin_lock(&vgdev->ctrlq.qlock);
>> goto retry;
>> } else {
>> @@ -368,7 +368,7 @@ static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
>> ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC);
>> if (ret == -ENOSPC) {
>> spin_unlock(&vgdev->cursorq.qlock);
>> - wait_event(vgdev->cursorq.ack_queue, vq->num_free);
>> + wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt);
>> spin_lock(&vgdev->cursorq.qlock);
>> goto retry;
>> } else {
>> --
>> 2.9.3
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [RFC V3 PATCH 0/8] Packed ring for vhost
From: Jason Wang @ 2018-04-23 5:34 UTC (permalink / raw)
To: mst, jasowang; +Cc: kvm, netdev, linux-kernel, virtualization, wexu
Hi all:
This RFC implement packed ring layout. The code were tested with
Tiwei's RFC V2 a thttps://lkml.org/lkml/2018/4/1/48. Some fixups and
tweaks were needed on top of Tiwei's code to make it run. TCP stream
and pktgen does not show obvious difference compared with split ring.
Changes from V2:
- do not use & in checking desc_event_flags
- off should be most significant bit
- remove the workaround of mergeable buffer for dpdk prototype
- id should be in the last descriptor in the chain
- keep _F_WRITE for write descriptor when adding used
- device flags updating should use ADDR_USED type
- return error on unexpected unavail descriptor in a chain
- return false in vhost_ve_avail_empty is descriptor is available
- track last seen avail_wrap_counter
- correctly examine available descriptor in get_indirect_packed()
- vhost_idx_diff should return u16 instead of bool
Changes from V1:
- Refactor vhost used elem code to avoid open coding on used elem
- Event suppression support (compile test only).
- Indirect descriptor support (compile test only).
- Zerocopy support.
- vIOMMU support.
- SCSI/VSOCK support (compile test only).
- Fix several bugs
For simplicity, I don't implement batching or other optimizations.
Please review.
Jason Wang (8):
vhost: move get_rx_bufs to vhost.c
vhost: hide used ring layout from device
vhost: do not use vring_used_elem
vhost_net: do not explicitly manipulate vhost_used_elem
vhost: vhost_put_user() can accept metadata type
virtio: introduce packed ring defines
vhost: packed ring support
vhost: event suppression for packed ring
drivers/vhost/net.c | 136 ++----
drivers/vhost/scsi.c | 62 +--
drivers/vhost/vhost.c | 824 ++++++++++++++++++++++++++++++++++---
drivers/vhost/vhost.h | 47 ++-
drivers/vhost/vsock.c | 42 +-
include/uapi/linux/virtio_config.h | 9 +
include/uapi/linux/virtio_ring.h | 32 ++
7 files changed, 926 insertions(+), 226 deletions(-)
--
2.7.4
^ 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