[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08738940ed9a42ff887220ffb796cb1f@AMSPEX02CL03.citrite.net>
Date: Mon, 13 Nov 2017 11:03:44 +0000
From: Paul Durrant <Paul.Durrant@...rix.com>
To: 'Jan Beulich' <JBeulich@...e.com>
CC: Wei Liu <wei.liu2@...rix.com>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
'Joao Martins' <joao.m.martins@...cle.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch size
configurable
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@...e.com]
> Sent: 13 November 2017 10:50
> To: Paul Durrant <Paul.Durrant@...rix.com>
> Cc: Wei Liu <wei.liu2@...rix.com>; xen-devel@...ts.xenproject.org; 'Joao
> Martins' <joao.m.martins@...cle.com>; netdev@...r.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch
> size configurable
>
> >>> On 13.11.17 at 11:33, <Paul.Durrant@...rix.com> wrote:
> >> From: Joao Martins [mailto:joao.m.martins@...cle.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
Powered by blists - more mailing lists