Re: [Xen-devel] [PATCH V4 2/3] xen-netback: switch to NAPI + kthread 1:1 model

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Aug 06, 2013 at 02:50:04PM +0100, Ian Campbell wrote:
> On Tue, 2013-08-06 at 14:33 +0100, Wei Liu wrote:
> > I sent this patch right before Ian requested more comments in code. Now
> > I've updated this one. Not going to resend the whole series as this is
> > only changes in comments.
> > 
> > Sorry for my bad English, I don't know whether I've made them clear
> > enough. Suggestions are welcomed.
> > 
> > 
> > Wei.
> > 
> > ---8<---
> > From bb33027fe4bafeea546352cd3e409466f8bd7aa4 Mon Sep 17 00:00:00 2001
> > From: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Date: Tue, 6 Aug 2013 06:59:15 +0100
> > Subject: [PATCH] xen-netback: switch to NAPI + kthread 1:1 model
> > 
> > This patch implements 1:1 model netback. NAPI and kthread are utilized
> > to do the weight-lifting job:
> > 
> > - NAPI is used for guest side TX (host side RX)
> > - kthread is used for guest side RX (host side TX)
> > 
> > Xenvif and xen_netbk are made into one structure to reduce code size.
> > 
> > This model provides better scheduling fairness among vifs. It is also
> > prerequisite for implementing multiqueue for Xen netback.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  drivers/net/xen-netback/common.h    |  128 +++++---
> >  drivers/net/xen-netback/interface.c |  119 ++++---
> >  drivers/net/xen-netback/netback.c   |  607 
> > ++++++++++-------------------------
> >  3 files changed, 347 insertions(+), 507 deletions(-)
> > 
> > diff --git a/drivers/net/xen-netback/common.h 
> > b/drivers/net/xen-netback/common.h
> > index 8a4d77e..184ae0a 100644
> > --- a/drivers/net/xen-netback/common.h
> > +++ b/drivers/net/xen-netback/common.h
> > @@ -45,31 +45,105 @@
> >  #include <xen/grant_table.h>
> >  #include <xen/xenbus.h>
> >  
> > -struct xen_netbk;
> > +typedef unsigned int pending_ring_idx_t;
> > +#define INVALID_PENDING_RING_IDX (~0U)
> > +
> > +/* For the head field in pending_tx_info: It is used to indicate
> > + * whether this tx info is the head of one or more coalesced requests.
> > + *
> > + * When head != INVALID_PENDING_RING_IDX, it means the start of a new
> > + * tx requests queue and the end of previous queue.
> > + *
> > + * An example sequence of head fields (I = INVALID_PENDING_RING_IDX):
> > + *
> > + * ... 0 I I I 5 I 9 I I I ...
> > + *
> > + * After consming the first packet we have:
> 
> "consuming"
> 

Oops...

> The first packet here is "0 I I I"? Consisting of the head (0) and 3
> extra data slots (the 3xI)?
> 

Yes. But the term "extra data slot" is not acurate. "0 I I I" means
we've coalesced 3 more slots into the first slot. And the index of first
slot into various arrays is 0.

> > + *
> > + * ... V V V V 5 I 9 I I I ...
> > + *
> > + * where V stands for "valid pending ring index", any number other
> > + * than INVALID_PENDING_RING_IDX is OK. In practice we use 0.
> 
> OK, this is where I get confused, because 0 is also valid in a different
> state, this one:
> > + * The non-INVALID_PENDING_RING_IDX (say 0, 5 and 9 in the above
> > + * example) number is the index into pending_tx_info and mmap_pages
> > + * arrays.
> 
> So what does V mean and how to you distinguish this from a
> non-INVALID_PENDING_RING_IDX which happens to correspond to the 0 you
> use in practice for V?
> 

Oh, V is just another way of saying non-INVALID_PENDING_RING_IDX. :-/

The head field is always manipulated when coalescing slots and releasing
slots. So if the routine sees 0 when processing a slot is actually means
index 0.

> I'm also not sure how 0 is considered a "valid pending ring index". I
> suppose it is not INVALID_PENDING_RING_IDX but it appears that it
> doesn't have any actual meaning? So it's just an arbitrary valid but
> meaningless number perhaps?
> 

Yes. 0 is as good as any number other than INVALID_PENDING_RING_IDX.


Wei.

> Ian.

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