Re: [PATCH net-next v4 8/9] xen-netback: Timeout packets in RX path

From: Zoltan Kiss
Date: Fri Jan 17 2014 - 14:27:46 EST


On 16/01/14 00:03, Wei Liu wrote:
On Tue, Jan 14, 2014 at 08:39:54PM +0000, Zoltan Kiss wrote:
[...]
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 109c29f..d1cd8ce 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -129,6 +129,9 @@ struct xenvif {
struct xen_netif_rx_back_ring rx;
struct sk_buff_head rx_queue;
RING_IDX rx_last_skb_slots;

Hmm... You seemed to mix your other patch with this series. :-)
Yep, this series doesn't work without that patch (actually that is a bug in netback even without my series), so at the moment it is based on it.


+ bool rx_queue_purge;
+
+ struct timer_list wake_queue;

/* This array is allocated seperately as it is large */
struct gnttab_copy *grant_copy_op;
@@ -225,4 +228,7 @@ void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);

extern bool separate_tx_rx_irq;

[...]
@@ -559,7 +579,7 @@ void xenvif_free(struct xenvif *vif)
if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
unmap_timeout++;
schedule_timeout(msecs_to_jiffies(1000));
- if (unmap_timeout > 9 &&
+ if (unmap_timeout > ((rx_drain_timeout_msecs/1000) * DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS))) &&

This line is really too long. And what's the rationale behind this long
expression?
It calculates how many times you should ditch the internal queue of an another (maybe stucked) vif before Qdisc empties it's actual content. After that there shouldn't be any mapped handle left, so we should start printing these messages. Actually it should use vif->dev->tx_queue_len, and yes, it is probably better to move it to the beginning of the function into a new variable, and use that here.

Zoli
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/