* [PATCH net] mlx4: do not call napi_schedule() without care
@ 2017-01-13 16:39 Eric Dumazet
2017-01-13 23:07 ` Alexander Duyck
2017-01-16 9:02 ` Tariq Toukan
0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-01-13 16:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Erez Shitrit, Eugenia Emantayev, Tariq Toukan
From: Eric Dumazet <edumazet@google.com>
Disable BH around the call to napi_schedule() to avoid following warning
[ 52.095499] NOHZ: local_softirq_pending 08
[ 52.421291] NOHZ: local_softirq_pending 08
[ 52.608313] NOHZ: local_softirq_pending 08
Fixes: 8d59de8f7bb3 ("net/mlx4_en: Process all completions in RX rings after port goes up")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Erez Shitrit <erezsh@mellanox.com>
Cc: Eugenia Emantayev <eugenia@mellanox.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 4910d9af19335d4b97d39760c163b41eecc26242..761f8b12399cab245abccc0f7d7f84fde742c14d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1748,8 +1748,11 @@ int mlx4_en_start_port(struct net_device *dev)
/* Process all completions if exist to prevent
* the queues freezing if they are full
*/
- for (i = 0; i < priv->rx_ring_num; i++)
+ for (i = 0; i < priv->rx_ring_num; i++) {
+ local_bh_disable();
napi_schedule(&priv->rx_cq[i]->napi);
+ local_bh_enable();
+ }
netif_tx_start_all_queues(dev);
netif_device_attach(dev);
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] mlx4: do not call napi_schedule() without care
2017-01-13 16:39 [PATCH net] mlx4: do not call napi_schedule() without care Eric Dumazet
@ 2017-01-13 23:07 ` Alexander Duyck
2017-01-13 23:40 ` Eric Dumazet
2017-01-16 9:02 ` Tariq Toukan
1 sibling, 1 reply; 5+ messages in thread
From: Alexander Duyck @ 2017-01-13 23:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Erez Shitrit, Eugenia Emantayev,
Tariq Toukan
On Fri, Jan 13, 2017 at 8:39 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Disable BH around the call to napi_schedule() to avoid following warning
>
> [ 52.095499] NOHZ: local_softirq_pending 08
> [ 52.421291] NOHZ: local_softirq_pending 08
> [ 52.608313] NOHZ: local_softirq_pending 08
>
> Fixes: 8d59de8f7bb3 ("net/mlx4_en: Process all completions in RX rings after port goes up")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Erez Shitrit <erezsh@mellanox.com>
> Cc: Eugenia Emantayev <eugenia@mellanox.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 4910d9af19335d4b97d39760c163b41eecc26242..761f8b12399cab245abccc0f7d7f84fde742c14d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1748,8 +1748,11 @@ int mlx4_en_start_port(struct net_device *dev)
> /* Process all completions if exist to prevent
> * the queues freezing if they are full
> */
> - for (i = 0; i < priv->rx_ring_num; i++)
> + for (i = 0; i < priv->rx_ring_num; i++) {
> + local_bh_disable();
> napi_schedule(&priv->rx_cq[i]->napi);
> + local_bh_enable();
> + }
Couldn't you save yourself a ton of trouble by wrapping the loop
inside of the local_bh_disable/enable instead of wrapping them up
inside the loop? It just seems like it might be more efficient to
schedule them and then process them as a block instead of doing it one
at a time.
- Alex
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] mlx4: do not call napi_schedule() without care
2017-01-13 23:07 ` Alexander Duyck
@ 2017-01-13 23:40 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-01-13 23:40 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, netdev, Erez Shitrit, Eugenia Emantayev,
Tariq Toukan
On Fri, 2017-01-13 at 15:07 -0800, Alexander Duyck wrote:
> On Fri, Jan 13, 2017 at 8:39 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Disable BH around the call to napi_schedule() to avoid following warning
> >
> > [ 52.095499] NOHZ: local_softirq_pending 08
> > [ 52.421291] NOHZ: local_softirq_pending 08
> > [ 52.608313] NOHZ: local_softirq_pending 08
> >
> > Fixes: 8d59de8f7bb3 ("net/mlx4_en: Process all completions in RX rings after port goes up")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Erez Shitrit <erezsh@mellanox.com>
> > Cc: Eugenia Emantayev <eugenia@mellanox.com>
> > Cc: Tariq Toukan <tariqt@mellanox.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > index 4910d9af19335d4b97d39760c163b41eecc26242..761f8b12399cab245abccc0f7d7f84fde742c14d 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > @@ -1748,8 +1748,11 @@ int mlx4_en_start_port(struct net_device *dev)
> > /* Process all completions if exist to prevent
> > * the queues freezing if they are full
> > */
> > - for (i = 0; i < priv->rx_ring_num; i++)
> > + for (i = 0; i < priv->rx_ring_num; i++) {
> > + local_bh_disable();
> > napi_schedule(&priv->rx_cq[i]->napi);
> > + local_bh_enable();
> > + }
>
> Couldn't you save yourself a ton of trouble by wrapping the loop
> inside of the local_bh_disable/enable instead of wrapping them up
> inside the loop? It just seems like it might be more efficient to
> schedule them and then process them as a block instead of doing it one
> at a time.
What kind of troubles ?
Given the problem might be happening under flood, I believe it is much
safer to do as I did.
Otherwise, we will have to process a ton of messages at the
local_bh_enable() time and lock the {softirq}IRQ on one cpu.
I chose to do this on purpose.
Batching can be dangerous, and this is exactly the point we do not want
batching, with say 64 queues.
This code is driver starts, hardly fast path.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] mlx4: do not call napi_schedule() without care
2017-01-13 16:39 [PATCH net] mlx4: do not call napi_schedule() without care Eric Dumazet
2017-01-13 23:07 ` Alexander Duyck
@ 2017-01-16 9:02 ` Tariq Toukan
2017-01-16 16:45 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Tariq Toukan @ 2017-01-16 9:02 UTC (permalink / raw)
To: Eric Dumazet, David Miller
Cc: netdev, Erez Shitrit, Eugenia Emantayev, Tariq Toukan
Thanks Eric.
On 13/01/2017 6:39 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Disable BH around the call to napi_schedule() to avoid following warning
>
> [ 52.095499] NOHZ: local_softirq_pending 08
> [ 52.421291] NOHZ: local_softirq_pending 08
> [ 52.608313] NOHZ: local_softirq_pending 08
>
> Fixes: 8d59de8f7bb3 ("net/mlx4_en: Process all completions in RX rings after port goes up")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Erez Shitrit <erezsh@mellanox.com>
> Cc: Eugenia Emantayev <eugenia@mellanox.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 4910d9af19335d4b97d39760c163b41eecc26242..761f8b12399cab245abccc0f7d7f84fde742c14d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1748,8 +1748,11 @@ int mlx4_en_start_port(struct net_device *dev)
> /* Process all completions if exist to prevent
> * the queues freezing if they are full
> */
> - for (i = 0; i < priv->rx_ring_num; i++)
> + for (i = 0; i < priv->rx_ring_num; i++) {
> + local_bh_disable();
> napi_schedule(&priv->rx_cq[i]->napi);
> + local_bh_enable();
> + }
>
> netif_tx_start_all_queues(dev);
> netif_device_attach(dev);
>
>
Acked-by: Tariq Toukan <tariqt@mellanox.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] mlx4: do not call napi_schedule() without care
2017-01-16 9:02 ` Tariq Toukan
@ 2017-01-16 16:45 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-01-16 16:45 UTC (permalink / raw)
To: ttoukan.linux; +Cc: eric.dumazet, netdev, erezsh, eugenia, tariqt
From: Tariq Toukan <ttoukan.linux@gmail.com>
Date: Mon, 16 Jan 2017 11:02:44 +0200
> Thanks Eric.
>
> On 13/01/2017 6:39 PM, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Disable BH around the call to napi_schedule() to avoid following
>> warning
>>
>> [ 52.095499] NOHZ: local_softirq_pending 08
>> [ 52.421291] NOHZ: local_softirq_pending 08
>> [ 52.608313] NOHZ: local_softirq_pending 08
>>
>> Fixes: 8d59de8f7bb3 ("net/mlx4_en: Process all completions in RX rings
>> after port goes up")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
...
> Acked-by: Tariq Toukan <tariqt@mellanox.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-16 16:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-13 16:39 [PATCH net] mlx4: do not call napi_schedule() without care Eric Dumazet
2017-01-13 23:07 ` Alexander Duyck
2017-01-13 23:40 ` Eric Dumazet
2017-01-16 9:02 ` Tariq Toukan
2017-01-16 16:45 ` 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).