* [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
@ 2012-09-17 7:29 Eric Dumazet
2012-09-18 19:58 ` Or Gerlitz
2012-09-19 7:58 ` Yevgeny Petrilin
0 siblings, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2012-09-17 7:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Ying Cai
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(-)
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;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
2012-09-17 7:29 [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any() Eric Dumazet
@ 2012-09-18 19:58 ` Or Gerlitz
2012-09-19 7:58 ` Yevgeny Petrilin
1 sibling, 0 replies; 10+ messages in thread
From: Or Gerlitz @ 2012-09-18 19:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Yevgeny Petrilin, Or Gerlitz, Ying Cai
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
2012-09-17 7:29 [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any() Eric Dumazet
2012-09-18 19:58 ` Or Gerlitz
@ 2012-09-19 7:58 ` Yevgeny Petrilin
2012-09-19 12:12 ` Eric Dumazet
1 sibling, 1 reply; 10+ messages in thread
From: Yevgeny Petrilin @ 2012-09-19 7:58 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Or Gerlitz, Ying Cai
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
2012-09-19 7:58 ` Yevgeny Petrilin
@ 2012-09-19 12:12 ` Eric Dumazet
2012-09-19 21:21 ` Ying Cai
2012-09-28 17:53 ` [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit() Eric Dumazet
0 siblings, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2012-09-19 12:12 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: David Miller, netdev, Or Gerlitz, Ying Cai
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
2012-09-19 12:12 ` Eric Dumazet
@ 2012-09-19 21:21 ` Ying Cai
2012-09-20 7:03 ` Yevgeny Petrilin
2012-09-28 17:53 ` [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit() Eric Dumazet
1 sibling, 1 reply; 10+ messages in thread
From: Ying Cai @ 2012-09-19 21:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Yevgeny Petrilin, David Miller, netdev, Or Gerlitz
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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
2012-09-19 21:21 ` Ying Cai
@ 2012-09-20 7:03 ` Yevgeny Petrilin
0 siblings, 0 replies; 10+ messages in thread
From: Yevgeny Petrilin @ 2012-09-20 7:03 UTC (permalink / raw)
To: Ying Cai, Eric Dumazet; +Cc: David Miller, netdev, Or Gerlitz
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit()
2012-09-19 12:12 ` Eric Dumazet
2012-09-19 21:21 ` Ying Cai
@ 2012-09-28 17:53 ` Eric Dumazet
2012-09-30 6:49 ` Yevgeny Petrilin
2012-10-01 21:03 ` David Miller
1 sibling, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2012-09-28 17:53 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: David Miller, netdev, Or Gerlitz
From: Eric Dumazet <edumazet@google.com>
After commit e22979d96a55d (mlx4_en: Moving to Interrupts for TX
completions) we no longer need to orphan skbs in mlx4_en_xmit()
since skb wont stay a long time in TX ring before their release.
Orphaning skbs in ndo_start_xmit() should be avoided as much as
possible, since it breaks TCP Small Queue or other flow control
mechanisms (per socket limits)
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yevgeny Petrilin <yevgenyp@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 10bba09..c10e3a6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -712,10 +712,6 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
if (bounce)
tx_desc = mlx4_en_bounce_to_desc(priv, ring, index, desc_size);
- /* Run destructor before passing skb to HW */
- if (likely(!skb_shared(skb)))
- skb_orphan(skb);
-
if (ring->bf_enabled && desc_size <= MAX_BF && !bounce && !vlan_tag) {
*(__be32 *) (&tx_desc->ctrl.vlan_tag) |= cpu_to_be32(ring->doorbell_qpn);
op_own |= htonl((bf_index & 0xffff) << 8);
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit()
2012-09-28 17:53 ` [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit() Eric Dumazet
@ 2012-09-30 6:49 ` Yevgeny Petrilin
2012-10-01 21:02 ` David Miller
2012-10-01 21:03 ` David Miller
1 sibling, 1 reply; 10+ messages in thread
From: Yevgeny Petrilin @ 2012-09-30 6:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Or Gerlitz
Acked-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, September 28, 2012 7:54 PM
> To: Yevgeny Petrilin
> Cc: David Miller; netdev; Or Gerlitz
> Subject: [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit()
>
> From: Eric Dumazet <edumazet@google.com>
>
> After commit e22979d96a55d (mlx4_en: Moving to Interrupts for TX
> completions) we no longer need to orphan skbs in mlx4_en_xmit()
> since skb wont stay a long time in TX ring before their release.
>
> Orphaning skbs in ndo_start_xmit() should be avoided as much as
> possible, since it breaks TCP Small Queue or other flow control
> mechanisms (per socket limits)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Yevgeny Petrilin <yevgenyp@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index 10bba09..c10e3a6 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> @@ -712,10 +712,6 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
> if (bounce)
> tx_desc = mlx4_en_bounce_to_desc(priv, ring, index, desc_size);
>
> - /* Run destructor before passing skb to HW */
> - if (likely(!skb_shared(skb)))
> - skb_orphan(skb);
> -
> if (ring->bf_enabled && desc_size <= MAX_BF && !bounce && !vlan_tag) {
> *(__be32 *) (&tx_desc->ctrl.vlan_tag) |= cpu_to_be32(ring->doorbell_qpn);
> op_own |= htonl((bf_index & 0xffff) << 8);
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit()
2012-09-28 17:53 ` [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit() Eric Dumazet
2012-09-30 6:49 ` Yevgeny Petrilin
@ 2012-10-01 21:03 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2012-10-01 21:03 UTC (permalink / raw)
To: eric.dumazet; +Cc: yevgenyp, netdev, ogerlitz
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 28 Sep 2012 19:53:26 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> After commit e22979d96a55d (mlx4_en: Moving to Interrupts for TX
> completions) we no longer need to orphan skbs in mlx4_en_xmit()
> since skb wont stay a long time in TX ring before their release.
>
> Orphaning skbs in ndo_start_xmit() should be avoided as much as
> possible, since it breaks TCP Small Queue or other flow control
> mechanisms (per socket limits)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-10-01 21:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-17 7:29 [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any() Eric Dumazet
2012-09-18 19:58 ` Or Gerlitz
2012-09-19 7:58 ` Yevgeny Petrilin
2012-09-19 12:12 ` Eric Dumazet
2012-09-19 21:21 ` Ying Cai
2012-09-20 7:03 ` Yevgeny Petrilin
2012-09-28 17:53 ` [PATCH net-next] mlx4: dont orphan skbs in mlx4_en_xmit() Eric Dumazet
2012-09-30 6:49 ` Yevgeny Petrilin
2012-10-01 21:02 ` David Miller
2012-10-01 21:03 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).