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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-05-14 at 19:59 +0100, Zoltan Kiss wrote:
> 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!

OK.

Perhaps consider adding a stat to count the occurrences of this path?
(not necessarily right now)

Ian.



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