[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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!

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.