Subject: Re: [PATCH net v3] xen-netback: Fix grant ref resolution in RX path

From: Zoltan Kiss <[email-masked]>
Date: Wed, 14 May 2014 19:59:44 +0100

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!