Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
On Thu, May 16, 2013 at 01:04:58PM +0930, Rusty Russell wrote:
Asias He as...@redhat.com writes:
On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
Asias He as...@redhat.com writes:
scsi.c includes vhost.c which uses memcpy_fromiovec.
This patch fixes this build failure.
From Randy Dunlap:
'''
on x86_64:
ERROR: memcpy_fromiovec [drivers/vhost/vhost_scsi.ko] undefined!
It needs to depend on NET since net/core/ provides that function.
'''
Proper fix please.
--verbose please ;-)
Making VHOST_SCSI depends on NET looks weird but this is because vhost
core depends on it. A bunch of patches are cleaning this up. Since MST
wanted do the vhost.ko split up in 3.11, plus your WIP vringh work, so I
wanted the fix for 3.10 as minimum as possible.
If this isn't the only symbol causing the problem, then this should be
mentioned in the changelog. If it is, it should be fixed: we have
plenty of time for that.
Either way, your commit message or the commit itself needs to justify
it!
Other users are using memcpy_fromiovec and friends outside net. It seems
a good idea to put it in a util library. e.g. crypto/algif_skcipher.c
which also depends on NET for it.
Though I can't see why you thought this was a good idea. Nonetheless, I
shan't highlight why: I have far too much respect for your intellects
and abilities.
No, don't thank me!
Interesting.
Heh... I originally wrote an explanation, then found it a bit insulting:
I knew I didn't need to tell you :)
How's this?
From: Rusty Russell ru...@rustcorp.com.au
Subject: Hoist memcpy_fromiovec into lib/
ERROR: memcpy_fromiovec [drivers/vhost/vhost_scsi.ko] undefined!
That function is only present with CONFIG_NET. Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.
socket.h already include uio.h, so no callers need updating.
Reported-by: Randy Dunlap rdun...@infradead.org
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Acked-by: Michael S. Tsirkin m...@redhat.com
Would you like me to merge this through the vhost tree?
If I do I can drop #include linux/socket.h from vhost.c
right now.
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..7266775 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct
ucred *ucred);
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int
len);
extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int offset, int len);
extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..21628d3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov,
unsigned long nr_segs)
}
unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t
to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
#endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o dump_stack.o timerqueue.o\
- idr.o int_sqrt.o extable.o \
+ idr.o int_sqrt.o extable.o iovec.o \
sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 000..632c5ea
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,29 @@
+#include linux/uaccess.h
+#include linux/export.h
+#include linux/uio.h
+
+/*
+ * Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ * Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+ while (len 0) {
+ if (iov-iov_len) {
+ int copy = min_t(unsigned int, len, iov-iov_len);
+ if (copy_from_user(kdata, iov-iov_base, copy))
+ return -EFAULT;
+ len -= copy;
+ kdata += copy;
+ iov-iov_base += copy;
+ iov-iov_len -= copy;
+ }
+ iov++;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(memcpy_fromiovec);
+
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..d81257f 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned
char *kdata,
EXPORT_SYMBOL(memcpy_toiovecend);
/*
- * Copy
Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
On Wed, May 15, 2013 at 03:37:30PM -0700, Nicholas A. Bellinger wrote:
On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
Asias He as...@redhat.com writes:
scsi.c includes vhost.c which uses memcpy_fromiovec.
This patch fixes this build failure.
From Randy Dunlap:
'''
on x86_64:
ERROR: memcpy_fromiovec [drivers/vhost/vhost_scsi.ko] undefined!
It needs to depend on NET since net/core/ provides that function.
'''
Proper fix please.
Though I can't see why you thought this was a good idea. Nonetheless, I
shan't highlight why: I have far too much respect for your intellects
and abilities.
No, don't thank me!
Hi Rusty Asias,
I assume you mean something like the following patch to allow kbuild to
work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
yes..?
Also included is dropping the now unnecessary vhost.c include, and
allowing vhost_work_flush() to be accessed externally as scsi.c
currently requires.
MST, care to pick this up..?
--nab
We'll do this for 3.11, 3.10 is bugfixes only now.
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 8b9226d..016387f 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -1,3 +1,6 @@
+config VHOST
+ tristate
+
config VHOST_NET
tristate Host kernel accelerator for virtio net
depends on NET EVENTFD (TUN || !TUN) (MACVTAP || !MACVTAP)
@@ -12,7 +15,7 @@ config VHOST_NET
config VHOST_SCSI
tristate VHOST_SCSI TCM fabric driver
- depends on TARGET_CORE EVENTFD m
+ depends on NET EVENTFD TARGET_CORE
select VHOST_RING
default n
---help---
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 654e9afb..e5b5f0b 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -1,7 +1,9 @@
+obj-$(CONFIG_VHOST) += vhost.o
+
obj-$(CONFIG_VHOST_NET) += vhost_net.o
-vhost_net-y := vhost.o net.o
+vhost_net-objs := net.o
obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o
-vhost_scsi-y := scsi.o
+vhost_scsi-objs := scsi.o
obj-$(CONFIG_VHOST_RING) += vringh.o
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 7014202..b5836a2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -49,7 +49,6 @@
#include linux/llist.h
#include linux/bitmap.h
-#include vhost.c
#include vhost.h
#define TCM_VHOST_VERSION v0.1
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index beee7f5..8cd1562 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -123,7 +123,7 @@ static bool vhost_work_seq_done(struct vhost_dev *dev,
struct vhost_work *work,
return left = 0;
}
-static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
+void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
{
unsigned seq;
int flushing;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a7ad635..50ee396 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -44,6 +44,7 @@ void vhost_poll_init(struct vhost_poll *poll,
vhost_work_fn_t fn,
unsigned long mask, struct vhost_dev *dev);
int vhost_poll_start(struct vhost_poll *poll, struct file *file);
void vhost_poll_stop(struct vhost_poll *poll);
+void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work);
void vhost_poll_flush(struct vhost_poll *poll);
void vhost_poll_queue(struct vhost_poll *poll);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
On Wed, May 15, 2013 at 08:10:55PM -0700, David Miller wrote: From: Rusty Russell ru...@rustcorp.com.au Date: Thu, 16 May 2013 09:05:38 +0930 memcpy_fromiovec() has nothing to do with networking: that was just the first user. Note that crypto/algif_skcipher.c also uses it. The obvious answer is to move it into lib/. +1 Rusty sent a patch that does this: http://patchwork.ozlabs.org/patch/244207/ David, looks like you weren't CC'd. If you agree could you please Ack that patch and then I can merge it through the vhost tree? Or if you prefer merge it directly and I'll sort out the dependencies... Thanks, -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
From: Michael S. Tsirkin m...@redhat.com Date: Thu, 16 May 2013 09:46:21 +0300 On Wed, May 15, 2013 at 08:10:55PM -0700, David Miller wrote: From: Rusty Russell ru...@rustcorp.com.au Date: Thu, 16 May 2013 09:05:38 +0930 memcpy_fromiovec() has nothing to do with networking: that was just the first user. Note that crypto/algif_skcipher.c also uses it. The obvious answer is to move it into lib/. +1 Rusty sent a patch that does this: http://patchwork.ozlabs.org/patch/244207/ David, looks like you weren't CC'd. If you agree could you please Ack that patch and then I can merge it through the vhost tree? Or if you prefer merge it directly and I'll sort out the dependencies... Acked-by: David S. Miller da...@davemloft.net ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[RFC PATCH v2, part3 07/11] PCI, xen-pcifront: use new PCI interfaces to simplify implementation
Use new PCI interfaces to simplify xen-pcifront implementation:
1) Use pci_create_root_bus() instead of pci_scan_bus_parented()
because pci_scan_bus_parented() is marked as __deprecated.This
also gets rid of a duplicated call of pci_bus_start_devices().
2) Use pci_stop_root_bus() and pci_remove_root_bus() instead of
open-coded private implementation.
3) Use pci_set_host_bridge_release() to release data structures
associated with PCI root buses.
4) Use pci_bus_get()/pci_bus_put() to manage PCI root bus reference
count.
This is also a preparation for coming PCI bus lock enhancement.
Signed-off-by: Jiang Liu jiang@huawei.com
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Jeremy Fitzhardinge jer...@goop.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/pci/xen-pcifront.c | 81 ++
1 file changed, 39 insertions(+), 42 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 816cf94..6aa2c0f 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -25,11 +25,6 @@
#define INVALID_GRANT_REF (0)
#define INVALID_EVTCHN(-1)
-struct pci_bus_entry {
- struct list_head list;
- struct pci_bus *bus;
-};
-
#define _PDEVB_op_active (0)
#define PDEVB_op_active(1 (_PDEVB_op_active))
@@ -47,12 +42,12 @@ struct pcifront_device {
struct xen_pci_sharedinfo *sh_info;
struct work_struct op_work;
unsigned long flags;
-
};
struct pcifront_sd {
int domain;
struct pcifront_device *pdev;
+ struct resource busn_res;
};
static inline struct pcifront_device *
@@ -67,6 +62,12 @@ static inline void pcifront_init_sd(struct pcifront_sd *sd,
{
sd-domain = domain;
sd-pdev = pdev;
+
+ /* Xen pci-backend doesn't export P2P bridges */
+ sd-busn_res.start = bus;
+ sd-busn_res.end = bus;
+ sd-busn_res.flags = IORESOURCE_BUS;
+ sd-busn_res.name = PCI busn;
}
static DEFINE_SPINLOCK(pcifront_dev_lock);
@@ -441,12 +442,19 @@ static int pcifront_scan_bus(struct pcifront_device *pdev,
return 0;
}
+static void pcifront_release_sd(struct pci_host_bridge *bridge)
+{
+ struct pcifront_sd *sd = bridge-release_data;
+
+ kfree(sd);
+}
+
static int pcifront_scan_root(struct pcifront_device *pdev,
unsigned int domain, unsigned int bus)
{
struct pci_bus *b;
struct pcifront_sd *sd = NULL;
- struct pci_bus_entry *bus_entry = NULL;
+ LIST_HEAD(resources);
int err = 0;
#ifndef CONFIG_PCI_DOMAINS
@@ -463,16 +471,18 @@ static int pcifront_scan_root(struct pcifront_device
*pdev,
dev_info(pdev-xdev-dev, Creating PCI Frontend Bus %04x:%02x\n,
domain, bus);
- bus_entry = kmalloc(sizeof(*bus_entry), GFP_KERNEL);
- sd = kmalloc(sizeof(*sd), GFP_KERNEL);
- if (!bus_entry || !sd) {
+ sd = kzalloc(sizeof(*sd), GFP_KERNEL);
+ if (!sd) {
err = -ENOMEM;
goto err_out;
}
pcifront_init_sd(sd, domain, bus, pdev);
- b = pci_scan_bus_parented(pdev-xdev-dev, bus,
- pcifront_bus_ops, sd);
+ pci_add_resource(resources, ioport_resource);
+ pci_add_resource(resources, iomem_resource);
+ pci_add_resource(resources, sd-busn_res);
+ b = pci_create_root_bus(pdev-xdev-dev, bus, pcifront_bus_ops,
+ sd, resources);
if (!b) {
dev_err(pdev-xdev-dev,
Error creating PCI Frontend Bus!\n);
@@ -480,12 +490,14 @@ static int pcifront_scan_root(struct pcifront_device
*pdev,
goto err_out;
}
- bus_entry-bus = b;
+ pci_set_host_bridge_release(to_pci_host_bridge(b-bridge),
+ pcifront_release_sd, sd);
- list_add(bus_entry-list, pdev-root_buses);
-
- /* pci_scan_bus_parented skips devices which do not have a have
- * devfn==0. The pcifront_scan_bus enumerates all devfn. */
+ /*
+* Every PCI physical device should have function 0, but that's not
+* true for xen.
+* So use pcifront_scan_bus() instead of pci_scan_child_bus().
+*/
err = pcifront_scan_bus(pdev, domain, bus, b);
/* Claim resources before going live with our devices */
@@ -497,7 +509,6 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
return err;
err_out:
- kfree(bus_entry);
kfree(sd);
return err;
@@ -539,35 +550,19 @@ static int pcifront_rescan_root(struct pcifront_device
*pdev,
return err;
}
-static void free_root_bus_devs(struct pci_bus *bus)
-{
- struct pci_dev *dev;
-
- while (!list_empty(bus-devices)) {
[RFC PATCH v2, part3 08/11] PCI, xen-pcifront: use PCI bus lock to protect PCI device hotplug
Use PCI bus lock to protect concurrent PCI device hotplug operations.
Signed-off-by: Jiang Liu jiang@huawei.com
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Cc: Jeremy Fitzhardinge jer...@goop.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/pci/xen-pcifront.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 6aa2c0f..2b213b3 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -505,6 +505,7 @@ static int pcifront_scan_root(struct pcifront_device *pdev,
/* Create SysFS and notify udev of the devices. Aka: going live */
pci_bus_add_devices(b);
+ pci_bus_unlock(b, true);
return err;
@@ -538,6 +539,11 @@ static int pcifront_rescan_root(struct pcifront_device
*pdev,
/* If the bus is unknown, create it. */
return pcifront_scan_root(pdev, domain, bus);
+ if (pci_bus_lock(b, PCI_BUS_STATE_STOPPING - 1, true) 0) {
+ err = -EBUSY;
+ goto out;
+ }
+
err = pcifront_scan_bus(pdev, domain, bus, b);
/* Claim resources before going live with our devices */
@@ -545,6 +551,9 @@ static int pcifront_rescan_root(struct pcifront_device
*pdev,
/* Create SysFS and notify udev of the devices. Aka: going live */
pci_bus_add_devices(b);
+ pci_bus_unlock(b, true);
+
+out:
pci_bus_put(b);
return err;
@@ -559,8 +568,11 @@ static void pcifront_free_roots(struct pcifront_device
*pdev)
for_each_pci_root_bus(bus) {
sd = bus-sysdata;
- if (sd-pdev == pdev) {
+ if (sd-pdev != pdev)
+ continue;
+ if (pci_bus_lock(bus, PCI_BUS_STATE_REMOVED - 1, true) == 0) {
pci_stop_root_bus(bus);
+ /* it also unlocks PCI buses */
pci_remove_root_bus(bus);
}
}
@@ -1042,7 +1054,7 @@ static int pcifront_detach_devices(struct pcifront_device
*pdev)
domain, bus, slot, func);
continue;
}
- pci_stop_and_remove_bus_device(pci_dev);
+ pci_stop_and_remove_device(pci_dev);
pci_dev_put(pci_dev);
dev_dbg(pdev-xdev-dev,
--
1.8.1.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio_pci: fix macro exported in uapi
macro VIRTIO_PCI_CONFIG except in the unlikely event userspace
actually has a structure with a field named msix_enabled.
Get the msix_enabled by value instead, to make it useful
for userspace.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/virtio/virtio_pci.c | 8
include/uapi/linux/virtio_pci.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index a7ce730..1bb6ff3 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -132,8 +132,8 @@ static void vp_get(struct virtio_device *vdev, unsigned
offset,
void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- void __iomem *ioaddr = vp_dev-ioaddr +
- VIRTIO_PCI_CONFIG(vp_dev) + offset;
+ void __iomem *ioaddr = vp_dev-ioaddr + offset +
+ VIRTIO_PCI_CONFIG(vp_dev-msix_enabled);
u8 *ptr = buf;
int i;
@@ -147,8 +147,8 @@ static void vp_set(struct virtio_device *vdev, unsigned
offset,
const void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- void __iomem *ioaddr = vp_dev-ioaddr +
- VIRTIO_PCI_CONFIG(vp_dev) + offset;
+ void __iomem *ioaddr = vp_dev-ioaddr + offset +
+ VIRTIO_PCI_CONFIG(vp_dev-msix_enabled);
const u8 *ptr = buf;
int i;
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index ea66f3f..610c6c6 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -80,7 +80,7 @@
/* The remaining space is defined by each driver as the per-driver
* configuration space */
-#define VIRTIO_PCI_CONFIG(dev) ((dev)-msix_enabled ? 24 : 20)
+#define VIRTIO_PCI_CONFIG(msix_enabled)((msix_enabled) ? 24 : 20)
/* Virtio ABI version, this must match exactly */
#define VIRTIO_PCI_ABI_VERSION 0
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
2013 Linux Plumbers Virtualization Microconference proposal call for participation
Hey folks, We'd like to hold another virtualization microconference as part of this year's Linux Plumbers Conference. To do so, we need to show that there's enough interest, materials, and people willing to attend. Anthony and Amit have already started a wiki page for the microconference: http://wiki.linuxplumbersconf.org/2013:virtualization Please help to fill it out and add topics you're interested in discussing or presenting, or relevant topics to current development that you'd like others to present. We don't need full abstracts or proposals at this time, just enough to show that we have things to discuss and enough people interested in attending to allocate space in the program. If approved by the program committee we'll give precedence to proposals that address current issues needing attention, help, or resolution. This is also an excellent opportunity for cross-project discussions. We're not sure when the LPC program committee will make a decision, so please don't hesitate to add things to the wiki as soon as possible. There's no commitment at this point though we obviously hope that many of you will follow through with participation if this microconference is approved. Please feel free to forward to anyone I've missed. Thanks, Alex ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio-net: Reporting traffic queue distribution statistics through ethtool
This patch allows virtio-net driver to report traffic distribution
to inbound/outbound queues through ethtool -S. The per_cpu
virtnet_stats is split into receive and transmit stats and are
maintained on a per receive_queue and send_queue basis.
virtnet_stats() is modified to aggregate interface level statistics
from per-queue statistics. Sample output below:
NIC statistics:
rxq0: rx_packets: 4357802
rxq0: rx_bytes: 292642052
txq0: tx_packets: 824540
txq0: tx_bytes: 55256404
rxq1: rx_packets: 0
rxq1: rx_bytes: 0
txq1: tx_packets: 1094268
txq1: tx_bytes: 73328316
rxq2: rx_packets: 0
rxq2: rx_bytes: 0
txq2: tx_packets: 1091466
txq2: tx_bytes: 73140566
rxq3: rx_packets: 0
rxq3: rx_bytes: 0
txq3: tx_packets: 1093043
txq3: tx_bytes: 73246142
Signed-off-by: Sriram Narasimhan sriram.narasim...@hp.com
---
drivers/net/virtio_net.c | 157 +-
1 files changed, 128 insertions(+), 29 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3c23fdc..3c58c52 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -41,15 +41,46 @@ module_param(gso, bool, 0444);
#define VIRTNET_DRIVER_VERSION 1.0.0
-struct virtnet_stats {
- struct u64_stats_sync tx_syncp;
+struct virtnet_rx_stats {
struct u64_stats_sync rx_syncp;
- u64 tx_bytes;
+ u64 rx_packets;
+ u64 rx_bytes;
+};
+
+struct virtnet_tx_stats {
+ struct u64_stats_sync tx_syncp;
u64 tx_packets;
+ u64 tx_bytes;
+};
- u64 rx_bytes;
- u64 rx_packets;
+struct virtnet_ethtool_stats {
+ char desc[ETH_GSTRING_LEN];
+ int type;
+ int size;
+ int offset;
+};
+
+enum {VIRTNET_STATS_TX, VIRTNET_STATS_RX};
+
+#define VIRTNET_RX_STATS_INFO(_struct, _field) \
+ {#_field, VIRTNET_STATS_RX, FIELD_SIZEOF(_struct, _field), \
+ offsetof(_struct, _field)}
+
+#define VIRTNET_TX_STATS_INFO(_struct, _field) \
+ {#_field, VIRTNET_STATS_TX, FIELD_SIZEOF(_struct, _field), \
+ offsetof(_struct, _field)}
+
+static const struct virtnet_ethtool_stats virtnet_et_rx_stats[] = {
+ VIRTNET_RX_STATS_INFO(struct virtnet_rx_stats, rx_packets),
+ VIRTNET_RX_STATS_INFO(struct virtnet_rx_stats, rx_bytes)
+};
+#define VIRTNET_RX_STATS_NUM (ARRAY_SIZE(virtnet_et_rx_stats))
+
+static const struct virtnet_ethtool_stats virtnet_et_tx_stats[] = {
+ VIRTNET_TX_STATS_INFO(struct virtnet_tx_stats, tx_packets),
+ VIRTNET_TX_STATS_INFO(struct virtnet_tx_stats, tx_bytes)
};
+#define VIRTNET_TX_STATS_NUM (ARRAY_SIZE(virtnet_et_tx_stats))
/* Internal representation of a send virtqueue */
struct send_queue {
@@ -61,6 +92,9 @@ struct send_queue {
/* Name of the send queue: output.$index */
char name[40];
+
+ /* Active send queue statistics */
+ struct virtnet_tx_stats stats;
};
/* Internal representation of a receive virtqueue */
@@ -81,6 +115,9 @@ struct receive_queue {
/* Name of this receive queue: input.$index */
char name[40];
+
+ /* Active receive queue statistics */
+ struct virtnet_rx_stats stats;
};
struct virtnet_info {
@@ -109,9 +146,6 @@ struct virtnet_info {
/* enable config space updates */
bool config_enable;
- /* Active statistics */
- struct virtnet_stats __percpu *stats;
-
/* Work struct for refilling if we run low on memory. */
struct delayed_work refill;
@@ -330,7 +364,7 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
{
struct virtnet_info *vi = rq-vq-vdev-priv;
struct net_device *dev = vi-dev;
- struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
+ struct virtnet_rx_stats *stats = rq-stats;
struct sk_buff *skb;
struct page *page;
struct skb_vnet_hdr *hdr;
@@ -650,8 +684,7 @@ static void free_old_xmit_skbs(struct send_queue *sq)
{
struct sk_buff *skb;
unsigned int len;
- struct virtnet_info *vi = sq-vq-vdev-priv;
- struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
+ struct virtnet_tx_stats *stats = sq-stats;
while ((skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
pr_debug(Sent skb %p\n, skb);
@@ -841,24 +874,25 @@ static struct rtnl_link_stats64 *virtnet_stats(struct
net_device *dev,
struct rtnl_link_stats64 *tot)
{
struct virtnet_info *vi = netdev_priv(dev);
- int cpu;
+ int i;
unsigned int start;
- for_each_possible_cpu(cpu) {
- struct virtnet_stats *stats = per_cpu_ptr(vi-stats, cpu);
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct virtnet_tx_stats *tstats = vi-sq[i].stats;
+ struct virtnet_rx_stats *rstats = vi-rq[i].stats;
u64 tpackets, tbytes, rpackets, rbytes;
Re: [PATCH] virtio-net: Reporting traffic queue distribution statistics through ethtool
On Thu, 2013-05-16 at 13:24 -0700, Sriram Narasimhan wrote: This patch allows virtio-net driver to report traffic distribution to inbound/outbound queues through ethtool -S. The per_cpu virtnet_stats is split into receive and transmit stats and are maintained on a per receive_queue and send_queue basis. virtnet_stats() is modified to aggregate interface level statistics from per-queue statistics. Sample output below: NIC statistics: rxq0: rx_packets: 4357802 rxq0: rx_bytes: 292642052 txq0: tx_packets: 824540 txq0: tx_bytes: 55256404 rxq1: rx_packets: 0 rxq1: rx_bytes: 0 txq1: tx_packets: 1094268 txq1: tx_bytes: 73328316 rxq2: rx_packets: 0 rxq2: rx_bytes: 0 txq2: tx_packets: 1091466 txq2: tx_bytes: 73140566 rxq3: rx_packets: 0 rxq3: rx_bytes: 0 txq3: tx_packets: 1093043 txq3: tx_bytes: 73246142 Signed-off-by: Sriram Narasimhan sriram.narasim...@hp.com [...] That ordering is totally unreadable. I want to see patches for ethtool to let the user order and aggregate the queue statistics in a sensible way: $ ethtool -S eth0 # default would show aggregated statistics NIC statistics: rx_packets: 4357802 rx_bytes: 292642052 tx_packets: 4103317 tx_bytes: 274971428 $ ethtool -S eth0 full group queue # full statistics, grouped by queue name NIC statistics: rxq0: rx_packets: 4357802 rx_bytes: 292642052 rxq1: rx_packets: 0 rx_bytes: 0 [...] txq0: tx_packets: 824540 tx_bytes: 55256404 txq1: tx_packets: 1094268 tx_bytes: 73328316 [...] $ ethtool -S eth0 full group statistic # full statistics, grouped by statistic name NIC statistics: rx_packets: rxq0: 4357802 rxq1: 0 rxq2: 0 rxq3: 0 rx_bytes: rxq0: 292642052 rxq1: 0 rxq2: 0 rxq3: 0 [...] Maybe even: $ ethtool -S eth0 full table NIC statistics: rx_packets rx_bytes rxq0: 4357802 292642052 rxq1:0 0 rxq2:0 0 rxq3:0 0 tx_packets tx_bytes txq0: 824540 55256404 txq1: 1094268 73328316 txq2: 1091466 73140566 txq3: 1093043 73246142 (Difficult to do, but probably very useful!) The queue names should be rx-index and tx-index like in sysfs. We'll need to reach a consensus on the naming scheme for per-queue and otherwise disaggregated statistics (see previous discussions in http://thread.gmane.org/gmane.linux.kernel.virtualization/15572/focus=15608). I don't much care what they look like as long as there's an implementation for the ethtool side and they don't look too awful in older versions of ethtool. 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_pci: fix macro exported in uapi
Michael S. Tsirkin m...@redhat.com writes: macro VIRTIO_PCI_CONFIG except in the unlikely event userspace actually has a structure with a field named msix_enabled. Get the msix_enabled by value instead, to make it useful for userspace. Signed-off-by: Michael S. Tsirkin m...@redhat.com Macro still isn't usable, because userspace can't know whether it's the new or old. We need to either remove it from UAPI, or rename it to VIRTIO_PCI_CONFIG_OFF. Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
Joe Perches j...@perches.com writes:
On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
Asias He as...@redhat.com writes:
On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
[]
Other users are using memcpy_fromiovec and friends outside net. It seems
a good idea to put it in a util library. e.g. crypto/algif_skcipher.c
which also depends on NET for it.
[]
Subject: Hoist memcpy_fromiovec into lib/
You'll need the friends memcpy_toiovec too.
$ git grep -E \bmemcpy\w+iovec\w*
crypto/algif_hash.c:err = memcpy_toiovec(msg-msg_iov, ctx-result, len);
crypto/algif_skcipher.c:err =
memcpy_fromiovec(page_address(sg_page(sg)) +
crypto/algif_skcipher.c:err =
memcpy_fromiovec(page_address(sg_page(sg + i)),
drivers/dma/iovlock.c:#include net/tcp.h /* for memcpy_toiovec */
drivers/dma/iovlock.c: return memcpy_toiovec(iov, kdata, len);
drivers/dma/iovlock.c: err = memcpy_toiovec(iov, vaddr + offset,
len);
drivers/isdn/mISDN/socket.c:if (memcpy_fromiovec(skb_put(skb, len),
msg-msg_iov, len)) {
drivers/misc/vmw_vmci/vmci_queue_pair.c:err =
memcpy_fromiovec((u8 *)va + page_o
drivers/misc/vmw_vmci/vmci_queue_pair.c:err =
memcpy_toiovec(iov, (u8 *)va + pag
Fascinating. These all indirectly depend on NET, so there's no problem
at the moment. But it is a bit weird...
crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH - NET
crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER - NET
drivers/dma/iovlock.c: depends on NET_DMA - NET
drivers/isdn/mISDN/socket.c: depends on MISDN - ISDN - NET
drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI - NET
Patch welcome.
Meanwhile, to avoid more bikeshedding I've put the patch I posted with
all acks in my fixes branch. One cycle through linux-next, then
straight to Linus.
Subject: Hoist memcpy_fromiovec into lib/
ERROR: memcpy_fromiovec [drivers/vhost/vhost_scsi.ko] undefined!
That function is only present with CONFIG_NET. Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.
socket.h already include uio.h, so no callers need updating.
Reported-by: Randy Dunlap rdun...@infradead.org
Acked-by: David S. Miller da...@davemloft.net
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..7266775 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct
ucred *ucred);
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int offset, int len);
extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..21628d3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov,
unsigned long nr_segs)
}
unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
#endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o dump_stack.o timerqueue.o\
-idr.o int_sqrt.o extable.o \
+idr.o int_sqrt.o extable.o iovec.o \
sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 000..632c5ea
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,29 @@
+#include linux/uaccess.h
+#include linux/export.h
+#include linux/uio.h
+
+/*
+ * Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ * Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+ while (len 0) {
+ if (iov-iov_len) {
+ int copy = min_t(unsigned int, len, iov-iov_len);
+ if (copy_from_user(kdata, iov-iov_base, copy))
+ return -EFAULT;
+ len -= copy;
+ kdata += copy;
+ iov-iov_base += copy;
+ iov-iov_len -= copy;
+ }
+ iov++;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(memcpy_fromiovec);
+
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..d81257f 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@
Re: [PATCH] virtio_console: fix uapi header
Michael S. Tsirkin m...@redhat.com writes: uapi should use __u32 not u32. Fix a macro in virtio_console.h which uses u32. Signed-off-by: Michael S. Tsirkin m...@redhat.com Ouch. Added CC:stable, and put into fixes. Mainly because it's embarrassing :) Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: [PATCH] virtio-net: Reporting traffic queue distribution statistics through ethtool
Thanks for the pointer to the earlier discussion thread. I am open to any consensus format. I would also like to get feedback from Michael on maintaining the stats in the per-queue data structure. Sriram -Original Message- From: Ben Hutchings [mailto:bhutchi...@solarflare.com] Sent: Thursday, May 16, 2013 2:48 PM To: Narasimhan, Sriram Cc: m...@redhat.com; ru...@rustcorp.com.au; virtualization@lists.linux-foundation.org; k...@vger.kernel.org; net...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH] virtio-net: Reporting traffic queue distribution statistics through ethtool On Thu, 2013-05-16 at 13:24 -0700, Sriram Narasimhan wrote: This patch allows virtio-net driver to report traffic distribution to inbound/outbound queues through ethtool -S. The per_cpu virtnet_stats is split into receive and transmit stats and are maintained on a per receive_queue and send_queue basis. virtnet_stats() is modified to aggregate interface level statistics from per-queue statistics. Sample output below: NIC statistics: rxq0: rx_packets: 4357802 rxq0: rx_bytes: 292642052 txq0: tx_packets: 824540 txq0: tx_bytes: 55256404 rxq1: rx_packets: 0 rxq1: rx_bytes: 0 txq1: tx_packets: 1094268 txq1: tx_bytes: 73328316 rxq2: rx_packets: 0 rxq2: rx_bytes: 0 txq2: tx_packets: 1091466 txq2: tx_bytes: 73140566 rxq3: rx_packets: 0 rxq3: rx_bytes: 0 txq3: tx_packets: 1093043 txq3: tx_bytes: 73246142 Signed-off-by: Sriram Narasimhan sriram.narasim...@hp.com [...] That ordering is totally unreadable. I want to see patches for ethtool to let the user order and aggregate the queue statistics in a sensible way: $ ethtool -S eth0 # default would show aggregated statistics NIC statistics: rx_packets: 4357802 rx_bytes: 292642052 tx_packets: 4103317 tx_bytes: 274971428 $ ethtool -S eth0 full group queue # full statistics, grouped by queue name NIC statistics: rxq0: rx_packets: 4357802 rx_bytes: 292642052 rxq1: rx_packets: 0 rx_bytes: 0 [...] txq0: tx_packets: 824540 tx_bytes: 55256404 txq1: tx_packets: 1094268 tx_bytes: 73328316 [...] $ ethtool -S eth0 full group statistic # full statistics, grouped by statistic name NIC statistics: rx_packets: rxq0: 4357802 rxq1: 0 rxq2: 0 rxq3: 0 rx_bytes: rxq0: 292642052 rxq1: 0 rxq2: 0 rxq3: 0 [...] Maybe even: $ ethtool -S eth0 full table NIC statistics: rx_packets rx_bytes rxq0: 4357802 292642052 rxq1:0 0 rxq2:0 0 rxq3:0 0 tx_packets tx_bytes txq0: 824540 55256404 txq1: 1094268 73328316 txq2: 1091466 73140566 txq3: 1093043 73246142 (Difficult to do, but probably very useful!) The queue names should be rx-index and tx-index like in sysfs. We'll need to reach a consensus on the naming scheme for per-queue and otherwise disaggregated statistics (see previous discussions in http://thread.gmane.org/gmane.linux.kernel.virtualization/15572/focus=15608). I don't much care what they look like as long as there's an implementation for the ethtool side and they don't look too awful in older versions of ethtool. 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec
On 05/16/13 16:42, Rusty Russell wrote:
Joe Perches j...@perches.com writes:
On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
Asias He as...@redhat.com writes:
On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
[]
Other users are using memcpy_fromiovec and friends outside net. It seems
a good idea to put it in a util library. e.g. crypto/algif_skcipher.c
which also depends on NET for it.
[]
Subject: Hoist memcpy_fromiovec into lib/
You'll need the friends memcpy_toiovec too.
$ git grep -E \bmemcpy\w+iovec\w*
crypto/algif_hash.c:err = memcpy_toiovec(msg-msg_iov, ctx-result, len);
crypto/algif_skcipher.c:err =
memcpy_fromiovec(page_address(sg_page(sg)) +
crypto/algif_skcipher.c:err =
memcpy_fromiovec(page_address(sg_page(sg + i)),
drivers/dma/iovlock.c:#include net/tcp.h /* for memcpy_toiovec */
drivers/dma/iovlock.c: return memcpy_toiovec(iov, kdata, len);
drivers/dma/iovlock.c: err = memcpy_toiovec(iov, vaddr + offset,
len);
drivers/isdn/mISDN/socket.c:if (memcpy_fromiovec(skb_put(skb, len),
msg-msg_iov, len)) {
drivers/misc/vmw_vmci/vmci_queue_pair.c:err =
memcpy_fromiovec((u8 *)va + page_o
drivers/misc/vmw_vmci/vmci_queue_pair.c:err =
memcpy_toiovec(iov, (u8 *)va + pag
Fascinating. These all indirectly depend on NET, so there's no problem
at the moment. But it is a bit weird...
crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH - NET
crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER - NET
drivers/dma/iovlock.c: depends on NET_DMA - NET
drivers/isdn/mISDN/socket.c: depends on MISDN - ISDN - NET
drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI - NET
Patch welcome.
Meanwhile, to avoid more bikeshedding I've put the patch I posted with
all acks in my fixes branch. One cycle through linux-next, then
straight to Linus.
I agree with whoever suggested that more be moved into /lib.
E.g., drivers/misc/vmw_vmci/Kconfig uses depends on NET because the
code there uses both memcpy_toiovec() and memcpy_fromiovec().
See commit ID 6d4f0139d642c45411a47879325891ce2a7c164a.
Subject: Hoist memcpy_fromiovec into lib/
ERROR: memcpy_fromiovec [drivers/vhost/vhost_scsi.ko] undefined!
That function is only present with CONFIG_NET. Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.
socket.h already include uio.h, so no callers need updating.
Reported-by: Randy Dunlap rdun...@infradead.org
Acked-by: David S. Miller da...@davemloft.net
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..7266775 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct
ucred *ucred);
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int
len);
extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int offset, int len);
extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..21628d3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov,
unsigned long nr_segs)
}
unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t
to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
#endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o dump_stack.o timerqueue.o\
- idr.o int_sqrt.o extable.o \
+ idr.o int_sqrt.o extable.o iovec.o \
sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 000..632c5ea
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,29 @@
+#include linux/uaccess.h
+#include linux/export.h
+#include linux/uio.h
+
+/*
+ * Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ * Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+ while (len 0) {
+ if (iov-iov_len) {
+ int copy = min_t(unsigned int, len, iov-iov_len);
+ if (copy_from_user(kdata, iov-iov_base, copy))
+ return -EFAULT;
+ len -= copy;
+
