netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zoltan Kiss <zoltan.kiss@citrix.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: <xen-devel@lists.xenproject.org>, <wei.liu2@citrix.com>,
	<linux@eikelenboom.it>, <paul.durrant@citrix.com>,
	<netdev@vger.kernel.org>, <david.vrabel@citrix.com>,
	<davem@davemloft.net>
Subject: Re: [PATCH net v3] xen-netback: Fix grant ref resolution in RX path
Date: Wed, 14 May 2014 19:59:44 +0100	[thread overview]
Message-ID: <5373BD20.8030800@citrix.com> (raw)
In-Reply-To: <1400085628.26934.16.camel@kazak.uk.xensource.com>

All the comments are acked and applied, expect this:

On 14/05/14 17:40, Ian Campbell wrote:
> On Wed, 2014-05-14 at 13:08 +0100, Zoltan Kiss wrote:

>
>> +		/* This variable also signals whether foreign_gref has a real
>> +		 * value or not.
>> +		 */
>> +		struct xenvif *foreign_vif = NULL;
>> +
>> +		if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) &&
>> +			(ubuf->callback == &xenvif_zerocopy_callback)) {
>> +			const struct ubuf_info *const startpoint = ubuf;
>> +
>> +			/* Ideally ubuf points to the chain element which
>> +			 * belongs to this frag. Or if frags were removed from
>> +			 * the beginning, then shortly before it.
>> +			 */
>> +			ubuf = xenvif_find_gref(skb, i, ubuf);
>> +
>> +			/* Try again from the beginning of the list, if we
>> +			 * haven't tried from there. This only makes sense in
>> +			 * the unlikely event of reordering the original frags.
>> +			 * For injected local pages it's an unnecessary second
>> +			 * run.
>
> A second run is unfortunate. Can we also pass startpoint to
> xenvif_find_gref as an end value and have it only search that far?
That would help, but it would just increase the complexity here, and I 
think this is absolutely a non fast-path case. But the first search is, 
passing a new parameter and doing another comparison can consume a few 
cycles.
We don't know about any scenarios where frag reordering or injection of 
local frags can actually happen, this is just about closing out 
theoretical possibilities. Let's optimize it when it actually needed!

  reply	other threads:[~2014-05-14 18:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-14 12:08 [PATCH net v3] xen-netback: Fix grant ref resolution in RX path Zoltan Kiss
2014-05-14 16:40 ` Ian Campbell
2014-05-14 18:59   ` Zoltan Kiss [this message]
2014-05-15  8:29     ` Ian Campbell

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=5373BD20.8030800@citrix.com \
    --to=zoltan.kiss@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=davem@davemloft.net \
    --cc=david.vrabel@citrix.com \
    --cc=linux@eikelenboom.it \
    --cc=netdev@vger.kernel.org \
    --cc=paul.durrant@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).