Re: [RFC PATCH 03/13] xen-netback: implement TX persistent grants

[Wei Liu <wei.liu2@citrix.com>]

On Fri, May 22, 2015 at 10:24:39AM +0000, Joao Martins wrote:
> 
> On 19 May 2015, at 17:23, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, May 12, 2015 at 07:18:27PM +0200, Joao Martins wrote:
> >> Introduces persistent grants for TX path which follows similar code path
> >> as the grant mapping.
> >> 
> >> It starts by checking if there's a persistent grant available for header
> >> and frags grefs and if so setting it in tx_pgrants. If no persistent grant
> >> is found in the tree for the header it will resort to grant copy (but
> >> preparing the map ops and add them laster). For the frags it will use the
> >                                     ^
> >                                     later
> > 
> >> tree page pool, and in case of no pages it fallbacks to grant map/unmap
> >> using mmap_pages. When skb destructor callback gets called we release the
> >> slot and persistent grant within the callback to avoid waking up the
> >> dealloc thread. As long as there are no unmaps to done the dealloc thread
> >> will remain inactive.
> >> 
> > 
> > This scheme looks complicated. Can we just only use one
> > scheme at a time? What's the rationale for using this combined scheme?
> > Maybe you're thinking about using a max_grants < ring_size to save
> > memory?
> 
> Yes, my purpose was to allow a max_grants < ring_size to save amount of
> memory mapped. I did a bulk transfer test with iperf and the max amount of
> grants in tree was <160 TX gnts, without affecting the max performance;
> tough using pktgen fills the tree completely.
> The second reason is to handle the case for a (malicious?) frontend providing
> more grefs than the max allowed in which I would fallback to grant map/unmap.
> 

This is indeed a valid concern. The only method is to expires oldest
grant when that happens -- but this is just complexity in another place,
not really simplifying anything.

> > 
> > Only skim the patch. I will do detailed reviews after we're sure this is
> > the right way to go.
> > 
[...]
> > 
> > Under what circumstance can we retrieve a already in use persistent
> > grant? You seem to suggest this is a bug in RX case.
> 
> A guest could share try to share the same mapped page in multiple frags,
> in which case I fallback to map/unmap. I think this is a limitation in
> the way we manage the persistent gnts where we can only have a single
> reference of a persistent grant inflight.
> 

How much harder would it be to ref-count inflight grants? Would that
simplify or perplex things? I'm just asking, not suggesting you should
choose ref-counting over current scheme.

In principle I favour simple code path over optimisation for every
possible corner case.

Wei.