From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: RE: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch size configurable Date: Mon, 13 Nov 2017 11:03:44 +0000 Message-ID: <08738940ed9a42ff887220ffb796cb1f@AMSPEX02CL03.citrite.net> References: <20171110193458.14204-1-joao.m.martins@oracle.com> <5A0986F3020000780018E479@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: Wei Liu , "xen-devel@lists.xenproject.org" , 'Joao Martins' , "netdev@vger.kernel.org" To: 'Jan Beulich' Return-path: Received: from smtp.citrix.com.au ([103.14.252.240]:48104 "EHLO SMTP.CITRIX.COM.AU" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751986AbdKMLDs (ORCPT ); Mon, 13 Nov 2017 06:03:48 -0500 In-Reply-To: <5A0986F3020000780018E479@prv-mh.provo.novell.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 13 November 2017 10:50 > To: Paul Durrant > Cc: Wei Liu ; xen-devel@lists.xenproject.org; 'Joao > Martins' ; netdev@vger.kernel.org > Subject: Re: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch > size configurable > > >>> On 13.11.17 at 11:33, 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 ... > > >> --- 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? > > 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