> -----Original Message----- > From: Joao Martins [mailto:joao.m.mart...@oracle.com] > Sent: 13 November 2017 16:34 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: netdev@vger.kernel.org; Wei Liu <wei.l...@citrix.com>; xen- > de...@lists.xenproject.org > Subject: Re: [PATCH net-next v1] xen-netback: make copy batch size > configurable > > On Mon, Nov 13, 2017 at 11:58:03AM +0000, Paul Durrant wrote: > > On Mon, Nov 13, 2017 at 11:54:00AM +0000, Joao Martins wrote: > > > On 11/13/2017 10:33 AM, Paul Durrant wrote: > > > > On 11/10/2017 19:35 PM, Joao Martins wrote: > > [snip] > > > > >> 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); > > > > > > > > Surely queue->rx_copy.size and xenvif_copy_batch_size are always > > > > identical? Why do you need this statement (and hence stack variable)? > > > > > > > This statement was to allow to be changed dynamically and would > > > affect all newly created guests or running guests if value happened > > > to be smaller than initially allocated. But I suppose I should make > > > behaviour more consistent with the other params we have right now > > > and just look at initially allocated one `queue->rx_copy.batch_size` ? > > > > Yes, that would certainly be consistent but I can see value in > > allowing it to be dynamically tuned, so perhaps adding some re-allocation > > code to allow the batch to be grown as well as shrunk might be nice. > > The shrink one we potentially risk losing data, so we need to gate the > reallocation whenever `rx_copy.num` is less than the new requested > batch. Worst case means guestrx_thread simply uses the initial > allocated value.
Can't you just re-alloc immediately after the flush (when num is guaranteed to be zero)? Paul
