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

[Zoltan Kiss <zoltan.kiss@citrix.com>]

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