Message ID | 1347866974.26523.53.camel@edumazet-glaptop |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, Sep 17, 2012 at 10:29 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Since commit e22979d96a5 (mlx4_en: Moving to Interrupts for TX > completions), we no longer can free TX skb from hard IRQ, but only from > normal softirq or process context. > > Therefore, we can directly call dev_kfree_skb() from > mlx4_en_free_tx_desc() like other conventional NAPI drivers. Hi Eric, The team is all off till tomorrow, we will look and get back to you Or. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Since commit e22979d96a5 (mlx4_en: Moving to Interrupts for TX > completions), we no longer can free TX skb from hard IRQ, but only from > normal softirq or process context. > > Therefore, we can directly call dev_kfree_skb() from > mlx4_en_free_tx_desc() like other conventional NAPI drivers. > Hi Eric, At the moment the TX completion processing is done from IRQ context. So I think we need to change the driver to work with NAPI for TX completions before making this change. I'll send the patch in a few days. Yevgeny
On Wed, 2012-09-19 at 07:58 +0000, Yevgeny Petrilin wrote: > > > > Since commit e22979d96a5 (mlx4_en: Moving to Interrupts for TX > > completions), we no longer can free TX skb from hard IRQ, but only from > > normal softirq or process context. > > > > Therefore, we can directly call dev_kfree_skb() from > > mlx4_en_free_tx_desc() like other conventional NAPI drivers. > > > > Hi Eric, > At the moment the TX completion processing is done from IRQ context. > So I think we need to change the driver to work with NAPI for TX completions > before making this change. > > I'll send the patch in a few days. Oops you're right, it seems I misread e22979d96 commit. irq term is a bit generic, you might add soft/hard qualifiers to distinguish the variant. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Yevgeny, It seems all the TxQs are sharing the same interrupt for Tx completions. Will it be better to have separate interrupt per num_tx_rings_p_up (8) queues? E.g. for a 16 core system, with 16 * 8 Tx queues, to have 16 interrupts for Tx completions of those 128 Tx queues? Also I'm looking at mlx4_en_select_queue(), it is using __skb_tx_hash(). Use something to achieve XPS may bring better performances. Thanks. On Wed, Sep 19, 2012 at 5:12 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > On Wed, 2012-09-19 at 07:58 +0000, Yevgeny Petrilin wrote: > > > > > > Since commit e22979d96a5 (mlx4_en: Moving to Interrupts for TX > > > completions), we no longer can free TX skb from hard IRQ, but only > > > from > > > normal softirq or process context. > > > > > > Therefore, we can directly call dev_kfree_skb() from > > > mlx4_en_free_tx_desc() like other conventional NAPI drivers. > > > > > > > Hi Eric, > > At the moment the TX completion processing is done from IRQ context. > > So I think we need to change the driver to work with NAPI for TX > > completions > > before making this change. > > > > I'll send the patch in a few days. > > Oops you're right, it seems I misread e22979d96 commit. > > irq term is a bit generic, you might add soft/hard qualifiers to > distinguish the variant. > > Thanks > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ying, > It seems all the TxQs are sharing the same interrupt for Tx > completions. Will it be better to have separate interrupt per > num_tx_rings_p_up (8) queues? E.g. for a 16 core system, with 16 * 8 > Tx queues, to have 16 interrupts for Tx completions of those 128 Tx > queues? Actually not all TxQs share same interrupt vector. In commit 76532d0c we assigned an interrupt vector for each TX ring. When the number of Queues is higher than number of interrupt vectors, there are queues that share interrupts And actually reaching the assignment you specified. > > Also I'm looking at mlx4_en_select_queue(), it is using > __skb_tx_hash(). Use something to achieve XPS may bring better > performances. > We are considering this change. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c index 10bba09..e182762 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c @@ -266,7 +266,7 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv, } } - dev_kfree_skb_any(skb); + dev_kfree_skb(skb); return tx_info->nr_txbb; }