[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



> -----Original Message-----
> From: Joao Martins [mailto:joao.m.martins@xxxxxxxxxx]
> Sent: 13 November 2017 16:34
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Wei Liu <wei.liu2@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> 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

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.