[PATCH net-next v1] xen-netback: make copy batch size configurable

* [PATCH net-next v1] xen-netback: make copy batch size configurable
@ 2017-11-10 19:34 Joao Martins
  0 siblings, 0 replies; 15+ messages in thread
From: Joao Martins @ 2017-11-10 19:34 UTC (permalink / raw)
  To: netdev; +Cc: xen-devel, Joao Martins, Paul Durrant, Wei Liu

Commit eb1723a29b9a ("xen-netback: refactor guest rx") refactored Rx
handling and as a result decreased max grant copy ops from 4352 to 64.
Before this commit it would drain the rx_queue (while there are
enough slots in the ring to put packets) then copy to all pages and write
responses on the ring. With the refactor we do almost the same albeit
the last two steps are done every COPY_BATCH_SIZE (64) copies.

For big packets, the value of 64 means copying 3 packets best case scenario
(17 copies) and worst-case only 1 packet (34 copies, i.e. if all frags
plus head cross the 4k grant boundary) which could be the case when
packets go from local backend process.

Instead of making it static to 64 grant copies, lets allow the user to
select its value (while keeping the current as default) by introducing
the `copy_batch_size` module parameter. This allows users to select
the higher batches (i.e. for better throughput with big packets) as it
was prior to the above mentioned commit.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 drivers/net/xen-netback/common.h    |  6 ++++--
 drivers/net/xen-netback/interface.c | 25 ++++++++++++++++++++++++-
 drivers/net/xen-netback/netback.c   |  5 +++++
 drivers/net/xen-netback/rx.c        |  5 ++++-
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index a46a1e94505d..a5fe36e098a7 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -129,8 +129,9 @@ struct xenvif_stats {
 #define COPY_BATCH_SIZE 64
 
 struct xenvif_copy_state {
-	struct gnttab_copy op[COPY_BATCH_SIZE];
-	RING_IDX idx[COPY_BATCH_SIZE];
+	struct gnttab_copy *op;
+	RING_IDX *idx;
+	unsigned int size;
 	unsigned int num;
 	struct sk_buff_head *completed;
 };
@@ -381,6 +382,7 @@ extern unsigned int rx_drain_timeout_msecs;
 extern unsigned int rx_stall_timeout_msecs;
 extern unsigned int xenvif_max_queues;
 extern unsigned int xenvif_hash_cache_size;
+extern unsigned int xenvif_copy_batch_size;
 
 #ifdef CONFIG_DEBUG_FS
 extern struct dentry *xen_netback_dbg_root;
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index d6dff347f896..a558868a883f 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -516,7 +516,20 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 
 int xenvif_init_queue(struct xenvif_queue *queue)
 {
+	int size = xenvif_copy_batch_size;
 	int err, i;
+	void *addr;
+
+	addr = vzalloc(size * sizeof(struct gnttab_copy));
+	if (!addr)
+		goto err;
+	queue->rx_copy.op = addr;
+
+	addr = vzalloc(size * sizeof(RING_IDX));
+	if (!addr)
+		goto err;
+	queue->rx_copy.idx = addr;
+	queue->rx_copy.size = size;
 
 	queue->credit_bytes = queue->remaining_credit = ~0UL;
 	queue->credit_usec  = 0UL;
@@ -544,7 +557,7 @@ int xenvif_init_queue(struct xenvif_queue *queue)
 				 queue->mmap_pages);
 	if (err) {
 		netdev_err(queue->vif->dev, "Could not reserve mmap_pages\n");
-		return -ENOMEM;
+		goto err;
 	}
 
 	for (i = 0; i < MAX_PENDING_REQS; i++) {
@@ -556,6 +569,13 @@ int xenvif_init_queue(struct xenvif_queue *queue)
 	}
 
 	return 0;
+
+err:
+	if (queue->rx_copy.op)
+		vfree(queue->rx_copy.op);
+	if (queue->rx_copy.idx)
+		vfree(queue->rx_copy.idx);
+	return -ENOMEM;
 }
 
 void xenvif_carrier_on(struct xenvif *vif)
@@ -788,6 +808,9 @@ void xenvif_disconnect_ctrl(struct xenvif *vif)
  */
 void xenvif_deinit_queue(struct xenvif_queue *queue)
 {
+	vfree(queue->rx_copy.op);
+	vfree(queue->rx_copy.idx);
+	queue->rx_copy.size = 0;
 	gnttab_free_pages(MAX_PENDING_REQS, queue->mmap_pages);
 }
 
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index a27daa23c9dc..3a5e1d7ac2f4 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -96,6 +96,11 @@ unsigned int xenvif_hash_cache_size = XENVIF_HASH_CACHE_SIZE_DEFAULT;
 module_param_named(hash_cache_size, xenvif_hash_cache_size, uint, 0644);
 MODULE_PARM_DESC(hash_cache_size, "Number of flows in the hash cache");
 
+/* This is the maximum batch of grant copies on Rx */
+unsigned int xenvif_copy_batch_size = COPY_BATCH_SIZE;
+module_param_named(copy_batch_size, xenvif_copy_batch_size, uint, 0644);
+MODULE_PARM_DESC(copy_batch_size, "Maximum batch of grant copies on Rx");
+
 static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
 			       u8 status);
 
diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
index b1cf7c6f407a..793a85f61f9d 100644
--- a/drivers/net/xen-netback/rx.c
+++ b/drivers/net/xen-netback/rx.c
@@ -168,11 +168,14 @@ static void xenvif_rx_copy_add(struct xenvif_queue *queue,
 			       struct xen_netif_rx_request *req,
 			       unsigned int offset, void *data, size_t len)
 {
+	unsigned int batch_size;
 	struct gnttab_copy *op;
 	struct page *page;
 	struct xen_page_foreign *foreign;
 
-	if (queue->rx_copy.num == COPY_BATCH_SIZE)
+	batch_size = min(xenvif_copy_batch_size, queue->rx_copy.size);
+
+	if (queue->rx_copy.num == batch_size)
 		xenvif_rx_copy_flush(queue);
 
 	op = &queue->rx_copy.op[queue->rx_copy.num];
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread