* Re: Large Patch Series in Email (was Re: [PATCH 0000/0117] Staging: hv: Driver cleanup)
From: Michael Witten @ 2011-07-15 22:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: K. Y. Srinivasan, Greg Kroah-Hartman, linux-kernel, devel,
virtualization
In-Reply-To: <20110715210903.GF18655@shale.localdomain>
On Sat, 16 Jul 2011 00:09:03 +0300, Dan Carpenter wrote:
> On Fri, Jul 15, 2011 at 06:25:55PM -0000, Michael Witten wrote:
>> Do not send more than 15 patches at once to the vger
>> mailing lists!!!
>
> ... Don't be a whinge bucket.
Or be respectful of bandwidth, differing email environments, and the
official guidelines for submitting patches, which I will quote again:
If you cannot condense your patch set into a smaller set
of patches, then only post say 15 or so at a time and wait
for review and integration.
...
Do not send more than 15 patches at once to the vger
mailing lists!!!
Sincerely,
Michael Witten
^ permalink raw reply
* Re: Large Patch Series in Email (was Re: [PATCH 0000/0117] Staging: hv: Driver cleanup)
From: Alan Cox @ 2011-07-15 23:07 UTC (permalink / raw)
To: Michael Witten
Cc: K. Y. Srinivasan, Greg Kroah-Hartman, linux-kernel, devel,
virtualization
In-Reply-To: <84fa825a57d34571a5aa66ae7299461c-mfwitten@gmail.com>
> Do not send more than 15 patches at once to the vger
> mailing lists!!!
>
> and, accordingly, I went to the trouble of setting up a GitHub
> account to host a repo from which I could issue *one* single
> PULL request email; I get a little miffed every time my
> inbox gets blasted with hundreds of patches when others don't
> do similarly.
The problem with dumping stuff that needs review into a git tree is it's
a lot of hassle to review so the advice is kind of outdated in such cases.
It's good advice for things like big new subsystems perhaps but not for
review.
As is always the case social norms evolve faster than the people who feel
compelled to attempt to document them.
There are lots of web archives of the list and it's also not hard to set
up mail tools to shuffle long emails into a folder so there are plenty of
ways to manage and read the lists without being part of it.
And someone should probably updating the CodingStyle document to reflect
reality 8)
Alan
^ permalink raw reply
* Re: Large Patch Series in Email
From: David Miller @ 2011-07-15 23:22 UTC (permalink / raw)
To: mfwitten; +Cc: error27, gregkh, linux-kernel, virtualization, devel
In-Reply-To: <2069ac4443284c47983ba731368ddb56-mfwitten@gmail.com>
From: Michael Witten <mfwitten@gmail.com>
Date: Fri, 15 Jul 2011 22:55:29 -0000
> On Sat, 16 Jul 2011 00:09:03 +0300, Dan Carpenter wrote:
>
>> On Fri, Jul 15, 2011 at 06:25:55PM -0000, Michael Witten wrote:
>>> Do not send more than 15 patches at once to the vger
>>> mailing lists!!!
>>
>> ... Don't be a whinge bucket.
>
> Or be respectful of bandwidth, differing email environments, and the
> official guidelines for submitting patches, which I will quote again:
>
> If you cannot condense your patch set into a smaller set
> of patches, then only post say 15 or so at a time and wait
> for review and integration.
> ...
> Do not send more than 15 patches at once to the vger
> mailing lists!!!
Indeed, it really sucks when people send huge patch sets, do
not do it.
If the official SubmittingPatches document isn't convincing
enough, then maybe me (the vger postmaster) saying it will.
^ permalink raw reply
* [PATCH 1/1] Staging: hv: Integrate the time source driver with Hyper-V detection code
From: K. Y. Srinivasan @ 2011-07-16 1:01 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization, tglx
Cc: K. Y. Srinivasan, Haiyang Zhang
The Hyper-V timesource driver is best integrated with Hyper-V detection code
since: (a) Linux guests running on Hyper-V need this timesource and (b)
by integrating with Hyper-V detection, we could significantly minimize the
code in the timesource driver.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
arch/x86/kernel/cpu/mshyperv.c | 24 ++++++++
drivers/staging/hv/Makefile | 2 +-
drivers/staging/hv/hv_timesource.c | 102 ------------------------------------
3 files changed, 25 insertions(+), 103 deletions(-)
delete mode 100644 drivers/staging/hv/hv_timesource.c
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index d944bf6..c97f88d 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -11,6 +11,8 @@
*/
#include <linux/types.h>
+#include <linux/time.h>
+#include <linux/clocksource.h>
#include <linux/module.h>
#include <asm/processor.h>
#include <asm/hypervisor.h>
@@ -36,6 +38,25 @@ static bool __init ms_hyperv_platform(void)
!memcmp("Microsoft Hv", hyp_signature, 12);
}
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+ cycle_t current_tick;
+ /*
+ * Read the partition counter to get the current tick count. This count
+ * is set to 0 when the partition is created and is incremented in
+ * 100 nanosecond units.
+ */
+ rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+ return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+ .name = "hyperv_clocksource",
+ .rating = 400, /* use this when running on Hyperv*/
+ .read = read_hv_clock,
+ .mask = CLOCKSOURCE_MASK(64),
+};
+
static void __init ms_hyperv_init_platform(void)
{
/*
@@ -46,6 +67,9 @@ static void __init ms_hyperv_init_platform(void)
printk(KERN_INFO "HyperV: features 0x%x, hints 0x%x\n",
ms_hyperv.features, ms_hyperv.hints);
+
+
+ clocksource_register_hz(&hyperv_cs, NSEC_PER_SEC/100);
}
const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
diff --git a/drivers/staging/hv/Makefile b/drivers/staging/hv/Makefile
index bd176b1..3e0d7e6 100644
--- a/drivers/staging/hv/Makefile
+++ b/drivers/staging/hv/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV) += hv_vmbus.o hv_timesource.o
+obj-$(CONFIG_HYPERV) += hv_vmbus.o
obj-$(CONFIG_HYPERV_STORAGE) += hv_storvsc.o
obj-$(CONFIG_HYPERV_NET) += hv_netvsc.o
obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
diff --git a/drivers/staging/hv/hv_timesource.c b/drivers/staging/hv/hv_timesource.c
deleted file mode 100644
index 0efb049..0000000
--- a/drivers/staging/hv/hv_timesource.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * A clocksource for Linux running on HyperV.
- *
- *
- * Copyright (C) 2010, Novell, Inc.
- * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT. See the GNU General Public License for more
- * details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/version.h>
-#include <linux/clocksource.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/dmi.h>
-#include <asm/hyperv.h>
-#include <asm/mshyperv.h>
-#include <asm/hypervisor.h>
-
-#define HV_CLOCK_SHIFT 22
-
-static cycle_t read_hv_clock(struct clocksource *arg)
-{
- cycle_t current_tick;
- /*
- * Read the partition counter to get the current tick count. This count
- * is set to 0 when the partition is created and is incremented in
- * 100 nanosecond units.
- */
- rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
- return current_tick;
-}
-
-static struct clocksource hyperv_cs = {
- .name = "hyperv_clocksource",
- .rating = 400, /* use this when running on Hyperv*/
- .read = read_hv_clock,
- .mask = CLOCKSOURCE_MASK(64),
- /*
- * The time ref counter in HyperV is in 100ns units.
- * The definition of mult is:
- * mult/2^shift = ns/cyc = 100
- * mult = (100 << shift)
- */
- .mult = (100 << HV_CLOCK_SHIFT),
- .shift = HV_CLOCK_SHIFT,
-};
-
-static const struct dmi_system_id __initconst
-hv_timesource_dmi_table[] __maybe_unused = {
- {
- .ident = "Hyper-V",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
- DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
- },
- },
- { },
-};
-MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
-
-static const struct pci_device_id __initconst
-hv_timesource_pci_table[] __maybe_unused = {
- { PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
- { 0 }
-};
-MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
-
-
-static int __init init_hv_clocksource(void)
-{
- if ((x86_hyper != &x86_hyper_ms_hyperv) ||
- !(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
- return -ENODEV;
-
- if (!dmi_check_system(hv_timesource_dmi_table))
- return -ENODEV;
-
- pr_info("Registering HyperV clock source\n");
- return clocksource_register(&hyperv_cs);
-}
-
-module_init(init_hv_clocksource);
-MODULE_DESCRIPTION("HyperV based clocksource");
-MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
-MODULE_LICENSE("GPL");
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 020/117] Staging: hv: vmbus: Support the notion of id tables in vmbus_match()
From: Christoph Hellwig @ 2011-07-16 2:02 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <1310752065-27895-20-git-send-email-kys@microsoft.com>
On Fri, Jul 15, 2011 at 10:46:08AM -0700, K. Y. Srinivasan wrote:
> +static bool is_not_null_guid(const __u8 *guid)
> +{
> + int i;
> +
> + for (i = 0; i < (sizeof(struct hv_vmbus_device_id)); i++)
> + if (guid[i] != 0)
> + return true;
> + return false;
> +}
would be nice to add uuid_{le,be}_is_nil helpers to uuid.h. I also
think simply using a memcpy might be more efficient than the hand-rolled
loop.
^ permalink raw reply
* Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
From: Christoph Hellwig @ 2011-07-16 2:04 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: devel, Haiyang Zhang, gregkh, linux-kernel, virtualization
In-Reply-To: <1310752065-27895-97-git-send-email-kys@microsoft.com>
Thanks, this looks much cleaner than the initial variant.
> + if (dev_is_ide) {
> + storvsc_get_ide_info(device, &target, &path);
> + host_dev->path = device_info.path_id;
> + host_dev->target = device_info.target_id;
> + } else {
> + host_dev->path = device_info.path_id;
> + host_dev->target = device_info.target_id;
> + }
Is using the device_info values in both branches intentional? If so
there's no need to have these assignments duplicated.
> + if (dev_is_ide) {
> + ret = scsi_add_device(host, 0, target, 0);
> + if (ret) {
> + scsi_remove_host(host);
> + goto err_out;
> + }
I'd add another goto error label for this piece of error handling.
^ permalink raw reply
* Re: [PATCH 098/117] Staging: hv: storvsc: Handle IDE devices using the storvsc driver
From: Christoph Hellwig @ 2011-07-16 2:05 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <1310752065-27895-98-git-send-email-kys@microsoft.com>
On Fri, Jul 15, 2011 at 10:47:26AM -0700, K. Y. Srinivasan wrote:
> Now, enable handling of all IDE devices by extending the storvsc
> device id table to handle IDE guid. As part of this cleanup Kconfig
> and Hyper-V Makefile to not build the IDE driver (blkvsc).
I think this should be part of the previous patch.
^ permalink raw reply
* Re: [PATCH 088/117] Staging: hv: netvsc: Inline the code for free_net_device()
From: Dan Carpenter @ 2011-07-16 10:33 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <1310752065-27895-88-git-send-email-kys@microsoft.com>
On Fri, Jul 15, 2011 at 10:47:16AM -0700, K. Y. Srinivasan wrote:
> -static void free_net_device(struct netvsc_device *device)
> -{
> - WARN_ON(atomic_read(&device->refcnt) != 0);
> - device->dev->ext = NULL;
device->dev->ext points to device. We set it NULL here to prevent
a use after free bug. What prevents that in the new code?
> - kfree(device);
> -}
regards,
dan carpenter
^ permalink raw reply
* RE: [PATCH 020/117] Staging: hv: vmbus: Support the notion of id tables in vmbus_match()
From: KY Srinivasan @ 2011-07-16 12:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <20110716020250.GA7199@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, July 15, 2011 10:03 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 020/117] Staging: hv: vmbus: Support the notion of id tables
> in vmbus_match()
>
> On Fri, Jul 15, 2011 at 10:46:08AM -0700, K. Y. Srinivasan wrote:
> > +static bool is_not_null_guid(const __u8 *guid)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < (sizeof(struct hv_vmbus_device_id)); i++)
> > + if (guid[i] != 0)
> > + return true;
> > + return false;
> > +}
>
> would be nice to add uuid_{le,be}_is_nil helpers to uuid.h. I also
> think simply using a memcpy might be more efficient than the hand-rolled
> loop.
Having a helper function would be great. With regards to the efficiency of this loop,
this is not a performance critical path. If people feel strongly about this, I could
change this; perhaps as a separate patch on top of this series; I don't want to send a large
patch-set like this again.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
From: KY Srinivasan @ 2011-07-16 12:57 UTC (permalink / raw)
To: Christoph Hellwig
Cc: devel@linuxdriverproject.org, Haiyang Zhang, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110716020447.GB7199@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, July 15, 2011 10:05 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices
> using the storvsc driver
>
> Thanks, this looks much cleaner than the initial variant.
>
> > + if (dev_is_ide) {
> > + storvsc_get_ide_info(device, &target, &path);
> > + host_dev->path = device_info.path_id;
> > + host_dev->target = device_info.target_id;
> > + } else {
> > + host_dev->path = device_info.path_id;
> > + host_dev->target = device_info.target_id;
> > + }
>
> Is using the device_info values in both branches intentional? If so
> there's no need to have these assignments duplicated.
While we set the values in both the branches, the value set is different;
The IDE side encodes the bits differently and is appropriately parsed in the
function storvsc_get_ide_info().
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
From: Sasha Levin @ 2011-07-16 13:01 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480817A4DA@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Sat, 2011-07-16 at 12:57 +0000, KY Srinivasan wrote:
>
> > -----Original Message-----
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Friday, July 15, 2011 10:05 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> > Subject: Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices
> > using the storvsc driver
> >
> > Thanks, this looks much cleaner than the initial variant.
> >
> > > + if (dev_is_ide) {
> > > + storvsc_get_ide_info(device, &target, &path);
> > > + host_dev->path = device_info.path_id;
> > > + host_dev->target = device_info.target_id;
> > > + } else {
> > > + host_dev->path = device_info.path_id;
> > > + host_dev->target = device_info.target_id;
> > > + }
> >
> > Is using the device_info values in both branches intentional? If so
> > there's no need to have these assignments duplicated.
>
> While we set the values in both the branches, the value set is different;
> The IDE side encodes the bits differently and is appropriately parsed in the
> function storvsc_get_ide_info().
Is think that what Christoph meant was simplifying it to:
if (dev_is_ide)
storvsc_get_ide_info(device, &target, &path);
host_dev->path = device_info.path_id;
host_dev->target = device_info.target_id;
--
Sasha.
^ permalink raw reply
* RE: [PATCH 088/117] Staging: hv: netvsc: Inline the code for free_net_device()
From: KY Srinivasan @ 2011-07-16 13:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <20110716103342.GJ18655@shale.localdomain>
> -----Original Message-----
> From: Dan Carpenter [mailto:error27@gmail.com]
> Sent: Saturday, July 16, 2011 6:34 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 088/117] Staging: hv: netvsc: Inline the code for
> free_net_device()
>
> On Fri, Jul 15, 2011 at 10:47:16AM -0700, K. Y. Srinivasan wrote:
> > -static void free_net_device(struct netvsc_device *device)
> > -{
> > - WARN_ON(atomic_read(&device->refcnt) != 0);
> > - device->dev->ext = NULL;
>
> device->dev->ext points to device. We set it NULL here to prevent
> a use after free bug. What prevents that in the new code?
>
> > - kfree(device);
> > -}
This function is called in only two places; a) during adding a device when we have a failure
(netvsc_device_add()) and during device removal (netvsc_device_remove()). I have handled
both these cases. In any event when we are freeing the containing object via kfree(), if somebody
is accessing the ext field via freed object, we have bigger problems to deal with.
Regards,
K. Y
>
> regards,
> dan carpenter
>
>
^ permalink raw reply
* RE: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
From: KY Srinivasan @ 2011-07-16 13:12 UTC (permalink / raw)
To: Sasha Levin
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <1310821299.1976.2.camel@lappy>
> -----Original Message-----
> From: Sasha Levin [mailto:levinsasha928@gmail.com]
> Sent: Saturday, July 16, 2011 9:02 AM
> To: KY Srinivasan
> Cc: Christoph Hellwig; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: RE: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices
> using the storvsc driver
>
> On Sat, 2011-07-16 at 12:57 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: Christoph Hellwig [mailto:hch@infradead.org]
> > > Sent: Friday, July 15, 2011 10:05 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> > > Subject: Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE
> devices
> > > using the storvsc driver
> > >
> > > Thanks, this looks much cleaner than the initial variant.
> > >
> > > > + if (dev_is_ide) {
> > > > + storvsc_get_ide_info(device, &target, &path);
> > > > + host_dev->path = device_info.path_id;
> > > > + host_dev->target = device_info.target_id;
> > > > + } else {
> > > > + host_dev->path = device_info.path_id;
> > > > + host_dev->target = device_info.target_id;
> > > > + }
> > >
> > > Is using the device_info values in both branches intentional? If so
> > > there's no need to have these assignments duplicated.
> >
> > While we set the values in both the branches, the value set is different;
> > The IDE side encodes the bits differently and is appropriately parsed in the
> > function storvsc_get_ide_info().
>
> Is think that what Christoph meant was simplifying it to:
>
> if (dev_is_ide)
> storvsc_get_ide_info(device, &target, &path);
>
> host_dev->path = device_info.path_id;
> host_dev->target = device_info.target_id;
Good point! It is still too early in the morning for me.
Greg, if it is ok with you, in the interest of keeping the patch
cops happy, I can submit a patch for this on top of this series.
Regards,
K. Y
>
> --
>
> Sasha.
>
^ permalink raw reply
* Re: [PATCH 020/117] Staging: hv: vmbus: Support the notion of id tables in vmbus_match()
From: Christoph Hellwig @ 2011-07-16 18:24 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480817A4C9@TK5EX14MBXC124.redmond.corp.microsoft.com>
On Sat, Jul 16, 2011 at 12:54:46PM +0000, KY Srinivasan wrote:
> > would be nice to add uuid_{le,be}_is_nil helpers to uuid.h. I also
> > think simply using a memcpy might be more efficient than the hand-rolled
> > loop.
> Having a helper function would be great. With regards to the efficiency of this loop,
> this is not a performance critical path. If people feel strongly about this, I could
> change this; perhaps as a separate patch on top of this series; I don't want to send a large
> patch-set like this again.
I'm fine with doing it later.
^ permalink raw reply
* Re: [PATCH 097/117] Staging: hv: storvsc: Add code to handle IDE devices using the storvsc driver
From: Christoph Hellwig @ 2011-07-16 18:27 UTC (permalink / raw)
To: Sasha Levin
Cc: Haiyang Zhang, gregkh@suse.de, linux-kernel@vger.kernel.org,
Christoph Hellwig, virtualization@lists.osdl.org,
devel@linuxdriverproject.org
In-Reply-To: <1310821299.1976.2.camel@lappy>
On Sat, Jul 16, 2011 at 04:01:39PM +0300, Sasha Levin wrote:
> Is think that what Christoph meant was simplifying it to:
>
> if (dev_is_ide)
> storvsc_get_ide_info(device, &target, &path);
>
> host_dev->path = device_info.path_id;
> host_dev->target = device_info.target_id;
Indeed. In addition path is unused and target is only used in the
second dev_is_ide conditional, so both the call and the target and path
variables can be moved into that one to further simplify the function.
But I'm fine with delaying that cleanup to a follow on patch so that
we can make progress in getting this pile merged.
^ permalink raw reply
* deadline extension 08/16/11 -- SI on Data Intensive Computing in the Clouds in JGC
From: Ioan Raicu @ 2011-07-17 9:20 UTC (permalink / raw)
To: virtualization
Hi all,
Due to numerous requests, we have extended the deadline for the Special
Issue on Data Intensive Computing in the Clouds in the Springer Journal
of Grid Computing to August 16th, 2011. Please see below for the CFP
announcement.
Regards,
Ioan Raicu and Tevfik Kosar
Guest Editors of the Special Issue on Data Intensive Computing in the Clouds
Springer Journal of Grid Computing
http://datasys.cs.iit.edu/events/JGC-DataCloud-2012/index.html
---------------------------------------------------------------------------------
*** Call for Papers ***
Springer Journal of Grid Computing
Special Issue on Data Intensive Computing in the Clouds
http://datasys.cs.iit.edu/events/JGC-DataCloud-2012/
---------------------------------------------------------------------------------
Applications and experiments in all areas of science are becoming increasingly
complex and more demanding in terms of their computational and data requirements.
Some applications generate data volumes reaching hundreds of terabytes and even
petabytes. As scientific applications become more data intensive, the management
of data resources and dataflow between the storage and compute resources is
becoming the main bottleneck. Analyzing, visualizing, and disseminating these
large data sets has become a major challenge and data intensive computing is now
considered as the "fourth paradigm" in scientific discovery after
empirical, theoretical, and computational scientific approaches.
The Special Issue on Data Intensive Computing in the Clouds will provide the
scientific community a dedicated forum, within the prestigious Springer Journal
of Grid Computing, for presenting new research, development, and deployment
efforts in running data-intensive computing workloads on Cloud Computing
infrastructures. This special issue will focus on the use of cloud-based
technologies to meet the new data intensive scientific challenges that are not
well served by the current supercomputers, grids or compute-intensive clouds.
We believe this venue will be an excellent place to help the community define the
current state, determine future goals, and present architectures and services for
future clouds supporting data intensive computing.
TOPICS
---------------------------------------------------------------------------------
- Data-intensive cloud computing applications, characteristics, challenges
- Case studies of data intensive computing in the clouds
- Performance evaluation of data clouds, data grids, and data centers
- Energy-efficient data cloud design and management
- Data placement, scheduling, and interoperability in the clouds
- Accountability, QoS, and SLAs
- Data privacy and protection in a public cloud environment
- Distributed file systems for clouds
- Data streaming and parallelization
- New programming models for data-intensive cloud computing
- Scalability issues in clouds
- Social computing and massively social gaming
- 3D Internet and implications
- Future research challenges in data-intensive cloud computing
Important Dates
---------------------------------------------------------------------------------------
* Papers Due: August 16, 2011
* First Round Decisions: October 15, 2011
* Major Revisions if needed: November 15, 2011
* Second Round Decisions: December 15, 2011
* Minor Revisions if needed: January 15, 2012
* Final Decision: February 1, 2012
* Publication Date: June 2012
PAPER SUBMISSION
---------------------------------------------------------------------------------
Authors are invited to submit original and unpublished technical papers. All
submissions will be peer-reviewed and judged on correctness, originality,
technical strength, significance, quality of presentation, and relevance to the
special issue topics of interest. Submitted papers may not have appeared in or be
under consideration for another workshop, conference or a journal, nor may they
be under review or submitted to another forum during the review process.
Submitted papers may not exceed 20 single-spaced double-column pages using
10-point size font on 8.5x11 inch pages (1" margins), including figures, tables,
and references; note that accepted papers will likely be between 15 to 20 pages,
depending on a variety of factors; for more information for preparing the
submitted papers, please see
http://www.springer.com/computer/communication+networks/journal/10723, under
"Instructions for Authors". The papers (PDF format) must be submitted online at
http://grid.edmgr.com/ before the extended deadline of August 16th, 2011
at 11:59PM PST. For any questions on the submission process, please email the
guest editors at jgc-datacloud-2012@datasys.cs.iit.edu.
Guest Editors
---------------------------------------------------------------------------------------
Special Issue Guest Editors
* Tevfik Kosar (tkosar@buffalo.edu), University at Buffalo
* Ioan Raicu (iraicu@cs.iit.edu), Illinois Institute of Technology&
Argonne National Laboratory
Editors-in-Chief
* Peter Kacsuk, Hungarian Academy of Sciences
* Ian Foster, University of Chicago& Argonne National Laboratory
--
=================================================================
Ioan Raicu, Ph.D.
Assistant Professor, Illinois Institute of Technology (IIT)
Guest Research Faculty, Argonne National Laboratory (ANL)
=================================================================
Data-Intensive Distributed Systems Laboratory, CS/IIT
Distributed Systems Laboratory, MCS/ANL
=================================================================
Cel: 1-847-722-0876
Office: 1-312-567-5704
Email: iraicu@cs.iit.edu
Web: http://www.cs.iit.edu/~iraicu/
Web: http://datasys.cs.iit.edu/
=================================================================
=================================================================
^ permalink raw reply
* Re: RFT: virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-07-17 9:42 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Krishna Kumar2, habanero, lguest, Shirley Ma, kvm, Carsten Otte,
linux-s390, Heiko Carstens, linux-kernel, virtualization, steved,
Christian Borntraeger, Tom Lendacky, netdev, Martin Schwidefsky,
linux390
In-Reply-To: <CA4493AD.2E273%roprabhu@cisco.com>
On Thu, Jul 14, 2011 at 12:38:05PM -0700, Roopa Prabhu wrote:
> Michael, below are some numbers I got from one round of runs.
> Thanks,
> Roopa
Thanks!
At this point it does not appear like there's any measureable
impact from moving the polling around.
--
MST
^ permalink raw reply
* Re: Large Patch Series in Email (was Re: [PATCH 0000/0117] Staging: hv: Driver cleanup)
From: Florian Mickler @ 2011-07-17 10:47 UTC (permalink / raw)
To: Alan Cox
Cc: Michael Witten, K. Y. Srinivasan, Greg Kroah-Hartman,
linux-kernel, devel, virtualization
In-Reply-To: <20110716000739.3239bac8@lxorguk.ukuu.org.uk>
On Sat, 16 Jul 2011 00:07:39 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > Do not send more than 15 patches at once to the vger
> > mailing lists!!!
> >
> > and, accordingly, I went to the trouble of setting up a GitHub
> > account to host a repo from which I could issue *one* single
> > PULL request email; I get a little miffed every time my
> > inbox gets blasted with hundreds of patches when others don't
> > do similarly.
>
> The problem with dumping stuff that needs review into a git tree is it's
> a lot of hassle to review so the advice is kind of outdated in such cases.
> It's good advice for things like big new subsystems perhaps but not for
> review.
>
> As is always the case social norms evolve faster than the people who feel
> compelled to attempt to document them.
>
> There are lots of web archives of the list and it's also not hard to set
> up mail tools to shuffle long emails into a folder so there are plenty of
> ways to manage and read the lists without being part of it.
>
> And someone should probably updating the CodingStyle document to reflect
> reality 8)
>
> Alan
And while at it, maybe add some tipps on how to keep patchseries
small... (people have probably more / better suggestions, please):
- 'submit early, submit often' instead of time based submittal (like...
oh I hacked 24/7 this week and now is friday, let's see how many
patches I got...)
- Putting controversial stuff at the end (so at least uncontroversial
stuff can be applied)
- try to split patchseries into stuff that can be applied independent
of one another.
- ...
Regards,
Flo
^ permalink raw reply
* [PATCHv9] vhost: experimental tx zero-copy support
From: Michael S. Tsirkin @ 2011-07-17 19:36 UTC (permalink / raw)
To: kvm, virtualization, netdev, linux-kernel
From: Shirley Ma <mashirle@us.ibm.com>
This adds experimental zero copy support in vhost-net,
disabled by default. To enable, set the zerocopytx
module option to 1.
This patch maintains the outstanding userspace buffers in the
sequence it is delivered to vhost. The outstanding userspace buffers
will be marked as done once the lower device buffers DMA has finished.
This is monitored through last reference of kfree_skb callback. Two
buffer indices are used for this purpose.
The vhost-net device passes the userspace buffers info to lower device
skb through message control. DMA done status check and guest
notification are handled by handle_tx: in the worst case is all buffers
in the vq are in pending/done status, so we need to notify guest to
release DMA done buffers first before we get any new buffers from the
vq.
One known problem is that if the guest stops submitting
buffers, buffers might never get used until some
further action, e.g. device reset. This does not
seem to affect linux guests.
Signed-off-by: Shirley <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
The below is what I came up with. We add the feature enabled
by default for now as there are known issues, but some
guests can benefit so there's value in putting this
in tree, to help the code get wider testing.
drivers/vhost/net.c | 73 +++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 29 +++++++++++++++++
3 files changed, 186 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e224a92..226ca6b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -12,6 +12,7 @@
#include <linux/virtio_net.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/workqueue.h>
#include <linux/rcupdate.h>
@@ -28,10 +29,18 @@
#include "vhost.h"
+static int zcopytx;
+module_param(zcopytx, int, 0444);
+MODULE_PARM_DESC(lnksts, "Enable Zero Copy Transmit");
+
/* Max number of bytes transferred before requeueing the job.
* Using this limit prevents one virtqueue from starving others. */
#define VHOST_NET_WEIGHT 0x80000
+/* MAX number of TX used buffers for outstanding zerocopy */
+#define VHOST_MAX_PEND 128
+#define VHOST_GOODCOPY_LEN 256
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -54,6 +63,11 @@ struct vhost_net {
enum vhost_net_poll_state tx_poll_state;
};
+static bool vhost_sock_zcopy(struct socket *sock)
+{
+ return unlikely(zcopytx) && sock_flag(sock->sk, SOCK_ZEROCOPY);
+}
+
/* Pop first len bytes from iovec. Return number of segments used. */
static int move_iovec_hdr(struct iovec *from, struct iovec *to,
size_t len, int iov_count)
@@ -129,6 +143,8 @@ static void handle_tx(struct vhost_net *net)
int err, wmem;
size_t hdr_size;
struct socket *sock;
+ struct vhost_ubuf_ref *uninitialized_var(ubufs);
+ bool zcopy;
/* TODO: check that we are running from vhost_worker? */
sock = rcu_dereference_check(vq->private_data, 1);
@@ -149,8 +165,13 @@ static void handle_tx(struct vhost_net *net)
if (wmem < sock->sk->sk_sndbuf / 2)
tx_poll_stop(net);
hdr_size = vq->vhost_hlen;
+ zcopy = vhost_sock_zcopy(sock);
for (;;) {
+ /* Release DMAs done buffers first */
+ if (zcopy)
+ vhost_zerocopy_signal_used(vq);
+
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
@@ -166,6 +187,12 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
+ /* If more outstanding DMAs, queue the work */
+ if (vq->upend_idx - vq->done_idx > VHOST_MAX_PEND) {
+ tx_poll_start(net, sock);
+ set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
+ break;
+ }
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
vhost_disable_notify(&net->dev, vq);
continue;
@@ -188,9 +215,39 @@ static void handle_tx(struct vhost_net *net)
iov_length(vq->hdr, s), hdr_size);
break;
}
+ /* use msg_control to pass vhost zerocopy ubuf info to skb */
+ if (zcopy) {
+ vq->heads[vq->upend_idx].id = head;
+ if (len < VHOST_GOODCOPY_LEN) {
+ /* copy don't need to wait for DMA done */
+ vq->heads[vq->upend_idx].len =
+ VHOST_DMA_DONE_LEN;
+ msg.msg_control = NULL;
+ msg.msg_controllen = 0;
+ ubufs = NULL;
+ } else {
+ struct ubuf_info *ubuf = &vq->ubuf_info[head];
+
+ vq->heads[vq->upend_idx].len = len;
+ ubuf->callback = vhost_zerocopy_callback;
+ ubuf->arg = vq->ubufs;
+ ubuf->desc = vq->upend_idx;
+ msg.msg_control = ubuf;
+ msg.msg_controllen = sizeof(ubuf);
+ ubufs = vq->ubufs;
+ kref_get(&ubufs->kref);
+ }
+ vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
+ }
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
+ if (zcopy) {
+ if (ubufs)
+ vhost_ubuf_put(ubufs);
+ vq->upend_idx = ((unsigned)vq->upend_idx - 1) %
+ UIO_MAXIOV;
+ }
vhost_discard_vq_desc(vq, 1);
tx_poll_start(net, sock);
break;
@@ -198,7 +255,8 @@ static void handle_tx(struct vhost_net *net)
if (err != len)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ if (!zcopy)
+ vhost_add_used_and_signal(&net->dev, vq, head, 0);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
@@ -603,6 +661,7 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
{
struct socket *sock, *oldsock;
struct vhost_virtqueue *vq;
+ struct vhost_ubuf_ref *ubufs, *oldubufs = NULL;
int r;
mutex_lock(&n->dev.mutex);
@@ -632,6 +691,13 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
oldsock = rcu_dereference_protected(vq->private_data,
lockdep_is_held(&vq->mutex));
if (sock != oldsock) {
+ ubufs = vhost_ubuf_alloc(vq, sock && vhost_sock_zcopy(sock));
+ if (IS_ERR(ubufs)) {
+ r = PTR_ERR(ubufs);
+ goto err_ubufs;
+ }
+ oldubufs = vq->ubufs;
+ vq->ubufs = ubufs;
vhost_net_disable_vq(n, vq);
rcu_assign_pointer(vq->private_data, sock);
vhost_net_enable_vq(n, vq);
@@ -639,6 +705,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&vq->mutex);
+ if (oldubufs)
+ vhost_ubuf_put_and_wait(oldubufs);
+
if (oldsock) {
vhost_net_flush_vq(n, index);
fput(oldsock->file);
@@ -647,6 +716,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&n->dev.mutex);
return 0;
+err_ubufs:
+ fput(sock->file);
err_vq:
mutex_unlock(&vq->mutex);
err:
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ea966b3..2ebf6fc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -179,6 +179,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->upend_idx = 0;
+ vq->done_idx = 0;
+ vq->ubufs = NULL;
}
static int vhost_worker(void *data)
@@ -237,6 +240,8 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
GFP_KERNEL);
dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
UIO_MAXIOV, GFP_KERNEL);
+ dev->vqs[i].ubuf_info = kmalloc(sizeof *dev->vqs[i].ubuf_info *
+ UIO_MAXIOV, GFP_KERNEL);
if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
!dev->vqs[i].heads)
@@ -249,6 +254,7 @@ err_nomem:
kfree(dev->vqs[i].indirect);
kfree(dev->vqs[i].log);
kfree(dev->vqs[i].heads);
+ kfree(dev->vqs[i].ubuf_info);
}
return -ENOMEM;
}
@@ -390,6 +396,29 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
+/* In case of DMA done not in order in lower device driver for some reason.
+ * upend_idx is used to track end of used idx, done_idx is used to track head
+ * of used idx. Once lower device DMA done contiguously, we will signal KVM
+ * guest used idx.
+ */
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+{
+ int i, j = 0;
+
+ for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
+ if ((vq->heads[i].len == VHOST_DMA_DONE_LEN)) {
+ vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
+ vhost_add_used_and_signal(vq->dev, vq,
+ vq->heads[i].id, 0);
+ ++j;
+ } else
+ break;
+ }
+ if (j)
+ vq->done_idx = i;
+ return j;
+}
+
/* Caller should have device mutex */
void vhost_dev_cleanup(struct vhost_dev *dev)
{
@@ -400,6 +429,13 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
vhost_poll_stop(&dev->vqs[i].poll);
vhost_poll_flush(&dev->vqs[i].poll);
}
+ /* Wait for all lower device DMAs done. */
+ if (dev->vqs[i].ubufs)
+ vhost_ubuf_put_and_wait(dev->vqs[i].ubufs);
+
+ /* Signal guest as appropriate. */
+ vhost_zerocopy_signal_used(&dev->vqs[i]);
+
if (dev->vqs[i].error_ctx)
eventfd_ctx_put(dev->vqs[i].error_ctx);
if (dev->vqs[i].error)
@@ -1486,3 +1522,52 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
&vq->used->flags, r);
}
}
+
+static void vhost_zerocopy_done_signal(struct kref *kref)
+{
+ struct vhost_ubuf_ref *ubufs = container_of(kref, struct vhost_ubuf_ref,
+ kref);
+ wake_up(&ubufs->wait);
+}
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *vq,
+ bool zcopy)
+{
+ struct vhost_ubuf_ref *ubufs;
+ /* No zero copy backend? Nothing to count. */
+ if (!zcopy)
+ return NULL;
+ ubufs = kmalloc(sizeof *ubufs, GFP_KERNEL);
+ if (!ubufs)
+ return ERR_PTR(-ENOMEM);
+ kref_init(&ubufs->kref);
+ kref_get(&ubufs->kref);
+ init_waitqueue_head(&ubufs->wait);
+ ubufs->vq = vq;
+ return ubufs;
+}
+
+void vhost_ubuf_put(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
+
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+ wait_event(ubufs->wait, !atomic_read(&ubufs->kref.refcount));
+ kfree(ubufs);
+}
+
+void vhost_zerocopy_callback(void *arg)
+{
+ struct ubuf_info *ubuf = (struct ubuf_info *)arg;
+ struct vhost_ubuf_ref *ubufs;
+ struct vhost_virtqueue *vq;
+
+ ubufs = ubuf->arg;
+ vq = ubufs->vq;
+ /* set len = 1 to mark this desc buffers done DMA */
+ vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8e03379..e287145 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -13,6 +13,11 @@
#include <linux/virtio_ring.h>
#include <asm/atomic.h>
+/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
+ * done */
+#define VHOST_DMA_DONE_LEN 1
+#define VHOST_DMA_CLEAR_LEN 0
+
struct vhost_device;
struct vhost_work;
@@ -50,6 +55,18 @@ struct vhost_log {
u64 len;
};
+struct vhost_virtqueue;
+
+struct vhost_ubuf_ref {
+ struct kref kref;
+ wait_queue_t wait;
+ struct vhost_virtqueue *vq;
+};
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *, bool zcopy);
+void vhost_ubuf_put(struct vhost_ubuf_ref *);
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *);
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -114,6 +131,16 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+ /* vhost zerocopy support fields below: */
+ /* last used idx for outstanding DMA zerocopy buffers */
+ int upend_idx;
+ /* first used idx for DMA done zerocopy buffers */
+ int done_idx;
+ /* an array of userspace buffers info */
+ struct ubuf_info *ubuf_info;
+ /* Reference counting for outstanding ubufs.
+ * Protected by vq mutex. Writers must also take device mutex. */
+ struct vhost_ubuf_ref *ubufs;
};
struct vhost_dev {
@@ -160,6 +187,8 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_zerocopy_callback(void *arg);
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCHv9] vhost: experimental tx zero-copy support
From: David Miller @ 2011-07-17 19:53 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20110717193540.GA29392@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Sun, 17 Jul 2011 22:36:14 +0300
> The below is what I came up with. We add the feature enabled
> by default ...
s/enabled/disabled/ Well, at least you got it right in the
commit message where it counts :-)
^ permalink raw reply
* Re: [PATCHv9] vhost: experimental tx zero-copy support
From: Jesper Juhl @ 2011-07-17 20:01 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20110717193540.GA29392@redhat.com>
On Sun, 17 Jul 2011, Michael S. Tsirkin wrote:
> From: Shirley Ma <mashirle@us.ibm.com>
>
> This adds experimental zero copy support in vhost-net,
> disabled by default. To enable, set the zerocopytx
> module option to 1.
>
> This patch maintains the outstanding userspace buffers in the
> sequence it is delivered to vhost. The outstanding userspace buffers
> will be marked as done once the lower device buffers DMA has finished.
> This is monitored through last reference of kfree_skb callback. Two
> buffer indices are used for this purpose.
>
> The vhost-net device passes the userspace buffers info to lower device
> skb through message control. DMA done status check and guest
> notification are handled by handle_tx: in the worst case is all buffers
> in the vq are in pending/done status, so we need to notify guest to
> release DMA done buffers first before we get any new buffers from the
> vq.
>
> One known problem is that if the guest stops submitting
> buffers, buffers might never get used until some
> further action, e.g. device reset. This does not
> seem to affect linux guests.
>
> Signed-off-by: Shirley <xma@us.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> The below is what I came up with. We add the feature enabled
> by default for now as there are known issues,
You mean "disabled" - right?
> but some
> guests can benefit so there's value in putting this
> in tree, to help the code get wider testing.
>
> drivers/vhost/net.c | 73 +++++++++++++++++++++++++++++++++++++++++-
> drivers/vhost/vhost.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/vhost/vhost.h | 29 +++++++++++++++++
> 3 files changed, 186 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index e224a92..226ca6b 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -12,6 +12,7 @@
> #include <linux/virtio_net.h>
> #include <linux/miscdevice.h>
> #include <linux/module.h>
> +#include <linux/moduleparam.h>
> #include <linux/mutex.h>
> #include <linux/workqueue.h>
> #include <linux/rcupdate.h>
> @@ -28,10 +29,18 @@
>
> #include "vhost.h"
>
> +static int zcopytx;
> +module_param(zcopytx, int, 0444);
Should everyone be able to read this? How about "0440" just to be
paranoid? or?
> +MODULE_PARM_DESC(lnksts, "Enable Zero Copy Transmit");
> +
> /* Max number of bytes transferred before requeueing the job.
> * Using this limit prevents one virtqueue from starving others. */
> #define VHOST_NET_WEIGHT 0x80000
>
> +/* MAX number of TX used buffers for outstanding zerocopy */
> +#define VHOST_MAX_PEND 128
> +#define VHOST_GOODCOPY_LEN 256
> +
> enum {
> VHOST_NET_VQ_RX = 0,
> VHOST_NET_VQ_TX = 1,
> @@ -54,6 +63,11 @@ struct vhost_net {
> enum vhost_net_poll_state tx_poll_state;
> };
>
> +static bool vhost_sock_zcopy(struct socket *sock)
> +{
> + return unlikely(zcopytx) && sock_flag(sock->sk, SOCK_ZEROCOPY);
> +}
> +
> /* Pop first len bytes from iovec. Return number of segments used. */
> static int move_iovec_hdr(struct iovec *from, struct iovec *to,
> size_t len, int iov_count)
> @@ -129,6 +143,8 @@ static void handle_tx(struct vhost_net *net)
> int err, wmem;
> size_t hdr_size;
> struct socket *sock;
> + struct vhost_ubuf_ref *uninitialized_var(ubufs);
> + bool zcopy;
>
> /* TODO: check that we are running from vhost_worker? */
> sock = rcu_dereference_check(vq->private_data, 1);
> @@ -149,8 +165,13 @@ static void handle_tx(struct vhost_net *net)
> if (wmem < sock->sk->sk_sndbuf / 2)
> tx_poll_stop(net);
> hdr_size = vq->vhost_hlen;
> + zcopy = vhost_sock_zcopy(sock);
>
> for (;;) {
> + /* Release DMAs done buffers first */
> + if (zcopy)
> + vhost_zerocopy_signal_used(vq);
> +
> head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> ARRAY_SIZE(vq->iov),
> &out, &in,
> @@ -166,6 +187,12 @@ static void handle_tx(struct vhost_net *net)
> set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> break;
> }
> + /* If more outstanding DMAs, queue the work */
> + if (vq->upend_idx - vq->done_idx > VHOST_MAX_PEND) {
> + tx_poll_start(net, sock);
> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> + break;
> + }
> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> vhost_disable_notify(&net->dev, vq);
> continue;
> @@ -188,9 +215,39 @@ static void handle_tx(struct vhost_net *net)
> iov_length(vq->hdr, s), hdr_size);
> break;
> }
> + /* use msg_control to pass vhost zerocopy ubuf info to skb */
> + if (zcopy) {
> + vq->heads[vq->upend_idx].id = head;
> + if (len < VHOST_GOODCOPY_LEN) {
> + /* copy don't need to wait for DMA done */
> + vq->heads[vq->upend_idx].len =
> + VHOST_DMA_DONE_LEN;
> + msg.msg_control = NULL;
> + msg.msg_controllen = 0;
> + ubufs = NULL;
> + } else {
> + struct ubuf_info *ubuf = &vq->ubuf_info[head];
> +
> + vq->heads[vq->upend_idx].len = len;
> + ubuf->callback = vhost_zerocopy_callback;
> + ubuf->arg = vq->ubufs;
> + ubuf->desc = vq->upend_idx;
> + msg.msg_control = ubuf;
> + msg.msg_controllen = sizeof(ubuf);
> + ubufs = vq->ubufs;
> + kref_get(&ubufs->kref);
> + }
> + vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
> + }
> /* TODO: Check specific error and bomb out unless ENOBUFS? */
> err = sock->ops->sendmsg(NULL, sock, &msg, len);
> if (unlikely(err < 0)) {
> + if (zcopy) {
> + if (ubufs)
> + vhost_ubuf_put(ubufs);
> + vq->upend_idx = ((unsigned)vq->upend_idx - 1) %
> + UIO_MAXIOV;
> + }
> vhost_discard_vq_desc(vq, 1);
> tx_poll_start(net, sock);
> break;
> @@ -198,7 +255,8 @@ static void handle_tx(struct vhost_net *net)
> if (err != len)
> pr_debug("Truncated TX packet: "
> " len %d != %zd\n", err, len);
> - vhost_add_used_and_signal(&net->dev, vq, head, 0);
> + if (!zcopy)
> + vhost_add_used_and_signal(&net->dev, vq, head, 0);
> total_len += len;
> if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> vhost_poll_queue(&vq->poll);
> @@ -603,6 +661,7 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> {
> struct socket *sock, *oldsock;
> struct vhost_virtqueue *vq;
> + struct vhost_ubuf_ref *ubufs, *oldubufs = NULL;
> int r;
>
> mutex_lock(&n->dev.mutex);
> @@ -632,6 +691,13 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> oldsock = rcu_dereference_protected(vq->private_data,
> lockdep_is_held(&vq->mutex));
> if (sock != oldsock) {
> + ubufs = vhost_ubuf_alloc(vq, sock && vhost_sock_zcopy(sock));
> + if (IS_ERR(ubufs)) {
> + r = PTR_ERR(ubufs);
> + goto err_ubufs;
> + }
> + oldubufs = vq->ubufs;
> + vq->ubufs = ubufs;
> vhost_net_disable_vq(n, vq);
> rcu_assign_pointer(vq->private_data, sock);
> vhost_net_enable_vq(n, vq);
> @@ -639,6 +705,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>
> mutex_unlock(&vq->mutex);
>
> + if (oldubufs)
> + vhost_ubuf_put_and_wait(oldubufs);
> +
> if (oldsock) {
> vhost_net_flush_vq(n, index);
> fput(oldsock->file);
> @@ -647,6 +716,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> mutex_unlock(&n->dev.mutex);
> return 0;
>
> +err_ubufs:
> + fput(sock->file);
> err_vq:
> mutex_unlock(&vq->mutex);
> err:
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index ea966b3..2ebf6fc 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -179,6 +179,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->call_ctx = NULL;
> vq->call = NULL;
> vq->log_ctx = NULL;
> + vq->upend_idx = 0;
> + vq->done_idx = 0;
> + vq->ubufs = NULL;
> }
>
> static int vhost_worker(void *data)
> @@ -237,6 +240,8 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
> GFP_KERNEL);
> dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
> UIO_MAXIOV, GFP_KERNEL);
> + dev->vqs[i].ubuf_info = kmalloc(sizeof *dev->vqs[i].ubuf_info *
> + UIO_MAXIOV, GFP_KERNEL);
>
> if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
> !dev->vqs[i].heads)
> @@ -249,6 +254,7 @@ err_nomem:
> kfree(dev->vqs[i].indirect);
> kfree(dev->vqs[i].log);
> kfree(dev->vqs[i].heads);
> + kfree(dev->vqs[i].ubuf_info);
> }
> return -ENOMEM;
> }
> @@ -390,6 +396,29 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
> return 0;
> }
>
> +/* In case of DMA done not in order in lower device driver for some reason.
> + * upend_idx is used to track end of used idx, done_idx is used to track head
> + * of used idx. Once lower device DMA done contiguously, we will signal KVM
> + * guest used idx.
> + */
> +int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
> +{
> + int i, j = 0;
On two lines?
int i;
int j = 0;
as per CodingStyle..
> +
> + for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
> + if ((vq->heads[i].len == VHOST_DMA_DONE_LEN)) {
> + vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
> + vhost_add_used_and_signal(vq->dev, vq,
> + vq->heads[i].id, 0);
> + ++j;
> + } else
> + break;
> + }
> + if (j)
> + vq->done_idx = i;
> + return j;
> +}
> +
> /* Caller should have device mutex */
> void vhost_dev_cleanup(struct vhost_dev *dev)
> {
> @@ -400,6 +429,13 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> vhost_poll_stop(&dev->vqs[i].poll);
> vhost_poll_flush(&dev->vqs[i].poll);
> }
> + /* Wait for all lower device DMAs done. */
> + if (dev->vqs[i].ubufs)
> + vhost_ubuf_put_and_wait(dev->vqs[i].ubufs);
> +
> + /* Signal guest as appropriate. */
> + vhost_zerocopy_signal_used(&dev->vqs[i]);
> +
> if (dev->vqs[i].error_ctx)
> eventfd_ctx_put(dev->vqs[i].error_ctx);
> if (dev->vqs[i].error)
> @@ -1486,3 +1522,52 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> &vq->used->flags, r);
> }
> }
> +
> +static void vhost_zerocopy_done_signal(struct kref *kref)
> +{
> + struct vhost_ubuf_ref *ubufs = container_of(kref, struct vhost_ubuf_ref,
> + kref);
> + wake_up(&ubufs->wait);
> +}
> +
> +struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *vq,
> + bool zcopy)
> +{
> + struct vhost_ubuf_ref *ubufs;
> + /* No zero copy backend? Nothing to count. */
> + if (!zcopy)
> + return NULL;
> + ubufs = kmalloc(sizeof *ubufs, GFP_KERNEL);
> + if (!ubufs)
> + return ERR_PTR(-ENOMEM);
> + kref_init(&ubufs->kref);
> + kref_get(&ubufs->kref);
> + init_waitqueue_head(&ubufs->wait);
> + ubufs->vq = vq;
> + return ubufs;
> +}
> +
> +void vhost_ubuf_put(struct vhost_ubuf_ref *ubufs)
> +{
> + kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> +}
> +
> +void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
> +{
> + kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> + wait_event(ubufs->wait, !atomic_read(&ubufs->kref.refcount));
> + kfree(ubufs);
> +}
> +
> +void vhost_zerocopy_callback(void *arg)
> +{
> + struct ubuf_info *ubuf = (struct ubuf_info *)arg;
No need to explicitly cast a void*, just do:
struct ubuf_info *ubuf = arg;
> + struct vhost_ubuf_ref *ubufs;
> + struct vhost_virtqueue *vq;
> +
> + ubufs = ubuf->arg;
> + vq = ubufs->vq;
Why not shorten this a bit as:
struct vhost_ubuf_ref *ubufs = ubuf->arg;
struct vhost_virtqueue *vq = ubufs->vq;
?
> + /* set len = 1 to mark this desc buffers done DMA */
> + vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> + kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
> +}
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 8e03379..e287145 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -13,6 +13,11 @@
> #include <linux/virtio_ring.h>
> #include <asm/atomic.h>
>
> +/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
> + * done */
> +#define VHOST_DMA_DONE_LEN 1
> +#define VHOST_DMA_CLEAR_LEN 0
> +
> struct vhost_device;
>
> struct vhost_work;
> @@ -50,6 +55,18 @@ struct vhost_log {
> u64 len;
> };
>
> +struct vhost_virtqueue;
> +
> +struct vhost_ubuf_ref {
> + struct kref kref;
> + wait_queue_t wait;
> + struct vhost_virtqueue *vq;
> +};
> +
> +struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *, bool zcopy);
> +void vhost_ubuf_put(struct vhost_ubuf_ref *);
> +void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *);
> +
> /* The virtqueue structure describes a queue attached to a device. */
> struct vhost_virtqueue {
> struct vhost_dev *dev;
> @@ -114,6 +131,16 @@ struct vhost_virtqueue {
> /* Log write descriptors */
> void __user *log_base;
> struct vhost_log *log;
> + /* vhost zerocopy support fields below: */
> + /* last used idx for outstanding DMA zerocopy buffers */
> + int upend_idx;
> + /* first used idx for DMA done zerocopy buffers */
> + int done_idx;
> + /* an array of userspace buffers info */
> + struct ubuf_info *ubuf_info;
> + /* Reference counting for outstanding ubufs.
> + * Protected by vq mutex. Writers must also take device mutex. */
> + struct vhost_ubuf_ref *ubufs;
> };
>
> struct vhost_dev {
> @@ -160,6 +187,8 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_zerocopy_callback(void *arg);
> +int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply
* Re: [PATCHv9] vhost: experimental tx zero-copy support
From: Michael S. Tsirkin @ 2011-07-18 7:16 UTC (permalink / raw)
To: Jesper Juhl; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <alpine.LNX.2.00.1107172153030.32359@swampdragon.chaosbits.net>
On Sun, Jul 17, 2011 at 10:01:41PM +0200, Jesper Juhl wrote:
> > @@ -28,10 +29,18 @@
> >
> > #include "vhost.h"
> >
> > +static int zcopytx;
> > +module_param(zcopytx, int, 0444);
>
> Should everyone be able to read this? How about "0440" just to be
> paranoid? or?
I find it very helpful to have the parameter visible in sysfs.
Given that:
[mst@tuck linux-2.6]$ grep module_param drivers/net/*c|grep [64]44|wc -l
14
[mst@tuck linux-2.6]$ grep module_param drivers/net/*c|grep [64]40|wc -l
0
[mst@tuck linux-2.6]$ grep module_param drivers/net/*c|grep [64]00|wc -l
7
So at least the precedent is against 0440. What do you think?
--
MST
^ permalink raw reply
* [PATCHv10] vhost: vhost TX zero-copy support
From: Michael S. Tsirkin @ 2011-07-18 10:59 UTC (permalink / raw)
To: Shirley Ma; +Cc: netdev, jj, linux-kernel, kvm, virtualization
This adds experimental zero copy support in vhost-net,
disabled by default. To enable, set the zerocopytx
module option to 1.
This patch maintains the outstanding userspace buffers in the
sequence it is delivered to vhost. The outstanding userspace buffers
will be marked as done once the lower device buffers DMA has finished.
This is monitored through last reference of kfree_skb callback. Two
buffer indices are used for this purpose.
The vhost-net device passes the userspace buffers info to lower device
skb through message control. DMA done status check and guest
notification are handled by handle_tx: in the worst case is all buffers
in the vq are in pending/done status, so we need to notify guest to
release DMA done buffers first before we get any new buffers from the
vq.
One known problem is that if the guest stops submitting
buffers, buffers might never get used until some
further action, e.g. device reset. This does not
seem to affect linux guests.
Signed-off-by: Shirley <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v9: coding style prettification suggested
by Jesper Juhl.
drivers/vhost/net.c | 73 ++++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 29 +++++++++++++++++
3 files changed, 185 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e224a92..226ca6b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -12,6 +12,7 @@
#include <linux/virtio_net.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/workqueue.h>
#include <linux/rcupdate.h>
@@ -28,10 +29,18 @@
#include "vhost.h"
+static int zcopytx;
+module_param(zcopytx, int, 0444);
+MODULE_PARM_DESC(lnksts, "Enable Zero Copy Transmit");
+
/* Max number of bytes transferred before requeueing the job.
* Using this limit prevents one virtqueue from starving others. */
#define VHOST_NET_WEIGHT 0x80000
+/* MAX number of TX used buffers for outstanding zerocopy */
+#define VHOST_MAX_PEND 128
+#define VHOST_GOODCOPY_LEN 256
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -54,6 +63,11 @@ struct vhost_net {
enum vhost_net_poll_state tx_poll_state;
};
+static bool vhost_sock_zcopy(struct socket *sock)
+{
+ return unlikely(zcopytx) && sock_flag(sock->sk, SOCK_ZEROCOPY);
+}
+
/* Pop first len bytes from iovec. Return number of segments used. */
static int move_iovec_hdr(struct iovec *from, struct iovec *to,
size_t len, int iov_count)
@@ -129,6 +143,8 @@ static void handle_tx(struct vhost_net *net)
int err, wmem;
size_t hdr_size;
struct socket *sock;
+ struct vhost_ubuf_ref *uninitialized_var(ubufs);
+ bool zcopy;
/* TODO: check that we are running from vhost_worker? */
sock = rcu_dereference_check(vq->private_data, 1);
@@ -149,8 +165,13 @@ static void handle_tx(struct vhost_net *net)
if (wmem < sock->sk->sk_sndbuf / 2)
tx_poll_stop(net);
hdr_size = vq->vhost_hlen;
+ zcopy = vhost_sock_zcopy(sock);
for (;;) {
+ /* Release DMAs done buffers first */
+ if (zcopy)
+ vhost_zerocopy_signal_used(vq);
+
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
@@ -166,6 +187,12 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
+ /* If more outstanding DMAs, queue the work */
+ if (vq->upend_idx - vq->done_idx > VHOST_MAX_PEND) {
+ tx_poll_start(net, sock);
+ set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
+ break;
+ }
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
vhost_disable_notify(&net->dev, vq);
continue;
@@ -188,9 +215,39 @@ static void handle_tx(struct vhost_net *net)
iov_length(vq->hdr, s), hdr_size);
break;
}
+ /* use msg_control to pass vhost zerocopy ubuf info to skb */
+ if (zcopy) {
+ vq->heads[vq->upend_idx].id = head;
+ if (len < VHOST_GOODCOPY_LEN) {
+ /* copy don't need to wait for DMA done */
+ vq->heads[vq->upend_idx].len =
+ VHOST_DMA_DONE_LEN;
+ msg.msg_control = NULL;
+ msg.msg_controllen = 0;
+ ubufs = NULL;
+ } else {
+ struct ubuf_info *ubuf = &vq->ubuf_info[head];
+
+ vq->heads[vq->upend_idx].len = len;
+ ubuf->callback = vhost_zerocopy_callback;
+ ubuf->arg = vq->ubufs;
+ ubuf->desc = vq->upend_idx;
+ msg.msg_control = ubuf;
+ msg.msg_controllen = sizeof(ubuf);
+ ubufs = vq->ubufs;
+ kref_get(&ubufs->kref);
+ }
+ vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
+ }
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
+ if (zcopy) {
+ if (ubufs)
+ vhost_ubuf_put(ubufs);
+ vq->upend_idx = ((unsigned)vq->upend_idx - 1) %
+ UIO_MAXIOV;
+ }
vhost_discard_vq_desc(vq, 1);
tx_poll_start(net, sock);
break;
@@ -198,7 +255,8 @@ static void handle_tx(struct vhost_net *net)
if (err != len)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ if (!zcopy)
+ vhost_add_used_and_signal(&net->dev, vq, head, 0);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
@@ -603,6 +661,7 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
{
struct socket *sock, *oldsock;
struct vhost_virtqueue *vq;
+ struct vhost_ubuf_ref *ubufs, *oldubufs = NULL;
int r;
mutex_lock(&n->dev.mutex);
@@ -632,6 +691,13 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
oldsock = rcu_dereference_protected(vq->private_data,
lockdep_is_held(&vq->mutex));
if (sock != oldsock) {
+ ubufs = vhost_ubuf_alloc(vq, sock && vhost_sock_zcopy(sock));
+ if (IS_ERR(ubufs)) {
+ r = PTR_ERR(ubufs);
+ goto err_ubufs;
+ }
+ oldubufs = vq->ubufs;
+ vq->ubufs = ubufs;
vhost_net_disable_vq(n, vq);
rcu_assign_pointer(vq->private_data, sock);
vhost_net_enable_vq(n, vq);
@@ -639,6 +705,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&vq->mutex);
+ if (oldubufs)
+ vhost_ubuf_put_and_wait(oldubufs);
+
if (oldsock) {
vhost_net_flush_vq(n, index);
fput(oldsock->file);
@@ -647,6 +716,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&n->dev.mutex);
return 0;
+err_ubufs:
+ fput(sock->file);
err_vq:
mutex_unlock(&vq->mutex);
err:
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ea966b3..de79aaa 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -179,6 +179,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->upend_idx = 0;
+ vq->done_idx = 0;
+ vq->ubufs = NULL;
}
static int vhost_worker(void *data)
@@ -237,6 +240,8 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
GFP_KERNEL);
dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
UIO_MAXIOV, GFP_KERNEL);
+ dev->vqs[i].ubuf_info = kmalloc(sizeof *dev->vqs[i].ubuf_info *
+ UIO_MAXIOV, GFP_KERNEL);
if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
!dev->vqs[i].heads)
@@ -249,6 +254,7 @@ err_nomem:
kfree(dev->vqs[i].indirect);
kfree(dev->vqs[i].log);
kfree(dev->vqs[i].heads);
+ kfree(dev->vqs[i].ubuf_info);
}
return -ENOMEM;
}
@@ -390,6 +396,30 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
+/* In case of DMA done not in order in lower device driver for some reason.
+ * upend_idx is used to track end of used idx, done_idx is used to track head
+ * of used idx. Once lower device DMA done contiguously, we will signal KVM
+ * guest used idx.
+ */
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+{
+ int i;
+ int j = 0;
+
+ for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
+ if ((vq->heads[i].len == VHOST_DMA_DONE_LEN)) {
+ vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
+ vhost_add_used_and_signal(vq->dev, vq,
+ vq->heads[i].id, 0);
+ ++j;
+ } else
+ break;
+ }
+ if (j)
+ vq->done_idx = i;
+ return j;
+}
+
/* Caller should have device mutex */
void vhost_dev_cleanup(struct vhost_dev *dev)
{
@@ -400,6 +430,13 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
vhost_poll_stop(&dev->vqs[i].poll);
vhost_poll_flush(&dev->vqs[i].poll);
}
+ /* Wait for all lower device DMAs done. */
+ if (dev->vqs[i].ubufs)
+ vhost_ubuf_put_and_wait(dev->vqs[i].ubufs);
+
+ /* Signal guest as appropriate. */
+ vhost_zerocopy_signal_used(&dev->vqs[i]);
+
if (dev->vqs[i].error_ctx)
eventfd_ctx_put(dev->vqs[i].error_ctx);
if (dev->vqs[i].error)
@@ -1486,3 +1523,50 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
&vq->used->flags, r);
}
}
+
+static void vhost_zerocopy_done_signal(struct kref *kref)
+{
+ struct vhost_ubuf_ref *ubufs = container_of(kref, struct vhost_ubuf_ref,
+ kref);
+ wake_up(&ubufs->wait);
+}
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *vq,
+ bool zcopy)
+{
+ struct vhost_ubuf_ref *ubufs;
+ /* No zero copy backend? Nothing to count. */
+ if (!zcopy)
+ return NULL;
+ ubufs = kmalloc(sizeof *ubufs, GFP_KERNEL);
+ if (!ubufs)
+ return ERR_PTR(-ENOMEM);
+ kref_init(&ubufs->kref);
+ kref_get(&ubufs->kref);
+ init_waitqueue_head(&ubufs->wait);
+ ubufs->vq = vq;
+ return ubufs;
+}
+
+void vhost_ubuf_put(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
+
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+ wait_event(ubufs->wait, !atomic_read(&ubufs->kref.refcount));
+ kfree(ubufs);
+}
+
+void vhost_zerocopy_callback(void *arg)
+{
+ struct ubuf_info *ubuf = arg;
+ struct vhost_ubuf_ref *ubufs = ubuf->arg;
+ struct vhost_virtqueue *vq = ubufs->vq;
+
+ /* set len = 1 to mark this desc buffers done DMA */
+ vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8e03379..e287145 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -13,6 +13,11 @@
#include <linux/virtio_ring.h>
#include <asm/atomic.h>
+/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
+ * done */
+#define VHOST_DMA_DONE_LEN 1
+#define VHOST_DMA_CLEAR_LEN 0
+
struct vhost_device;
struct vhost_work;
@@ -50,6 +55,18 @@ struct vhost_log {
u64 len;
};
+struct vhost_virtqueue;
+
+struct vhost_ubuf_ref {
+ struct kref kref;
+ wait_queue_t wait;
+ struct vhost_virtqueue *vq;
+};
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *, bool zcopy);
+void vhost_ubuf_put(struct vhost_ubuf_ref *);
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *);
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -114,6 +131,16 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+ /* vhost zerocopy support fields below: */
+ /* last used idx for outstanding DMA zerocopy buffers */
+ int upend_idx;
+ /* first used idx for DMA done zerocopy buffers */
+ int done_idx;
+ /* an array of userspace buffers info */
+ struct ubuf_info *ubuf_info;
+ /* Reference counting for outstanding ubufs.
+ * Protected by vq mutex. Writers must also take device mutex. */
+ struct vhost_ubuf_ref *ubufs;
};
struct vhost_dev {
@@ -160,6 +187,8 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_zerocopy_callback(void *arg);
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCHv10] vhost: vhost TX zero-copy support
From: Michael S. Tsirkin @ 2011-07-18 12:28 UTC (permalink / raw)
To: Shirley Ma; +Cc: netdev, jj, linux-kernel, kvm, virtualization
In-Reply-To: <20110718105905.GA4786@redhat.com>
On Mon, Jul 18, 2011 at 01:59:05PM +0300, Michael S. Tsirkin wrote:
> This adds experimental zero copy support in vhost-net,
> disabled by default.
Looks like there was a memory leak in the patch,
I'll address that and post a new version shortly.
--
MST
^ permalink raw reply
* [PATCHv11] vhost: vhost TX zero-copy support
From: Michael S. Tsirkin @ 2011-07-18 13:48 UTC (permalink / raw)
To: Shirley Ma; +Cc: netdev, jj, linux-kernel, kvm, virtualization
>From: Shirley Ma <mashirle@us.ibm.com>
This adds experimental zero copy support in vhost-net,
disabled by default. To enable, set
experimental_zcopytx module option to 1.
This patch maintains the outstanding userspace buffers in the
sequence it is delivered to vhost. The outstanding userspace buffers
will be marked as done once the lower device buffers DMA has finished.
This is monitored through last reference of kfree_skb callback. Two
buffer indices are used for this purpose.
The vhost-net device passes the userspace buffers info to lower device
skb through message control. DMA done status check and guest
notification are handled by handle_tx: in the worst case is all buffers
in the vq are in pending/done status, so we need to notify guest to
release DMA done buffers first before we get any new buffers from the
vq.
One known problem is that if the guest stops submitting
buffers, buffers might never get used until some
further action, e.g. device reset. This does not
seem to affect linux guests.
Signed-off-by: Shirley <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I run some light tests and this seems to work fine for me. Given that
the new functionality is disabled by default, it seems a pretty safe
patch to merge for 3.1.
Changes from v10:
don't leak ubuf_info
don't allocate ubuf_info for non zero copy vqs
Changes from v9:
coding style prettification suggested by Jesper Juhl
drivers/vhost/net.c | 77 +++++++++++++++++++++++++++++-
drivers/vhost/vhost.c | 128 +++++++++++++++++++++++++++++++++++++++++++------
drivers/vhost/vhost.h | 31 ++++++++++++
3 files changed, 220 insertions(+), 16 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e224a92..f0fd52c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -12,6 +12,7 @@
#include <linux/virtio_net.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/workqueue.h>
#include <linux/rcupdate.h>
@@ -28,10 +29,18 @@
#include "vhost.h"
+static int experimental_zcopytx;
+module_param(experimental_zcopytx, int, 0444);
+MODULE_PARM_DESC(experimental_zcopytx, "Enable Experimental Zero Copy TX");
+
/* Max number of bytes transferred before requeueing the job.
* Using this limit prevents one virtqueue from starving others. */
#define VHOST_NET_WEIGHT 0x80000
+/* MAX number of TX used buffers for outstanding zerocopy */
+#define VHOST_MAX_PEND 128
+#define VHOST_GOODCOPY_LEN 256
+
enum {
VHOST_NET_VQ_RX = 0,
VHOST_NET_VQ_TX = 1,
@@ -54,6 +63,12 @@ struct vhost_net {
enum vhost_net_poll_state tx_poll_state;
};
+static bool vhost_sock_zcopy(struct socket *sock)
+{
+ return unlikely(experimental_zcopytx) &&
+ sock_flag(sock->sk, SOCK_ZEROCOPY);
+}
+
/* Pop first len bytes from iovec. Return number of segments used. */
static int move_iovec_hdr(struct iovec *from, struct iovec *to,
size_t len, int iov_count)
@@ -129,6 +144,8 @@ static void handle_tx(struct vhost_net *net)
int err, wmem;
size_t hdr_size;
struct socket *sock;
+ struct vhost_ubuf_ref *uninitialized_var(ubufs);
+ bool zcopy;
/* TODO: check that we are running from vhost_worker? */
sock = rcu_dereference_check(vq->private_data, 1);
@@ -149,8 +166,13 @@ static void handle_tx(struct vhost_net *net)
if (wmem < sock->sk->sk_sndbuf / 2)
tx_poll_stop(net);
hdr_size = vq->vhost_hlen;
+ zcopy = vhost_sock_zcopy(sock);
for (;;) {
+ /* Release DMAs done buffers first */
+ if (zcopy)
+ vhost_zerocopy_signal_used(vq);
+
head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
ARRAY_SIZE(vq->iov),
&out, &in,
@@ -166,6 +188,13 @@ static void handle_tx(struct vhost_net *net)
set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
break;
}
+ /* If more outstanding DMAs, queue the work */
+ if (unlikely(vq->upend_idx - vq->done_idx >
+ VHOST_MAX_PEND)) {
+ tx_poll_start(net, sock);
+ set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
+ break;
+ }
if (unlikely(vhost_enable_notify(&net->dev, vq))) {
vhost_disable_notify(&net->dev, vq);
continue;
@@ -188,9 +217,39 @@ static void handle_tx(struct vhost_net *net)
iov_length(vq->hdr, s), hdr_size);
break;
}
+ /* use msg_control to pass vhost zerocopy ubuf info to skb */
+ if (zcopy) {
+ vq->heads[vq->upend_idx].id = head;
+ if (len < VHOST_GOODCOPY_LEN) {
+ /* copy don't need to wait for DMA done */
+ vq->heads[vq->upend_idx].len =
+ VHOST_DMA_DONE_LEN;
+ msg.msg_control = NULL;
+ msg.msg_controllen = 0;
+ ubufs = NULL;
+ } else {
+ struct ubuf_info *ubuf = &vq->ubuf_info[head];
+
+ vq->heads[vq->upend_idx].len = len;
+ ubuf->callback = vhost_zerocopy_callback;
+ ubuf->arg = vq->ubufs;
+ ubuf->desc = vq->upend_idx;
+ msg.msg_control = ubuf;
+ msg.msg_controllen = sizeof(ubuf);
+ ubufs = vq->ubufs;
+ kref_get(&ubufs->kref);
+ }
+ vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
+ }
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
+ if (zcopy) {
+ if (ubufs)
+ vhost_ubuf_put(ubufs);
+ vq->upend_idx = ((unsigned)vq->upend_idx - 1) %
+ UIO_MAXIOV;
+ }
vhost_discard_vq_desc(vq, 1);
tx_poll_start(net, sock);
break;
@@ -198,7 +257,8 @@ static void handle_tx(struct vhost_net *net)
if (err != len)
pr_debug("Truncated TX packet: "
" len %d != %zd\n", err, len);
- vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ if (!zcopy)
+ vhost_add_used_and_signal(&net->dev, vq, head, 0);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
@@ -603,6 +663,7 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
{
struct socket *sock, *oldsock;
struct vhost_virtqueue *vq;
+ struct vhost_ubuf_ref *ubufs, *oldubufs = NULL;
int r;
mutex_lock(&n->dev.mutex);
@@ -632,6 +693,13 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
oldsock = rcu_dereference_protected(vq->private_data,
lockdep_is_held(&vq->mutex));
if (sock != oldsock) {
+ ubufs = vhost_ubuf_alloc(vq, sock && vhost_sock_zcopy(sock));
+ if (IS_ERR(ubufs)) {
+ r = PTR_ERR(ubufs);
+ goto err_ubufs;
+ }
+ oldubufs = vq->ubufs;
+ vq->ubufs = ubufs;
vhost_net_disable_vq(n, vq);
rcu_assign_pointer(vq->private_data, sock);
vhost_net_enable_vq(n, vq);
@@ -639,6 +707,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&vq->mutex);
+ if (oldubufs)
+ vhost_ubuf_put_and_wait(oldubufs);
+
if (oldsock) {
vhost_net_flush_vq(n, index);
fput(oldsock->file);
@@ -647,6 +718,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&n->dev.mutex);
return 0;
+err_ubufs:
+ fput(sock->file);
err_vq:
mutex_unlock(&vq->mutex);
err:
@@ -776,6 +849,8 @@ static struct miscdevice vhost_net_misc = {
static int vhost_net_init(void)
{
+ if (experimental_zcopytx)
+ vhost_enable_zcopy(VHOST_NET_VQ_TX);
return misc_register(&vhost_net_misc);
}
module_init(vhost_net_init);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ea966b3..5ef2f62 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -37,6 +37,8 @@ enum {
VHOST_MEMORY_F_LOG = 0x1,
};
+static unsigned vhost_zcopy_mask __read_mostly;
+
#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
#define vhost_avail_event(vq) ((u16 __user *)&vq->used->ring[vq->num])
@@ -179,6 +181,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call_ctx = NULL;
vq->call = NULL;
vq->log_ctx = NULL;
+ vq->upend_idx = 0;
+ vq->done_idx = 0;
+ vq->ubufs = NULL;
}
static int vhost_worker(void *data)
@@ -225,10 +230,28 @@ static int vhost_worker(void *data)
return 0;
}
+static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
+{
+ kfree(vq->indirect);
+ vq->indirect = NULL;
+ kfree(vq->log);
+ vq->log = NULL;
+ kfree(vq->heads);
+ vq->heads = NULL;
+ kfree(vq->ubuf_info);
+ vq->ubuf_info = NULL;
+}
+
+void vhost_enable_zcopy(int vq)
+{
+ vhost_zcopy_mask |= 0x1 << vq;
+}
+
/* Helper to allocate iovec buffers for all vqs. */
static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
{
int i;
+ bool zcopy;
for (i = 0; i < dev->nvqs; ++i) {
dev->vqs[i].indirect = kmalloc(sizeof *dev->vqs[i].indirect *
@@ -237,19 +260,21 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
GFP_KERNEL);
dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
UIO_MAXIOV, GFP_KERNEL);
-
+ zcopy = vhost_zcopy_mask & (0x1 << i);
+ if (zcopy)
+ dev->vqs[i].ubuf_info =
+ kmalloc(sizeof *dev->vqs[i].ubuf_info *
+ UIO_MAXIOV, GFP_KERNEL);
if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
- !dev->vqs[i].heads)
+ !dev->vqs[i].heads ||
+ (zcopy && !dev->vqs[i].ubuf_info))
goto err_nomem;
}
return 0;
err_nomem:
- for (; i >= 0; --i) {
- kfree(dev->vqs[i].indirect);
- kfree(dev->vqs[i].log);
- kfree(dev->vqs[i].heads);
- }
+ for (; i >= 0; --i)
+ vhost_vq_free_iovecs(&dev->vqs[i]);
return -ENOMEM;
}
@@ -257,14 +282,8 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev)
{
int i;
- for (i = 0; i < dev->nvqs; ++i) {
- kfree(dev->vqs[i].indirect);
- dev->vqs[i].indirect = NULL;
- kfree(dev->vqs[i].log);
- dev->vqs[i].log = NULL;
- kfree(dev->vqs[i].heads);
- dev->vqs[i].heads = NULL;
- }
+ for (i = 0; i < dev->nvqs; ++i)
+ vhost_vq_free_iovecs(&dev->vqs[i]);
}
long vhost_dev_init(struct vhost_dev *dev,
@@ -287,6 +306,7 @@ long vhost_dev_init(struct vhost_dev *dev,
dev->vqs[i].log = NULL;
dev->vqs[i].indirect = NULL;
dev->vqs[i].heads = NULL;
+ dev->vqs[i].ubuf_info = NULL;
dev->vqs[i].dev = dev;
mutex_init(&dev->vqs[i].mutex);
vhost_vq_reset(dev, dev->vqs + i);
@@ -390,6 +410,30 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
return 0;
}
+/* In case of DMA done not in order in lower device driver for some reason.
+ * upend_idx is used to track end of used idx, done_idx is used to track head
+ * of used idx. Once lower device DMA done contiguously, we will signal KVM
+ * guest used idx.
+ */
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq)
+{
+ int i;
+ int j = 0;
+
+ for (i = vq->done_idx; i != vq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
+ if ((vq->heads[i].len == VHOST_DMA_DONE_LEN)) {
+ vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
+ vhost_add_used_and_signal(vq->dev, vq,
+ vq->heads[i].id, 0);
+ ++j;
+ } else
+ break;
+ }
+ if (j)
+ vq->done_idx = i;
+ return j;
+}
+
/* Caller should have device mutex */
void vhost_dev_cleanup(struct vhost_dev *dev)
{
@@ -400,6 +444,13 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
vhost_poll_stop(&dev->vqs[i].poll);
vhost_poll_flush(&dev->vqs[i].poll);
}
+ /* Wait for all lower device DMAs done. */
+ if (dev->vqs[i].ubufs)
+ vhost_ubuf_put_and_wait(dev->vqs[i].ubufs);
+
+ /* Signal guest as appropriate. */
+ vhost_zerocopy_signal_used(&dev->vqs[i]);
+
if (dev->vqs[i].error_ctx)
eventfd_ctx_put(dev->vqs[i].error_ctx);
if (dev->vqs[i].error)
@@ -1486,3 +1537,50 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
&vq->used->flags, r);
}
}
+
+static void vhost_zerocopy_done_signal(struct kref *kref)
+{
+ struct vhost_ubuf_ref *ubufs = container_of(kref, struct vhost_ubuf_ref,
+ kref);
+ wake_up(&ubufs->wait);
+}
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *vq,
+ bool zcopy)
+{
+ struct vhost_ubuf_ref *ubufs;
+ /* No zero copy backend? Nothing to count. */
+ if (!zcopy)
+ return NULL;
+ ubufs = kmalloc(sizeof *ubufs, GFP_KERNEL);
+ if (!ubufs)
+ return ERR_PTR(-ENOMEM);
+ kref_init(&ubufs->kref);
+ kref_get(&ubufs->kref);
+ init_waitqueue_head(&ubufs->wait);
+ ubufs->vq = vq;
+ return ubufs;
+}
+
+void vhost_ubuf_put(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
+
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *ubufs)
+{
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+ wait_event(ubufs->wait, !atomic_read(&ubufs->kref.refcount));
+ kfree(ubufs);
+}
+
+void vhost_zerocopy_callback(void *arg)
+{
+ struct ubuf_info *ubuf = arg;
+ struct vhost_ubuf_ref *ubufs = ubuf->arg;
+ struct vhost_virtqueue *vq = ubufs->vq;
+
+ /* set len = 1 to mark this desc buffers done DMA */
+ vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
+ kref_put(&ubufs->kref, vhost_zerocopy_done_signal);
+}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8e03379..1544b78 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -13,6 +13,11 @@
#include <linux/virtio_ring.h>
#include <asm/atomic.h>
+/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
+ * done */
+#define VHOST_DMA_DONE_LEN 1
+#define VHOST_DMA_CLEAR_LEN 0
+
struct vhost_device;
struct vhost_work;
@@ -50,6 +55,18 @@ struct vhost_log {
u64 len;
};
+struct vhost_virtqueue;
+
+struct vhost_ubuf_ref {
+ struct kref kref;
+ wait_queue_head_t wait;
+ struct vhost_virtqueue *vq;
+};
+
+struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *, bool zcopy);
+void vhost_ubuf_put(struct vhost_ubuf_ref *);
+void vhost_ubuf_put_and_wait(struct vhost_ubuf_ref *);
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -114,6 +131,16 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+ /* vhost zerocopy support fields below: */
+ /* last used idx for outstanding DMA zerocopy buffers */
+ int upend_idx;
+ /* first used idx for DMA done zerocopy buffers */
+ int done_idx;
+ /* an array of userspace buffers info */
+ struct ubuf_info *ubuf_info;
+ /* Reference counting for outstanding ubufs.
+ * Protected by vq mutex. Writers must also take device mutex. */
+ struct vhost_ubuf_ref *ubufs;
};
struct vhost_dev {
@@ -160,6 +187,8 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_zerocopy_callback(void *arg);
+int vhost_zerocopy_signal_used(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
@@ -186,4 +215,6 @@ static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
return acked_features & (1 << bit);
}
+void vhost_enable_zcopy(int vq);
+
#endif
--
1.7.5.53.gc233e
^ 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