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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-08-06 at 16:54 +0100, Wei Liu wrote:
> On Tue, Aug 06, 2013 at 04:50:02PM +0100, Ian Campbell wrote:
> > On Tue, 2013-08-06 at 16:47 +0100, Wei Liu wrote:
> > > On Tue, Aug 06, 2013 at 04:38:56PM +0100, Ian Campbell wrote:
> > > [...]
> > > > >  * An example sequence of head fields (I = INVALID_PENDING_RING_IDX):
> > > > >  *
> > > > >  * ...|0 I I I|5 I|9 I I I|...
> > > > >  * -->|<-INUSE----------------
> > > > >  *
> > > > >  * After consuming the first slot(s) we have:
> > > > >  *
> > > > >  * ...|V V V V|5 I|9 I I I|...
> > > > >  * -----FREE->|<-INUSE--------
> > > > >  *
> > > > >  * where V stands for "valid pending ring index". Any number other
> > > > >  * than INVALID_PENDING_RING_IDX is OK. In practice we use 0.
> > > > 
> > > > Perhaps add "These entries are considered free and can contain any
> > > > number other than INVALID_PENDING_RING_IDX" ?
> > > > 
> > > > Do you have a #define, like FREE_PENDING_RING_IDX for this value? Should
> > > > mention it if so I think.
> > > > 
> > > 
> > > Unfortunately no. This can be fixed later with separate patch? Or just
> > > have that change in this patch?
> > 
> > Separate I think, this patch is already pretty unwieldy.
> > 
> 
> So the comment for now:
> 
> /* 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|...
>  * -->|<-INUSE----------------
>  *
>  * After consuming the first slot(s) we have:
>  *
>  * ...|V V V V|5 I|9 I I I|...
>  * -----FREE->|<-INUSE--------
>  *
>  * where V stands for "valid pending ring index". Any number other
>  * than INVALID_PENDING_RING_IDX is OK. These entries are considered
>  * free and can contain any number other than
>  * INVALID_PENDING_RING_IDX. In practice we use 0.
>  *
>  * The in use 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.
>  */

Looks good.

> The FREE_PENDING_RING_IDX will be added with later patch.

thanks

Ian.


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