diff mbox

[net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()

Message ID 1347866974.26523.53.camel@edumazet-glaptop
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Sept. 17, 2012, 7:29 a.m. UTC 
From: Eric Dumazet <edumazet@google.com>

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.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Cc: Ying Cai <ycai@google.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)



--
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

Comments

Or Gerlitz Sept. 18, 2012, 7:58 p.m. UTC | #1 
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
Yevgeny Petrilin Sept. 19, 2012, 7:58 a.m. UTC | #2 
> 

> 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
Eric Dumazet Sept. 19, 2012, 12:12 p.m. UTC | #3 
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
Ying Cai Sept. 19, 2012, 9:21 p.m. UTC | #4 
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
Yevgeny Petrilin Sept. 20, 2012, 7:03 a.m. UTC | #5 
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 mbox

Patch

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;
 }