* Re: [RFC] [ver3 PATCH 3/6] virtio_net: virtio_net driver changes
From: Sasha Levin @ 2011-11-18 16:18 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Krishna Kumar, kvm, mst, netdev, virtualization, davem
In-Reply-To: <1321630839.2885.117.camel@deadeye>
On Fri, 2011-11-18 at 15:40 +0000, Ben Hutchings wrote:
> On Fri, 2011-11-18 at 08:24 +0200, Sasha Levin wrote:
> > On Fri, 2011-11-18 at 01:08 +0000, Ben Hutchings wrote:
> > > On Fri, 2011-11-11 at 18:34 +0530, Krishna Kumar wrote:
> > > > Changes for multiqueue virtio_net driver.
> > > [...]
> > > > @@ -677,25 +730,35 @@ static struct rtnl_link_stats64 *virtnet
> > > > {
> > > > struct virtnet_info *vi = netdev_priv(dev);
> > > > int cpu;
> > > > - unsigned int start;
> > > >
> > > > for_each_possible_cpu(cpu) {
> > > > - struct virtnet_stats __percpu *stats
> > > > - = per_cpu_ptr(vi->stats, cpu);
> > > > - u64 tpackets, tbytes, rpackets, rbytes;
> > > > -
> > > > - do {
> > > > - start = u64_stats_fetch_begin(&stats->syncp);
> > > > - tpackets = stats->tx_packets;
> > > > - tbytes = stats->tx_bytes;
> > > > - rpackets = stats->rx_packets;
> > > > - rbytes = stats->rx_bytes;
> > > > - } while (u64_stats_fetch_retry(&stats->syncp, start));
> > > > -
> > > > - tot->rx_packets += rpackets;
> > > > - tot->tx_packets += tpackets;
> > > > - tot->rx_bytes += rbytes;
> > > > - tot->tx_bytes += tbytes;
> > > > + int qpair;
> > > > +
> > > > + for (qpair = 0; qpair < vi->num_queue_pairs; qpair++) {
> > > > + struct virtnet_send_stats __percpu *tx_stat;
> > > > + struct virtnet_recv_stats __percpu *rx_stat;
> > >
> > > While you're at it, you can drop the per-CPU stats and make them only
> > > per-queue. There is unlikely to be any benefit in maintaining them
> > > per-CPU while receive and transmit processing is serialised per-queue.
> >
> > It allows you to update stats without a lock.
>
> But you'll already be holding a lock related to the queue.
Right, but now you're holding a queue lock just when playing with the
queue, we don't hold it when we process the data - which is when we
usually need to update stats.
> > Whats the benefit of having them per queue?
>
> It should save some memory (and a little time when summing stats, though
> that's unlikely to matter much).
>
> The important thing is that splitting up stats per-CPU *and* per-queue
> is a waste.
>
> Ben.
>
--
Sasha.
^ permalink raw reply
* Re: [RFC] [ver3 PATCH 3/6] virtio_net: virtio_net driver changes
From: Ben Hutchings @ 2011-11-18 15:40 UTC (permalink / raw)
To: Sasha Levin; +Cc: Krishna Kumar, kvm, mst, netdev, virtualization, davem
In-Reply-To: <1321597481.8010.19.camel@lappy>
On Fri, 2011-11-18 at 08:24 +0200, Sasha Levin wrote:
> On Fri, 2011-11-18 at 01:08 +0000, Ben Hutchings wrote:
> > On Fri, 2011-11-11 at 18:34 +0530, Krishna Kumar wrote:
> > > Changes for multiqueue virtio_net driver.
> > [...]
> > > @@ -677,25 +730,35 @@ static struct rtnl_link_stats64 *virtnet
> > > {
> > > struct virtnet_info *vi = netdev_priv(dev);
> > > int cpu;
> > > - unsigned int start;
> > >
> > > for_each_possible_cpu(cpu) {
> > > - struct virtnet_stats __percpu *stats
> > > - = per_cpu_ptr(vi->stats, cpu);
> > > - u64 tpackets, tbytes, rpackets, rbytes;
> > > -
> > > - do {
> > > - start = u64_stats_fetch_begin(&stats->syncp);
> > > - tpackets = stats->tx_packets;
> > > - tbytes = stats->tx_bytes;
> > > - rpackets = stats->rx_packets;
> > > - rbytes = stats->rx_bytes;
> > > - } while (u64_stats_fetch_retry(&stats->syncp, start));
> > > -
> > > - tot->rx_packets += rpackets;
> > > - tot->tx_packets += tpackets;
> > > - tot->rx_bytes += rbytes;
> > > - tot->tx_bytes += tbytes;
> > > + int qpair;
> > > +
> > > + for (qpair = 0; qpair < vi->num_queue_pairs; qpair++) {
> > > + struct virtnet_send_stats __percpu *tx_stat;
> > > + struct virtnet_recv_stats __percpu *rx_stat;
> >
> > While you're at it, you can drop the per-CPU stats and make them only
> > per-queue. There is unlikely to be any benefit in maintaining them
> > per-CPU while receive and transmit processing is serialised per-queue.
>
> It allows you to update stats without a lock.
But you'll already be holding a lock related to the queue.
> Whats the benefit of having them per queue?
It should save some memory (and a little time when summing stats, though
that's unlikely to matter much).
The important thing is that splitting up stats per-CPU *and* per-queue
is a waste.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] vhost-net: Acquire device lock when releasing device
From: Sasha Levin @ 2011-11-18 9:19 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, virtualization, Sasha Levin, kvm, Michael S. Tsirkin
Device lock should be held when releasing a device, and specifically
when calling vhost_dev_cleanup(). Otherwise, RCU complains about it:
[ 2025.642835] ===============================
[ 2025.643838] [ INFO: suspicious RCU usage. ]
[ 2025.645182] -------------------------------
[ 2025.645927] drivers/vhost/vhost.c:475 suspicious rcu_dereference_protected() usage!
[ 2025.647329]
[ 2025.647330] other info that might help us debug this:
[ 2025.647331]
[ 2025.649042]
[ 2025.649043] rcu_scheduler_active = 1, debug_locks = 1
[ 2025.650235] no locks held by trinity/21042.
[ 2025.650971]
[ 2025.650972] stack backtrace:
[ 2025.651789] Pid: 21042, comm: trinity Not tainted 3.2.0-rc2-sasha-00057-ga9098b3 #5
[ 2025.653342] Call Trace:
[ 2025.653792] [<ffffffff810b4a6a>] lockdep_rcu_suspicious+0xaf/0xb9
[ 2025.654916] [<ffffffff818d4c2c>] vhost_dev_cleanup+0x342/0x3ac
[ 2025.655985] [<ffffffff818d4f18>] vhost_net_release+0x48/0x7f
[ 2025.657247] [<ffffffff811416e3>] fput+0x11e/0x1dc
[ 2025.658091] [<ffffffff8113f1ec>] filp_close+0x6e/0x79
[ 2025.658964] [<ffffffff81089ed7>] put_files_struct+0xcc/0x196
[ 2025.659971] [<ffffffff8108a034>] exit_files+0x46/0x4f
[ 2025.660865] [<ffffffff8108a716>] do_exit+0x264/0x75c
[ 2025.662201] [<ffffffff8113f490>] ? fsnotify_modify+0x60/0x68
[ 2025.663260] [<ffffffff81bbdbca>] ? sysret_check+0x2e/0x69
[ 2025.664269] [<ffffffff8108acc1>] do_group_exit+0x83/0xb1
[ 2025.665448] [<ffffffff8108ad01>] sys_exit_group+0x12/0x16
[ 2025.666396] [<ffffffff81bbdb92>] system_call_fastpath+0x16/0x1b
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: kvm@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
drivers/vhost/net.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 882a51f..c9be601 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -586,6 +586,7 @@ static int vhost_net_release(struct inode *inode, struct file *f)
struct socket *tx_sock;
struct socket *rx_sock;
+ mutex_lock(&n->dev.mutex);
vhost_net_stop(n, &tx_sock, &rx_sock);
vhost_net_flush(n);
vhost_dev_cleanup(&n->dev);
@@ -596,6 +597,7 @@ static int vhost_net_release(struct inode *inode, struct file *f)
/* We do an extra flush before freeing memory,
* since jobs can re-queue themselves. */
vhost_net_flush(n);
+ mutex_unlock(&n->dev.mutex);
kfree(n);
return 0;
}
--
1.7.8.rc1
^ permalink raw reply related
* Re: [RFC] [ver3 PATCH 3/6] virtio_net: virtio_net driver changes
From: Sasha Levin @ 2011-11-18 6:24 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Krishna Kumar, kvm, mst, netdev, virtualization, davem
In-Reply-To: <1321578488.2749.66.camel@bwh-desktop>
On Fri, 2011-11-18 at 01:08 +0000, Ben Hutchings wrote:
> On Fri, 2011-11-11 at 18:34 +0530, Krishna Kumar wrote:
> > Changes for multiqueue virtio_net driver.
> [...]
> > @@ -677,25 +730,35 @@ static struct rtnl_link_stats64 *virtnet
> > {
> > struct virtnet_info *vi = netdev_priv(dev);
> > int cpu;
> > - unsigned int start;
> >
> > for_each_possible_cpu(cpu) {
> > - struct virtnet_stats __percpu *stats
> > - = per_cpu_ptr(vi->stats, cpu);
> > - u64 tpackets, tbytes, rpackets, rbytes;
> > -
> > - do {
> > - start = u64_stats_fetch_begin(&stats->syncp);
> > - tpackets = stats->tx_packets;
> > - tbytes = stats->tx_bytes;
> > - rpackets = stats->rx_packets;
> > - rbytes = stats->rx_bytes;
> > - } while (u64_stats_fetch_retry(&stats->syncp, start));
> > -
> > - tot->rx_packets += rpackets;
> > - tot->tx_packets += tpackets;
> > - tot->rx_bytes += rbytes;
> > - tot->tx_bytes += tbytes;
> > + int qpair;
> > +
> > + for (qpair = 0; qpair < vi->num_queue_pairs; qpair++) {
> > + struct virtnet_send_stats __percpu *tx_stat;
> > + struct virtnet_recv_stats __percpu *rx_stat;
>
> While you're at it, you can drop the per-CPU stats and make them only
> per-queue. There is unlikely to be any benefit in maintaining them
> per-CPU while receive and transmit processing is serialised per-queue.
It allows you to update stats without a lock.
Whats the benefit of having them per queue?
--
Sasha.
^ permalink raw reply
* RE: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of the staging area
From: KY Srinivasan @ 2011-11-18 4:53 UTC (permalink / raw)
To: James Bottomley
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
linux-scsi@vger.kernel.org, ohering@suse.com, hch@infradead.org
In-Reply-To: <1321554393.3041.77.camel@dabdike.int.hansenpartnership.com>
> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Thursday, November 17, 2011 1:27 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; linux-
> scsi@vger.kernel.org; ohering@suse.com; hch@infradead.org
> Subject: Re: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of the
> staging area
>
> On Tue, 2011-11-08 at 10:13 -0800, K. Y. Srinivasan wrote:
> > The storage driver (storvsc_drv.c) handles all block storage devices
> > assigned to Linux guests hosted on Hyper-V. This driver has been in the
> > staging tree for a while and this patch moves it out of the staging area.
> > As per Greg's recommendation, this patch makes no changes to the staging/hv
> > directory. Once the driver moves out of staging, we will cleanup the
> > staging/hv directory.
> >
> > This patch includes all the patches that I have sent against the staging/hv
> > tree to address the comments I have gotten to date on this storage driver.
>
> First comment is that it would have been easier to see the individual
> patches for comment before you committed them.
I am not sure if the patches have been committed yet. All patches were sent
to various mailing lists and you were copied as well. In the future, I will include
the scsi mailing list in the set of lists I include for the staging patches.
>
> The way you did mempool isn't entirely right: the problem is that to
> prevent a memory to I/O deadlock we need to ensure forward progress on
> the drain device. Just having 64 commands available to the host doesn't
> necessarily achieve this because LUN1 could consume them all and starve
> LUN0 which is the drain device leading to the deadlock, so the mempool
> really needs to be per device using slave_alloc.
I will do this per LUN.
>
> +static int storvsc_device_alloc(struct scsi_device *sdevice)
> +{
> + /*
> + * This enables luns to be located sparsely. Otherwise, we may not
> + * discovered them.
> + */
> + sdevice->sdev_bflags |= BLIST_SPARSELUN | BLIST_LARGELUN;
> + return 0;
> +}
>
> Looks bogus ... this should happen automatically for SCSI-3 devices ...
> unless your hypervisor has some strange (and wrong) identification? I
> really think you want to use SCSI-3 because it will do report LUN
> scanning, which consumes far fewer resources.
I will see if I can clean this up.
>
> I still think you need to disable clustering and junk the bvec merge
> function. Your object seems to be to accumulate in page size multiples
> (and not aggregate over this) ... that's what clustering is designed to
> do.
As part of addressing your first round of comments, I experimented with your
suggestions and I could not get rid of the code that does the bounce buffer handling.
I could generate I/O patterns that would require bounce buffer handling with your
suggestions in place.
Regards,
K. Y
^ permalink raw reply
* Re: [RFC] [ver3 PATCH 3/6] virtio_net: virtio_net driver changes
From: Ben Hutchings @ 2011-11-18 1:08 UTC (permalink / raw)
To: Krishna Kumar; +Cc: kvm, mst, netdev, virtualization, davem
In-Reply-To: <20111111130420.9878.19729.sendpatchset@krkumar2.in.ibm.com>
On Fri, 2011-11-11 at 18:34 +0530, Krishna Kumar wrote:
> Changes for multiqueue virtio_net driver.
[...]
> @@ -677,25 +730,35 @@ static struct rtnl_link_stats64 *virtnet
> {
> struct virtnet_info *vi = netdev_priv(dev);
> int cpu;
> - unsigned int start;
>
> for_each_possible_cpu(cpu) {
> - struct virtnet_stats __percpu *stats
> - = per_cpu_ptr(vi->stats, cpu);
> - u64 tpackets, tbytes, rpackets, rbytes;
> -
> - do {
> - start = u64_stats_fetch_begin(&stats->syncp);
> - tpackets = stats->tx_packets;
> - tbytes = stats->tx_bytes;
> - rpackets = stats->rx_packets;
> - rbytes = stats->rx_bytes;
> - } while (u64_stats_fetch_retry(&stats->syncp, start));
> -
> - tot->rx_packets += rpackets;
> - tot->tx_packets += tpackets;
> - tot->rx_bytes += rbytes;
> - tot->tx_bytes += tbytes;
> + int qpair;
> +
> + for (qpair = 0; qpair < vi->num_queue_pairs; qpair++) {
> + struct virtnet_send_stats __percpu *tx_stat;
> + struct virtnet_recv_stats __percpu *rx_stat;
While you're at it, you can drop the per-CPU stats and make them only
per-queue. There is unlikely to be any benefit in maintaining them
per-CPU while receive and transmit processing is serialised per-queue.
[...]
> +static int invoke_find_vqs(struct virtnet_info *vi)
> +{
> + vq_callback_t **callbacks;
> + struct virtqueue **vqs;
> + int ret = -ENOMEM;
> + int i, total_vqs;
> + char **names;
> +
> + /*
> + * We expect 1 RX virtqueue followed by 1 TX virtqueue, followed
> + * by the same 'vi->num_queue_pairs-1' more times, and optionally
> + * one control virtqueue.
> + */
> + total_vqs = vi->num_queue_pairs * 2 +
> + virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ);
> +
> + /* Allocate space for find_vqs parameters */
> + vqs = kmalloc(total_vqs * sizeof(*vqs), GFP_KERNEL);
> + callbacks = kmalloc(total_vqs * sizeof(*callbacks), GFP_KERNEL);
> + names = kmalloc(total_vqs * sizeof(*names), GFP_KERNEL);
> + if (!vqs || !callbacks || !names)
> + goto err;
> +
> + /* Allocate/initialize parameters for recv virtqueues */
> + for (i = 0; i < vi->num_queue_pairs * 2; i += 2) {
> + callbacks[i] = skb_recv_done;
> + names[i] = kasprintf(GFP_KERNEL, "input.%d", i / 2);
> + if (!names[i])
> + goto err;
> + }
> +
> + /* Allocate/initialize parameters for send virtqueues */
> + for (i = 1; i < vi->num_queue_pairs * 2; i += 2) {
> + callbacks[i] = skb_xmit_done;
> + names[i] = kasprintf(GFP_KERNEL, "output.%d", i / 2);
> + if (!names[i])
> + goto err;
> + }
[...]
The RX and TX interrupt names for a multiqueue device should follow the
formats "<device>-rx-<index>" and "<device>-tx-<index>".
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio_console: Fix locking of vtermno.
From: Amit Shah @ 2011-11-17 19:09 UTC (permalink / raw)
To: Rusty Russell
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk,
Benjamin Herrenschmidt, Greg Kroah-Hartman, Miche Baker-Harvey,
linux-kernel, virtualization, Anton Blanchard, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <877h37qo5z.fsf@rustcorp.com.au>
On (Fri) 11 Nov 2011 [14:57:20], Rusty Russell wrote:
> On Tue, 08 Nov 2011 13:44:58 -0800, Miche Baker-Harvey <miche@google.com> wrote:
> > Some modifications of vtermno were not done under the spinlock.
> >
> > Moved assignment from vtermno and increment of vtermno together,
> > putting both under the spinlock. Revert vtermno on failure.
> >
> > Signed-off-by: Miche Baker-Harvey <miche@google.com>
>
> Does it matter? It's normal not to lock in a function called
> "init_XXX", since it's not exposed yet.
>
> Or is it?
Slight misnomer, I suppose.
We do this init_console_port() as part of add_port() if the port is a
console port. Should it be named 'mark_console_port()'? Dunno,
doesn't sound like the right name. init fits closest.
Amit
^ permalink raw reply
* Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
From: Miche Baker-Harvey @ 2011-11-17 18:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk,
Benjamin Herrenschmidt, Greg Kroah-Hartman, linux-kernel,
virtualization, Anton Blanchard, Amit Shah, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <874nybqo0o.fsf@rustcorp.com.au>
Rusty, Michael, Stephen, et al,
Thanks for your comments on these patches.
For what I'm trying to do, all three patches are necessary, but maybe
I'm going about it the wrong way. Your input would be appreciated.
I'm in no way claiming that these patches are "right", just that it's
working for me, and that what's in the current pool is not.
What I'm trying to do is:
On X86,
under KVM,
start a virtio console device,
with multiple ports on the device,
at least one of which is also a console (as well as ttyS0).
(Eventually, we want to be able to add virtio console ports on the
fly, and to have multiple virtio console ports be consoles.)
When all three of the patches are in place, this works great. (By
great, I mean that getty's start up on all of ttyS0, hvc0 and hvc1,
and console output goes to ttyS0 and to hvc0.
"who" shows three users: ttyS0, hvc0, and hvc1.
"cat /proc/consoles" shows both ttyS0 and hvc0.
I can use all three getty's, and console output really does appear on
both the consoles.
Based on Rusty's comments, I tried removing each of the patches
individually. Here's what happens today. I've seen other failure modes
depending on what precisely I'm passing the guest.
There's three patches:
1/3 "fix locking of vtermno"
2/3 "enforce one-time initialization with hvc_init
"3/3 "use separate struct console * for each console"
If I remove the "fix locking of vtermno", I only get one virtio
console terminal. "who" shows the ttyS0 and the hvc0, and I can log
into the gettys on both. I don't get the second virtio console getty.
Interestingly, hvc0 shows up in /proc/consoles twice, and in fact the
console output is dumped twice to hvc0 (as you'd expect from looking
at printk.c, each line appears twice, followed by the next line.)
If I remove the "enforce one-time initialization with hvc_init" patch,
which makes sure only a single thread is doing the hvc_init, and gates
anyone from continuing until it has completed, I get different
failures, including hangs, and dereferences of NULL pointers.
If I remove the "use separate struct console * for each console"patch,
what I'm seeing now is that while all of ttyS0, hvc0, and hvc1 are
present with gettys running on them, of the three, only ttyS0 is a
console.
I also re-tried each patch alone:
For either the "fix locking of vtermno" or "use separate struct
console * for each console" patches (in other words, not the "enforce
one-time initialization with hvc_init" patch), I panic during boot
with a null dereference.
For just the "enforce one-time initialization with hvc_init" patch, I
see all of hvc0, hvc1, and ttyS0 in a "who" listing, but only one
getty is available with an hvc. Also, an echo to *either* "hvc0"
or"hvc1" appears on the single hvc getty. Also, no virtio console
appears in the /proc/consoles list.
Michael, I agree with you about the comment and naming of the mutex
around hvc_init.
Stephen, the duplicate messages are not something I'm seeing. It's
probably the case that there are two "consoles" (registered in printk)
that have the same tty as their target. I've added a call to
register_console in hvc_alloc, and I'm guessing that something in your
system is making your tty register as a console in hvc_instantiate,
and then it's re-registered in hvc_alloc, but I really am not sure. We
don't have earlyprintk support, so the register_console in
hvc_instantiate is never called.
Miche
^ permalink raw reply
* Re: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of the staging area
From: James Bottomley @ 2011-11-17 18:26 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, linux-scsi, ohering,
hch
In-Reply-To: <1320776028-29568-1-git-send-email-kys@microsoft.com>
On Tue, 2011-11-08 at 10:13 -0800, K. Y. Srinivasan wrote:
> The storage driver (storvsc_drv.c) handles all block storage devices
> assigned to Linux guests hosted on Hyper-V. This driver has been in the
> staging tree for a while and this patch moves it out of the staging area.
> As per Greg's recommendation, this patch makes no changes to the staging/hv
> directory. Once the driver moves out of staging, we will cleanup the
> staging/hv directory.
>
> This patch includes all the patches that I have sent against the staging/hv
> tree to address the comments I have gotten to date on this storage driver.
First comment is that it would have been easier to see the individual
patches for comment before you committed them.
The way you did mempool isn't entirely right: the problem is that to
prevent a memory to I/O deadlock we need to ensure forward progress on
the drain device. Just having 64 commands available to the host doesn't
necessarily achieve this because LUN1 could consume them all and starve
LUN0 which is the drain device leading to the deadlock, so the mempool
really needs to be per device using slave_alloc.
+static int storvsc_device_alloc(struct scsi_device *sdevice)
+{
+ /*
+ * This enables luns to be located sparsely. Otherwise, we may not
+ * discovered them.
+ */
+ sdevice->sdev_bflags |= BLIST_SPARSELUN | BLIST_LARGELUN;
+ return 0;
+}
Looks bogus ... this should happen automatically for SCSI-3 devices ...
unless your hypervisor has some strange (and wrong) identification? I
really think you want to use SCSI-3 because it will do report LUN
scanning, which consumes far fewer resources.
I still think you need to disable clustering and junk the bvec merge
function. Your object seems to be to accumulate in page size multiples
(and not aggregate over this) ... that's what clustering is designed to
do.
James
^ permalink raw reply
* [PATCH] virtio-pci: make reset operation safer
From: Michael S. Tsirkin @ 2011-11-17 15:41 UTC (permalink / raw)
To: Amit Shah; +Cc: virtualization, linux-kernel, Michael S. Tsirkin
virtio pci device reset actually just does an I/O
write, which in PCI is really posted, that is it
can complete on CPU before the device has received it.
Further, interrupts might have been pending on
another CPU, so device callback might get invoked after reset.
This conflicts with how drivers use reset, which is typically:
reset
unregister
a callback running after reset completed can race with
unregister, potentially leading to use after free bugs.
Fix by flushing out the write, and flushing pending interrupts.
This assumes that device is never reset from
its vq/config callbacks, or in parallel with being
added/removed, document this assumption.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Tested with virtio-net only.
Rusty, a bugfix, so 3.2 material?
drivers/virtio/virtio_pci.c | 18 ++++++++++++++++++
include/linux/virtio_config.h | 2 ++
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index d242fcc..cb1090e 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -321,11 +321,29 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
iowrite8(status, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
}
+/* wait for pending irq handlers */
+static void vp_synchronize_vectors(struct virtio_device *vdev)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ int i;
+
+ if (vp_dev->intx_enabled)
+ synchronize_irq(vp_dev->pci_dev->irq);
+
+ for (i = 0; i < vp_dev->msix_vectors; ++i)
+ synchronize_irq(vp_dev->msix_entries[i].vector);
+}
+
static void vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
/* 0 status means a reset. */
iowrite8(0, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
+ /* Flush out the status write, and flush in device writes,
+ * including MSi-X interrupts, if any. */
+ ioread8(vp_dev->ioaddr + VIRTIO_PCI_STATUS);
+ /* Flush pending VQ/configuration callbacks. */
+ vp_synchronize_vectors(vdev);
}
/* the notify function used when creating a virt queue */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index add4790..e9e72bd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -85,6 +85,8 @@
* @reset: reset the device
* vdev: the virtio device
* After this, status and feature negotiation must be done again
+ * Device must not be reset from its vq/config callbacks, or in
+ * parallel with being added/removed.
* @find_vqs: find virtqueues and instantiate them.
* vdev: the virtio_device
* nvqs: the number of virtqueues to find
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCH v3 11/11] virtio: balloon: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 13:03 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <20111117122920.GB2873@amit-x200.redhat.com>
On Thu, Nov 17, 2011 at 05:59:20PM +0530, Amit Shah wrote:
> On (Thu) 17 Nov 2011 [14:25:08], Michael S. Tsirkin wrote:
> > On Thu, Nov 17, 2011 at 05:27:42PM +0530, Amit Shah wrote:
> > > Delete the vqs on the freeze callback to prepare for hibernation.
> > > Re-create the vqs in the restore callback to resume normal function.
> > >
> > > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > > ---
> > > drivers/virtio/virtio_balloon.c | 20 ++++++++++++++++++++
> > > 1 files changed, 20 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > index 1ff3cf4..90149d1 100644
> > > --- a/drivers/virtio/virtio_balloon.c
> > > +++ b/drivers/virtio/virtio_balloon.c
> > > @@ -363,6 +363,22 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
> > > kfree(vb);
> > > }
> > >
> > > +#ifdef CONFIG_PM
> > > +static int virtballoon_freeze(struct virtio_device *vdev)
> > > +{
> > > + /* Now we reset the device so we can clean up the queues. */
> > > + vdev->config->reset(vdev);
> > > +
> >
> > This is a weird thing to do. We normally delete vqs then reset.
>
> No, all the drivers first reset, then delete.
Interesting. I note that for PCI, reset actually just did an I/O
write, which in PCI is really posted, that is it
can complete on CPU before the device has received it.
Further, interrupts might have been pending on the CPU,
so device might get used after reset.
Patch coming.
--
MST
^ permalink raw reply
* [PATCH v2] virtio-mmio: Devices parameter parsing
From: Pawel Moll @ 2011-11-17 12:42 UTC (permalink / raw)
To: Rusty Russell, linux-kernel, virtualization
Cc: Peter Maydell, Sasha Levin, Pawel Moll
In-Reply-To: <1321467222.3137.417.camel@hornet.cambridge.arm.com>
This patch adds an option to instantiate guest virtio-mmio devices
basing on a kernel command line (or module) parameter, for example:
virtio_mmio.devices=0x100@0x100b0000:48,1K@0x1001e000:74
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
drivers/virtio/Kconfig | 31 +++++++
drivers/virtio/virtio_mmio.c | 181 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 211 insertions(+), 1 deletions(-)
Hi Rusty,
This version adds the "first_id" parameter I mentioned yesterday,
but still using single charp parameter instead of the _cb version.
And unless you are really (_really_) against it, I'd rather see
this variant.
Cheers!
Paweł
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 816ed08..00f2c82 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -46,4 +46,35 @@ config VIRTIO_BALLOON
If unsure, say N.
+config VIRTIO_MMIO_CMDLINE_DEVICES
+ bool "Memory mapped virtio devices parameter parsing"
+ depends on VIRTIO_MMIO
+ ---help---
+ Allow virtio-mmio devices instantiation via the kernel command line
+ or module parameter. Be aware that using incorrect parameters (base
+ address in particular) can crash your system - you have been warned.
+
+ The format for the parameter is as follows:
+
+ [virtio_mmio.]devices=<device>[<delim><device>]
+
+ where:
+ <device> := <size>@<baseaddr>:<irq>
+ <delim> := ',' or ';'
+ <size> := size (can use standard suffixes like K or M)
+ <baseaddr> := physical base address
+ <irq> := interrupt number (as passed to request_irq())
+
+ Example kernel command line parameter:
+
+ virtio_mmio.devices=0x100@0x100b0000:48,1K@0x1001e000:74
+
+ This will register platform devices virtio_mmio.<id>, where <id>
+ values are consecutive integer numbers starting from 0 by default.
+ The first id value can be changed with "first_id" parameter:
+
+ [virtio_mmio.]first_id=<id>
+
+ If unsure, say 'N'.
+
endmenu
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index acc5e43..05b39c1 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -6,6 +6,55 @@
* This module allows virtio devices to be used over a virtual, memory mapped
* platform device.
*
+ * The guest device(s) may be instantiated in one of three equivalent ways:
+ *
+ * 1. Static platform device in board's code, eg.:
+ *
+ * static struct platform_device v2m_virtio_device = {
+ * .name = "virtio-mmio",
+ * .id = -1,
+ * .num_resources = 2,
+ * .resource = (struct resource []) {
+ * {
+ * .start = 0x1001e000,
+ * .end = 0x1001e0ff,
+ * .flags = IORESOURCE_MEM,
+ * }, {
+ * .start = 42 + 32,
+ * .end = 42 + 32,
+ * .flags = IORESOURCE_IRQ,
+ * },
+ * }
+ * };
+ *
+ * 2. Device Tree node, eg.:
+ *
+ * virtio_block@1e000 {
+ * compatible = "virtio,mmio";
+ * reg = <0x1e000 0x100>;
+ * interrupts = <42>;
+ * }
+ *
+ * 3. Kernel module (or command line) parameter:
+ *
+ * [virtio_mmio.]devices=<device>[<delim><device>]
+ * where:
+ * <device> := <size>@<baseaddr>:<irq>
+ * <delim> := ',' or ';'
+ * <size> := size (can use standard suffixes like K or M)
+ * <baseaddr> := physical base address
+ * <irq> := interrupt number (as passed to request_irq())
+ * eg.:
+ * virtio_mmio.devices=0x100@0x100b0000:48,1K@0x1001e000:74
+ *
+ * This will register platform devices virtio_mmio.<id>, where <id>
+ * values are consecutive integer numbers starting from 0 by default.
+ * The first id value can be changed with "first_id" parameter:
+ *
+ * [virtio_mmio.]first_id=<id>
+ *
+ *
+ *
* Registers layout (all 32-bit wide):
*
* offset d. name description
@@ -42,6 +91,8 @@
* See the COPYING file in the top-level directory.
*/
+#define pr_fmt(fmt) "virtio-mmio: " fmt
+
#include <linux/highmem.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -443,6 +494,130 @@ static int __devexit virtio_mmio_remove(struct platform_device *pdev)
+/* Devices list parameter */
+
+#if defined(CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES)
+
+static char *virtio_mmio_cmdline_devices;
+module_param_named(devices, virtio_mmio_cmdline_devices, charp, 0);
+
+static int virtio_mmio_cmdline_id;
+module_param_named(first_id, virtio_mmio_cmdline_id, int, 0);
+
+static struct device virtio_mmio_cmdline_parent = {
+ .init_name = "virtio-mmio-cmdline",
+};
+
+static int virtio_mmio_register_cmdline_devices(void)
+{
+ int err;
+ char *device;
+ char *token = NULL;
+
+ err = device_register(&virtio_mmio_cmdline_parent);
+ if (err) {
+ pr_err("Failed to register %s!\n",
+ virtio_mmio_cmdline_parent.init_name);
+ return err;
+ }
+
+ /* Split colon-or-semicolon-separated devices */
+ while ((device = strsep(&virtio_mmio_cmdline_devices, ",;")) != NULL) {
+ struct resource resources[] = {
+ {
+ .flags = IORESOURCE_IRQ,
+ }, {
+ .flags = IORESOURCE_MEM,
+ }
+ };
+ char *size, *base;
+ unsigned long long val;
+
+ if (!*device)
+ continue;
+
+ kfree(token);
+ token = kstrdup(device, GFP_KERNEL);
+
+ /* Split memory and IRQ resources */
+ base = strsep(&token, ":");
+ if (base == token || !token || !*token) {
+ pr_err("No IRQ in '%s'!\n", device);
+ continue;
+ }
+
+ /* Get IRQ */
+ if (kstrtoull(token, 0, &val) != 0) {
+ pr_err("Wrong IRQ in '%s'!\n", device);
+ continue;
+ }
+ resources[0].start = val;
+ resources[0].end = val;
+
+ /* Split base address and size */
+ size = strsep(&base, "@");
+ if (size == base || !base || !*base) {
+ pr_err("No base in '%s'!\n", device);
+ continue;
+ }
+
+ /* Get base address */
+ if (kstrtoull(base, 0, &val) != 0) {
+ pr_err("Wrong base in '%s'!\n", device);
+ continue;
+ }
+ resources[1].start = val;
+ resources[1].end = val;
+
+ /* Get size */
+ resources[1].end += memparse(size, &token) - 1;
+ if (size == token || *token) {
+ pr_err("Wrong size in '%s'!\n", device);
+ continue;
+ }
+
+ pr_info("Registering device %d at 0x%x-0x%x, IRQ %u.\n",
+ virtio_mmio_cmdline_id, resources[1].start,
+ resources[1].end, resources[0].start);
+
+ platform_device_register_resndata(&virtio_mmio_cmdline_parent,
+ "virtio-mmio", virtio_mmio_cmdline_id++,
+ resources, ARRAY_SIZE(resources), NULL, 0);
+ }
+
+ kfree(token);
+
+ return 0;
+}
+
+static int virtio_mmio_unregister_cmdline_device(struct device *dev,
+ void *data)
+{
+ platform_device_unregister(to_platform_device(dev));
+
+ return 0;
+}
+
+static void virtio_mmio_unregister_cmdline_devices(void)
+{
+ device_for_each_child(&virtio_mmio_cmdline_parent, NULL,
+ virtio_mmio_unregister_cmdline_device);
+ device_unregister(&virtio_mmio_cmdline_parent);
+}
+
+#else
+
+static int virtio_mmio_register_cmdline_devices(void)
+{
+ return 0;
+}
+
+static void virtio_mmio_unregister_cmdline_devices(void)
+{
+}
+
+#endif
+
/* Platform driver */
static struct of_device_id virtio_mmio_match[] = {
@@ -463,11 +638,15 @@ static struct platform_driver virtio_mmio_driver = {
static int __init virtio_mmio_init(void)
{
- return platform_driver_register(&virtio_mmio_driver);
+ int err = virtio_mmio_register_cmdline_devices();
+
+ return err ? err : platform_driver_register(&virtio_mmio_driver);
}
static void __exit virtio_mmio_exit(void)
{
+ virtio_mmio_unregister_cmdline_devices();
+
platform_driver_unregister(&virtio_mmio_driver);
}
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH v3 11/11] virtio: balloon: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:36 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <20111117122920.GB2873@amit-x200.redhat.com>
On Thu, Nov 17, 2011 at 05:59:20PM +0530, Amit Shah wrote:
> On (Thu) 17 Nov 2011 [14:25:08], Michael S. Tsirkin wrote:
> > On Thu, Nov 17, 2011 at 05:27:42PM +0530, Amit Shah wrote:
> > > Delete the vqs on the freeze callback to prepare for hibernation.
> > > Re-create the vqs in the restore callback to resume normal function.
> > >
> > > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > > ---
> > > drivers/virtio/virtio_balloon.c | 20 ++++++++++++++++++++
> > > 1 files changed, 20 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > index 1ff3cf4..90149d1 100644
> > > --- a/drivers/virtio/virtio_balloon.c
> > > +++ b/drivers/virtio/virtio_balloon.c
> > > @@ -363,6 +363,22 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
> > > kfree(vb);
> > > }
> > >
> > > +#ifdef CONFIG_PM
> > > +static int virtballoon_freeze(struct virtio_device *vdev)
> > > +{
> > > + /* Now we reset the device so we can clean up the queues. */
> > > + vdev->config->reset(vdev);
> > > +
> >
> > This is a weird thing to do. We normally delete vqs then reset.
>
> No, all the drivers first reset, then delete.
BTW, reset does not flush callbacks at the moment.
It probably should.
> > For example, I don't know whether we guarantee what happens
> > to MSI-X vectors assigned meanwhile if you reset.
> > IIRC qemu doesn't use MSI-X for the balloon, but nothing
> > prevents it.
> >
> > > + vdev->config->del_vqs(vdev);
> >
> > What prevents requests being submitted at this point?
> > I see all kind of handling of freezing in the balloon thread ...
> > maybe that gives us some guarantees, but if yes
> > it makes sense to add a comment to point this out.
>
> Once reset, the host won't write anything to the vqs anyway..
>
> Amit
But thread might have already been scheduled, handling
previous requests.
--
MST
^ permalink raw reply
* Re: [PATCH v3 09/11] virtio: net: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:33 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <20111117122706.GA2873@amit-x200.redhat.com>
On Thu, Nov 17, 2011 at 05:57:06PM +0530, Amit Shah wrote:
> On (Thu) 17 Nov 2011 [14:19:09], Michael S. Tsirkin wrote:
> > On Thu, Nov 17, 2011 at 05:27:40PM +0530, Amit Shah wrote:
>
> > > +#ifdef CONFIG_PM
> > > +static int virtnet_freeze(struct virtio_device *vdev)
> > > +{
> > > + struct virtnet_info *vi = vdev->priv;
> > > +
> > > + netif_device_detach(vi->dev);
> > > + remove_vq_common(vi);
> >
> > This stops TX in progress, if any, but not RX
> > which might use the RX VQ. Then remove_vq_common
> > might delete this VQ while it's still in use.
> >
> > So I think we need to call something like napi_disable.
> > However, the subtle twist is that we need to call that
> > *after interrupts have been disabled*.
> > Otherwise we might schedule another napi callback.
>
> resetting the vqs will mean the host won't pass us any data in the
> vqs. Plus we're removing the vqs altogether. Also, we're disabling
> the pci device in virtio_pci.c, so all of these actions will take
> care of that, isn't it?
I don't think so.
> In addition, once the vqs are taken off, there's no chance for any
> other rx to happen, so napi_disable() after plugging off vqs doesn't
> make sense.
>
> Amit
Yes but napi might have been scheduled before remove_vq_common was
called, and run afterwards.
--
MST
^ permalink raw reply
* Re: [PATCH v3 04/11] virtio: console: Add freeze and restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:30 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <44ccf96a9f34b7b9a838af00b2abc97796cbdfe5.1321530505.git.amit.shah@redhat.com>
On Thu, Nov 17, 2011 at 05:27:35PM +0530, Amit Shah wrote:
> Remove all vqs and associated buffers in the freeze callback which
> prepares us to go into hibernation state. On restore, re-create all the
> vqs and populate the input vqs with buffers to get to the pre-hibernate
> state.
>
> Note: Any outstanding unconsumed buffers are discarded; which means
> there's a possibility of data loss in case the host or the guest didn't
> consume any data already present in the vqs. This can be addressed in a
> later patch series, perhaps in virtio common code.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/char/virtio_console.c | 58 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index e14f5aa..fd2fd6f 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1844,6 +1844,60 @@ static unsigned int features[] = {
> VIRTIO_CONSOLE_F_MULTIPORT,
> };
>
> +#ifdef CONFIG_PM
> +static int virtcons_freeze(struct virtio_device *vdev)
> +{
> + struct ports_device *portdev;
> + struct port *port;
> +
> + portdev = vdev->priv;
> +
> + vdev->config->reset(vdev);
This does a reset but that's not a guarantee that
interrupt is not running on another CPU.
> +
> + cancel_work_sync(&portdev->control_work);
And then work can get scheduled after this point.
> + remove_controlq_data(portdev);
And after this point this will lead to a use after free.
> +
> + list_for_each_entry(port, &portdev->ports, list) {
> + /*
> + * We'll ask the host later if the new invocation has
> + * the port opened or closed.
> + */
> + port->host_connected = false;
> + remove_port_data(port);
> + }
> + remove_vqs(portdev);
> +
> + return 0;
> +}
> +
> +static int virtcons_restore(struct virtio_device *vdev)
> +{
> + struct ports_device *portdev;
> + struct port *port;
> + int ret;
> +
> + portdev = vdev->priv;
> +
> + ret = init_vqs(portdev);
> + if (ret)
> + return ret;
> +
> + if (use_multiport(portdev))
> + fill_queue(portdev->c_ivq, &portdev->cvq_lock);
> +
> + list_for_each_entry(port, &portdev->ports, list) {
> + port->in_vq = portdev->in_vqs[port->id];
> + port->out_vq = portdev->out_vqs[port->id];
> +
> + fill_queue(port->in_vq, &port->inbuf_lock);
> +
> + /* Get port open/close status on the host */
> + send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
> + }
> + return 0;
> +}
> +#endif
> +
> static struct virtio_driver virtio_console = {
> .feature_table = features,
> .feature_table_size = ARRAY_SIZE(features),
> @@ -1853,6 +1907,10 @@ static struct virtio_driver virtio_console = {
> .probe = virtcons_probe,
> .remove = virtcons_remove,
> .config_changed = config_intr,
> +#ifdef CONFIG_PM
> + .freeze = virtcons_freeze,
> + .restore = virtcons_restore,
> +#endif
> };
>
> static int __init init(void)
> --
> 1.7.7.1
^ permalink raw reply
* Re: [PATCH v3 11/11] virtio: balloon: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-11-17 12:29 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <20111117122508.GB19682@redhat.com>
On (Thu) 17 Nov 2011 [14:25:08], Michael S. Tsirkin wrote:
> On Thu, Nov 17, 2011 at 05:27:42PM +0530, Amit Shah wrote:
> > Delete the vqs on the freeze callback to prepare for hibernation.
> > Re-create the vqs in the restore callback to resume normal function.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> > drivers/virtio/virtio_balloon.c | 20 ++++++++++++++++++++
> > 1 files changed, 20 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 1ff3cf4..90149d1 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -363,6 +363,22 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
> > kfree(vb);
> > }
> >
> > +#ifdef CONFIG_PM
> > +static int virtballoon_freeze(struct virtio_device *vdev)
> > +{
> > + /* Now we reset the device so we can clean up the queues. */
> > + vdev->config->reset(vdev);
> > +
>
> This is a weird thing to do. We normally delete vqs then reset.
No, all the drivers first reset, then delete.
> For example, I don't know whether we guarantee what happens
> to MSI-X vectors assigned meanwhile if you reset.
> IIRC qemu doesn't use MSI-X for the balloon, but nothing
> prevents it.
>
> > + vdev->config->del_vqs(vdev);
>
> What prevents requests being submitted at this point?
> I see all kind of handling of freezing in the balloon thread ...
> maybe that gives us some guarantees, but if yes
> it makes sense to add a comment to point this out.
Once reset, the host won't write anything to the vqs anyway..
Amit
^ permalink raw reply
* Re: [PATCH v3 06/11] virtio: blk: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:28 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <81e98fc00370152ded2eef20f0953f19fad6f0f5.1321530505.git.amit.shah@redhat.com>
On Thu, Nov 17, 2011 at 05:27:37PM +0530, Amit Shah wrote:
> Delete the vq and flush any pending requests from the block queue on the
> freeze callback to prepare for hibernation.
>
> Re-create the vq in the restore callback to resume normal function.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/block/virtio_blk.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 467f218..f73fb2d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -568,6 +568,38 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
> ida_simple_remove(&vd_index_ida, index);
> }
>
> +#ifdef CONFIG_PM
> +static int virtblk_freeze(struct virtio_device *vdev)
> +{
> + struct virtio_blk *vblk = vdev->priv;
> +
> + flush_work(&vblk->config_work);
What if a new config_work is scheduled after this point?
The right thing to do seems to be to flush after disabling interrupts.
> + spin_lock_irq(vblk->disk->queue->queue_lock);
> + blk_stop_queue(vblk->disk->queue);
> + spin_unlock_irq(vblk->disk->queue->queue_lock);
> + blk_sync_queue(vblk->disk->queue);
> +
> + /* Stop all the virtqueues. */
> + vdev->config->reset(vdev);
> + vdev->config->del_vqs(vdev);
I think del_vqs should be followed by reset:
del_vqs assumes the device in a state find put it in.
> + return 0;
> +}
> +
> +static int virtblk_restore(struct virtio_device *vdev)
> +{
> + struct virtio_blk *vblk = vdev->priv;
> + int ret;
> +
> + ret = init_vq(vdev->priv);
> + if (!ret) {
> + spin_lock_irq(vblk->disk->queue->queue_lock);
> + blk_start_queue(vblk->disk->queue);
> + spin_unlock_irq(vblk->disk->queue->queue_lock);
> + }
> + return ret;
> +}
> +#endif
> +
> static const struct virtio_device_id id_table[] = {
> { VIRTIO_ID_BLOCK, VIRTIO_DEV_ANY_ID },
> { 0 },
> @@ -593,6 +625,10 @@ static struct virtio_driver __refdata virtio_blk = {
> .probe = virtblk_probe,
> .remove = __devexit_p(virtblk_remove),
> .config_changed = virtblk_config_changed,
> +#ifdef CONFIG_PM
> + .freeze = virtblk_freeze,
> + .restore = virtblk_restore,
> +#endif
> };
>
> static int __init init(void)
> --
> 1.7.7.1
^ permalink raw reply
* Re: [PATCH v3 09/11] virtio: net: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-11-17 12:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <20111117121907.GA19682@redhat.com>
On (Thu) 17 Nov 2011 [14:19:09], Michael S. Tsirkin wrote:
> On Thu, Nov 17, 2011 at 05:27:40PM +0530, Amit Shah wrote:
> > +#ifdef CONFIG_PM
> > +static int virtnet_freeze(struct virtio_device *vdev)
> > +{
> > + struct virtnet_info *vi = vdev->priv;
> > +
> > + netif_device_detach(vi->dev);
> > + remove_vq_common(vi);
>
> This stops TX in progress, if any, but not RX
> which might use the RX VQ. Then remove_vq_common
> might delete this VQ while it's still in use.
>
> So I think we need to call something like napi_disable.
> However, the subtle twist is that we need to call that
> *after interrupts have been disabled*.
> Otherwise we might schedule another napi callback.
resetting the vqs will mean the host won't pass us any data in the
vqs. Plus we're removing the vqs altogether. Also, we're disabling
the pci device in virtio_pci.c, so all of these actions will take
care of that, isn't it?
In addition, once the vqs are taken off, there's no chance for any
other rx to happen, so napi_disable() after plugging off vqs doesn't
make sense.
Amit
^ permalink raw reply
* Re: [PATCH v3 11/11] virtio: balloon: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:25 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <1147d4f726e51f2e03a84f3732d60ccd0d8e480c.1321530505.git.amit.shah@redhat.com>
On Thu, Nov 17, 2011 at 05:27:42PM +0530, Amit Shah wrote:
> Delete the vqs on the freeze callback to prepare for hibernation.
> Re-create the vqs in the restore callback to resume normal function.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/virtio/virtio_balloon.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 1ff3cf4..90149d1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -363,6 +363,22 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
> kfree(vb);
> }
>
> +#ifdef CONFIG_PM
> +static int virtballoon_freeze(struct virtio_device *vdev)
> +{
> + /* Now we reset the device so we can clean up the queues. */
> + vdev->config->reset(vdev);
> +
This is a weird thing to do. We normally delete vqs then reset.
For example, I don't know whether we guarantee what happens
to MSI-X vectors assigned meanwhile if you reset.
IIRC qemu doesn't use MSI-X for the balloon, but nothing
prevents it.
> + vdev->config->del_vqs(vdev);
What prevents requests being submitted at this point?
I see all kind of handling of freezing in the balloon thread ...
maybe that gives us some guarantees, but if yes
it makes sense to add a comment to point this out.
> + return 0;
> +}
> +
> +static int virtballoon_restore(struct virtio_device *vdev)
> +{
> + return init_vqs(vdev->priv);
> +}
> +#endif
> +
> static unsigned int features[] = {
> VIRTIO_BALLOON_F_MUST_TELL_HOST,
> VIRTIO_BALLOON_F_STATS_VQ,
> @@ -377,6 +393,10 @@ static struct virtio_driver virtio_balloon_driver = {
> .probe = virtballoon_probe,
> .remove = __devexit_p(virtballoon_remove),
> .config_changed = virtballoon_changed,
> +#ifdef CONFIG_PM
> + .freeze = virtballoon_freeze,
> + .restore = virtballoon_restore,
> +#endif
> };
>
> static int __init init(void)
> --
> 1.7.7.1
^ permalink raw reply
* Re: [PATCH v3 09/11] virtio: net: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-11-17 12:19 UTC (permalink / raw)
To: Amit Shah; +Cc: linux-kernel, levinsasha928, Virtualization List
In-Reply-To: <85eca44fe83d2deabfdf402bfed72531ebad6026.1321530505.git.amit.shah@redhat.com>
On Thu, Nov 17, 2011 at 05:27:40PM +0530, Amit Shah wrote:
> Remove all the vqs and detach from the netdev on hibernation.
>
> Re-create vqs after restoring from a hibernated image and re-attach the
> netdev. This keeps networking working across hibernation.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/net/virtio_net.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index fbff37a..167b555 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1127,6 +1127,9 @@ static void remove_vq_common(struct virtnet_info *vi)
> {
> cancel_delayed_work_sync(&vi->refill);
>
> + /* Stop all the virtqueues. */
> + vi->vdev->config->reset(vi->vdev);
> +
> /* Free unused buffers in both send and recv, if any. */
> free_unused_bufs(vi);
>
> @@ -1140,9 +1143,6 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
> {
> struct virtnet_info *vi = vdev->priv;
>
> - /* Stop all the virtqueues. */
> - vdev->config->reset(vdev);
> -
> unregister_netdev(vi->dev);
>
> remove_vq_common(vi);
> @@ -1151,6 +1151,33 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
> free_netdev(vi->dev);
> }
>
> +#ifdef CONFIG_PM
> +static int virtnet_freeze(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> +
> + netif_device_detach(vi->dev);
> + remove_vq_common(vi);
This stops TX in progress, if any, but not RX
which might use the RX VQ. Then remove_vq_common
might delete this VQ while it's still in use.
So I think we need to call something like napi_disable.
However, the subtle twist is that we need to call that
*after interrupts have been disabled*.
Otherwise we might schedule another napi callback.
> +
> + return 0;
> +}
> +
> +static int virtnet_restore(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> + int err;
> +
> + err = init_vqs(vi);
> + if (err)
> + return err;
> +
> + try_fill_recv(vi, GFP_KERNEL);
> +
> + netif_device_attach(vi->dev);
> + return 0;
> +}
> +#endif
> +
> static struct virtio_device_id id_table[] = {
> { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> @@ -1175,6 +1202,10 @@ static struct virtio_driver virtio_net_driver = {
> .probe = virtnet_probe,
> .remove = __devexit_p(virtnet_remove),
> .config_changed = virtnet_config_changed,
> +#ifdef CONFIG_PM
> + .freeze = virtnet_freeze,
> + .restore = virtnet_restore,
> +#endif
> };
>
> static int __init init(void)
> --
> 1.7.7.1
^ permalink raw reply
* [PATCH v3 11/11] virtio: balloon: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-11-17 11:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Amit Shah, linux-kernel, Michael S. Tsirkin, levinsasha928,
Virtualization List
In-Reply-To: <cover.1321530505.git.amit.shah@redhat.com>
Delete the vqs on the freeze callback to prepare for hibernation.
Re-create the vqs in the restore callback to resume normal function.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_balloon.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 1ff3cf4..90149d1 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -363,6 +363,22 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
kfree(vb);
}
+#ifdef CONFIG_PM
+static int virtballoon_freeze(struct virtio_device *vdev)
+{
+ /* Now we reset the device so we can clean up the queues. */
+ vdev->config->reset(vdev);
+
+ vdev->config->del_vqs(vdev);
+ return 0;
+}
+
+static int virtballoon_restore(struct virtio_device *vdev)
+{
+ return init_vqs(vdev->priv);
+}
+#endif
+
static unsigned int features[] = {
VIRTIO_BALLOON_F_MUST_TELL_HOST,
VIRTIO_BALLOON_F_STATS_VQ,
@@ -377,6 +393,10 @@ static struct virtio_driver virtio_balloon_driver = {
.probe = virtballoon_probe,
.remove = __devexit_p(virtballoon_remove),
.config_changed = virtballoon_changed,
+#ifdef CONFIG_PM
+ .freeze = virtballoon_freeze,
+ .restore = virtballoon_restore,
+#endif
};
static int __init init(void)
--
1.7.7.1
^ permalink raw reply related
* [PATCH v3 10/11] virtio: balloon: Move out vq initialization into separate function
From: Amit Shah @ 2011-11-17 11:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Amit Shah, linux-kernel, Michael S. Tsirkin, levinsasha928,
Virtualization List
In-Reply-To: <cover.1321530505.git.amit.shah@redhat.com>
The probe and PM restore functions will share this code.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_balloon.c | 48 ++++++++++++++++++++++++--------------
1 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 94fd738..1ff3cf4 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -275,32 +275,21 @@ static int balloon(void *_vballoon)
return 0;
}
-static int virtballoon_probe(struct virtio_device *vdev)
+static int init_vqs(struct virtio_balloon *vb)
{
- struct virtio_balloon *vb;
struct virtqueue *vqs[3];
vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
const char *names[] = { "inflate", "deflate", "stats" };
int err, nvqs;
- vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
- if (!vb) {
- err = -ENOMEM;
- goto out;
- }
-
- INIT_LIST_HEAD(&vb->pages);
- vb->num_pages = 0;
- init_waitqueue_head(&vb->config_change);
- vb->vdev = vdev;
- vb->need_stats_update = 0;
-
- /* We expect two virtqueues: inflate and deflate,
- * and optionally stat. */
+ /*
+ * We expect two virtqueues: inflate and deflate, and
+ * optionally stat.
+ */
nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
- err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
+ err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
if (err)
- goto out_free_vb;
+ return err;
vb->inflate_vq = vqs[0];
vb->deflate_vq = vqs[1];
@@ -317,6 +306,29 @@ static int virtballoon_probe(struct virtio_device *vdev)
BUG();
virtqueue_kick(vb->stats_vq);
}
+ return 0;
+}
+
+static int virtballoon_probe(struct virtio_device *vdev)
+{
+ struct virtio_balloon *vb;
+ int err;
+
+ vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
+ if (!vb) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ INIT_LIST_HEAD(&vb->pages);
+ vb->num_pages = 0;
+ init_waitqueue_head(&vb->config_change);
+ vb->vdev = vdev;
+ vb->need_stats_update = 0;
+
+ err = init_vqs(vb);
+ if (err)
+ goto out_free_vb;
vb->thread = kthread_run(balloon, vb, "vballoon");
if (IS_ERR(vb->thread)) {
--
1.7.7.1
^ permalink raw reply related
* [PATCH v3 09/11] virtio: net: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-11-17 11:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Amit Shah, linux-kernel, Michael S. Tsirkin, levinsasha928,
Virtualization List
In-Reply-To: <cover.1321530505.git.amit.shah@redhat.com>
Remove all the vqs and detach from the netdev on hibernation.
Re-create vqs after restoring from a hibernated image and re-attach the
netdev. This keeps networking working across hibernation.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/net/virtio_net.c | 37 ++++++++++++++++++++++++++++++++++---
1 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fbff37a..167b555 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1127,6 +1127,9 @@ static void remove_vq_common(struct virtnet_info *vi)
{
cancel_delayed_work_sync(&vi->refill);
+ /* Stop all the virtqueues. */
+ vi->vdev->config->reset(vi->vdev);
+
/* Free unused buffers in both send and recv, if any. */
free_unused_bufs(vi);
@@ -1140,9 +1143,6 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev->priv;
- /* Stop all the virtqueues. */
- vdev->config->reset(vdev);
-
unregister_netdev(vi->dev);
remove_vq_common(vi);
@@ -1151,6 +1151,33 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}
+#ifdef CONFIG_PM
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ netif_device_detach(vi->dev);
+ remove_vq_common(vi);
+
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ int err;
+
+ err = init_vqs(vi);
+ if (err)
+ return err;
+
+ try_fill_recv(vi, GFP_KERNEL);
+
+ netif_device_attach(vi->dev);
+ return 0;
+}
+#endif
+
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
@@ -1175,6 +1202,10 @@ static struct virtio_driver virtio_net_driver = {
.probe = virtnet_probe,
.remove = __devexit_p(virtnet_remove),
.config_changed = virtnet_config_changed,
+#ifdef CONFIG_PM
+ .freeze = virtnet_freeze,
+ .restore = virtnet_restore,
+#endif
};
static int __init init(void)
--
1.7.7.1
^ permalink raw reply related
* [PATCH v3 08/11] virtio: net: Move out vq and vq buf removal into separate function
From: Amit Shah @ 2011-11-17 11:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Amit Shah, linux-kernel, Michael S. Tsirkin, levinsasha928,
Virtualization List
In-Reply-To: <cover.1321530505.git.amit.shah@redhat.com>
The remove and PM freeze functions will share this code.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/net/virtio_net.c | 23 ++++++++++++++---------
1 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6baa563..fbff37a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1123,24 +1123,29 @@ static void free_unused_bufs(struct virtnet_info *vi)
BUG_ON(vi->num != 0);
}
-static void __devexit virtnet_remove(struct virtio_device *vdev)
+static void remove_vq_common(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
-
- /* Stop all the virtqueues. */
- vdev->config->reset(vdev);
-
-
- unregister_netdev(vi->dev);
cancel_delayed_work_sync(&vi->refill);
/* Free unused buffers in both send and recv, if any. */
free_unused_bufs(vi);
- vdev->config->del_vqs(vi->vdev);
+ vi->vdev->config->del_vqs(vi->vdev);
while (vi->pages)
__free_pages(get_a_page(vi, GFP_KERNEL), 0);
+}
+
+static void __devexit virtnet_remove(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ /* Stop all the virtqueues. */
+ vdev->config->reset(vdev);
+
+ unregister_netdev(vi->dev);
+
+ remove_vq_common(vi);
free_percpu(vi->stats);
free_netdev(vi->dev);
--
1.7.7.1
^ permalink raw reply related
* [PATCH v3 07/11] virtio: net: Move out vq initialization into separate function
From: Amit Shah @ 2011-11-17 11:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Amit Shah, linux-kernel, Michael S. Tsirkin, levinsasha928,
Virtualization List
In-Reply-To: <cover.1321530505.git.amit.shah@redhat.com>
The probe and PM restore functions will share this code.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/net/virtio_net.c | 47 +++++++++++++++++++++++++++------------------
1 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6ee8410..6baa563 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -954,15 +954,38 @@ static void virtnet_config_changed(struct virtio_device *vdev)
virtnet_update_status(vi);
}
+static int init_vqs(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;
+
+ /* We expect two virtqueues, receive then send,
+ * and optionally control. */
+ nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
+
+ err = vi->vdev->config->find_vqs(vi->vdev, nvqs, vqs, callbacks, names);
+ if (err)
+ return err;
+
+ vi->rvq = vqs[0];
+ vi->svq = vqs[1];
+
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
+ vi->cvq = vqs[2];
+
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
+ vi->dev->features |= NETIF_F_HW_VLAN_FILTER;
+ }
+ return 0;
+}
+
static int virtnet_probe(struct virtio_device *vdev)
{
int err;
struct net_device *dev;
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;
/* Allocate ourselves a network device with room for our info */
dev = alloc_etherdev(sizeof(struct virtnet_info));
@@ -1034,24 +1057,10 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
vi->mergeable_rx_bufs = true;
- /* We expect two virtqueues, receive then send,
- * and optionally control. */
- nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
-
- err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
+ err = init_vqs(vi);
if (err)
goto free_stats;
- vi->rvq = vqs[0];
- vi->svq = vqs[1];
-
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
- vi->cvq = vqs[2];
-
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
- dev->features |= NETIF_F_HW_VLAN_FILTER;
- }
-
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
--
1.7.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox