public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Wei Liu <wei.liu2@citrix•com>
To: Joao Martins <Joao.Martins@neclab•eu>
Cc: Wei Liu <wei.liu2@citrix•com>,
	"xen-devel@lists•xenproject.org" <xen-devel@lists•xenproject.org>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
	"ian.campbell@citrix•com" <ian.campbell@citrix•com>,
	"david.vrabel@citrix•com" <david.vrabel@citrix•com>,
	"boris.ostrovsky@oracle•com" <boris.ostrovsky@oracle•com>,
	"konrad.wilk@oracle•com" <konrad.wilk@oracle•com>
Subject: Re: [RFC PATCH 03/13] xen-netback: implement TX persistent grants
Date: Tue, 2 Jun 2015 15:53:59 +0100	[thread overview]
Message-ID: <20150602145359.GP19403@zion.uk.xensource.com> (raw)
In-Reply-To: <30CF0FF9-3B1D-48AF-AFB4-73E20C404357@neclab.eu>

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.

  reply	other threads:[~2015-06-02 14:54 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-12 17:18 [RFC PATCH 00/13] Persistent grant maps for xen net drivers Joao Martins
2015-05-12 17:18 ` [RFC PATCH 01/13] xen-netback: add persistent grant tree ops Joao Martins
2015-05-12 17:18 ` [RFC PATCH 02/13] xen-netback: xenbus feature persistent support Joao Martins
2015-05-19 15:19   ` Wei Liu
2015-05-22 10:24     ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 03/13] xen-netback: implement TX persistent grants Joao Martins
2015-05-19 15:23   ` Wei Liu
2015-05-22 10:24     ` Joao Martins
2015-06-02 14:53       ` Wei Liu [this message]
2015-06-03 17:07         ` Joao Martins
2015-06-07 12:04           ` Wei Liu
2015-05-12 17:18 ` [RFC PATCH 04/13] xen-netback: implement RX " Joao Martins
2015-05-19 15:32   ` Wei Liu
2015-05-22 10:25     ` Joao Martins
2015-06-02 15:07       ` Wei Liu
2015-06-03 17:08         ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 05/13] xen-netback: refactor xenvif_rx_action Joao Martins
2015-05-19 15:32   ` Wei Liu
2015-05-12 17:18 ` [RFC PATCH 06/13] xen-netback: copy buffer on xenvif_start_xmit() Joao Martins
2015-05-19 15:35   ` Wei Liu
2015-05-22 10:26     ` Joao Martins
2015-06-02 15:10       ` Wei Liu
2015-05-12 17:18 ` [RFC PATCH 07/13] xen-netback: add persistent tree counters to debugfs Joao Martins
2015-05-19 15:36   ` Wei Liu
2015-05-12 17:18 ` [RFC PATCH 08/13] xen-netback: clone skb if skb->xmit_more is set Joao Martins
2015-05-19 15:36   ` Wei Liu
2015-05-22 17:14     ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 09/13] xen-netfront: move grant_{ref, page} to struct grant Joao Martins
2015-05-18 15:44   ` [Xen-devel] " David Vrabel
2015-05-19 10:19     ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 10/13] xen-netfront: refactor claim/release grant Joao Martins
2015-05-18 15:48   ` [Xen-devel] " David Vrabel
2015-05-19 10:19     ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 11/13] xen-netfront: feature-persistent xenbus support Joao Martins
2015-05-18 15:51   ` [Xen-devel] " David Vrabel
2015-05-19 10:19     ` Joao Martins
2015-05-12 17:18 ` [RFC PATCH 12/13] xen-netfront: implement TX persistent grants Joao Martins
2015-05-18 15:55   ` [Xen-devel] " David Vrabel
2015-05-19 10:20     ` Joao Martins
2015-05-19 10:23       ` David Vrabel
2015-05-12 17:18 ` [RFC PATCH 13/13] xen-netfront: implement RX " Joao Martins
2015-05-18 16:04   ` [Xen-devel] " David Vrabel
2015-05-19 10:22     ` Joao Martins
2015-05-13 10:50 ` [Xen-devel] [RFC PATCH 00/13] Persistent grant maps for xen net drivers David Vrabel
2015-05-13 13:01   ` Joao Martins
2015-05-19 15:39 ` Wei Liu
2015-05-22 10:27   ` Joao Martins
2015-05-29  6:53     ` [Xen-devel] " Yuzhou (C)
2015-05-29 14:51       ` Joao Martins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150602145359.GP19403@zion.uk.xensource.com \
    --to=wei.liu2@citrix$(echo .)com \
    --cc=Joao.Martins@neclab$(echo .)eu \
    --cc=boris.ostrovsky@oracle$(echo .)com \
    --cc=david.vrabel@citrix$(echo .)com \
    --cc=ian.campbell@citrix$(echo .)com \
    --cc=konrad.wilk@oracle$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=xen-devel@lists$(echo .)xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox