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

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: 13 November 2017 10:50
To: Paul Durrant <redacted>
Cc: Wei Liu <redacted>; xen-devel@lists.xenproject.org; 'Joao
Martins' [off-list ref]; netdev@vger.kernel.org
Subject: Re: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch
size configurable

quoted

quoted

quoted

On 13.11.17 at 11:33, [off-list ref] wrote:

From: Joao Martins [mailto:joao.m.martins@oracle.com]
Sent: 10 November 2017 19:35

--- 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);

Isn't the "owner-write" permission here ...

quoted

quoted

--- 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);

Surely queue->rx_copy.size and xenvif_copy_batch_size are always

identical?

quoted

Why do you need this statement (and hence stack variable)?

... the answer to your question?

Yes, I guess it could be... but since there's no re-alloc code for the arrays I wonder whether the intention was to make this dynamic or not.

  Paul

Jan