Commit fb4fa76a authored by Neil Horman's avatar Neil Horman Committed by David S. Miller

net: Convert netpoll blocking api in bonding driver to be a counter

A while back I made some changes to enable netpoll in the bonding driver.  Among
them was a per-cpu flag that indicated we were in a path that held locks which
could cause the netpoll path to block in during tx, and as such the tx path
should queue the frame for later use.  This appears to have given rise to a
regression.  If one of those paths on which we hold the per-cpu flag yields the
cpu, its possible for us to come back on a different cpu, leading to us clearing
a different flag than we set.  This results in odd netpoll drops, and BUG
backtraces appearing in the log, as we check to make sure that we only clear set
bits, and only set clear bits.  I had though briefly about changing the
offending paths so that they wouldn't sleep, but looking at my origional work
more closely, it doesn't appear that a per-cpu flag is warranted.  We alrady
gate the checking of this flag on IFF_IN_NETPOLL, so we don't hit this in the
normal tx case anyway.  And practically speaking, the normal use case for
netpoll is to only have one client anyway, so we're not going to erroneously
queue netpoll frames when its actually safe to do so.  As such, lets just
convert that per-cpu flag to an atomic counter.  It fixes the rescheduling bugs,
is equivalent from a performance perspective and actually eliminates some code
in the process.

Tested by the reporter and myself, successfully
Reported-by: default avatarLiang Zheng <lzheng@redhat.com>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: David S. Miller <davem@davemloft.net>
Signed-off-by: default avatarNeil Horman <nhorman@tuxdriver.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 4e085e76
...@@ -171,7 +171,7 @@ MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on link ...@@ -171,7 +171,7 @@ MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on link
/*----------------------------- Global variables ----------------------------*/ /*----------------------------- Global variables ----------------------------*/
#ifdef CONFIG_NET_POLL_CONTROLLER #ifdef CONFIG_NET_POLL_CONTROLLER
cpumask_var_t netpoll_block_tx; atomic_t netpoll_block_tx = ATOMIC_INIT(0);
#endif #endif
static const char * const version = static const char * const version =
...@@ -5299,13 +5299,6 @@ static int __init bonding_init(void) ...@@ -5299,13 +5299,6 @@ static int __init bonding_init(void)
if (res) if (res)
goto out; goto out;
#ifdef CONFIG_NET_POLL_CONTROLLER
if (!alloc_cpumask_var(&netpoll_block_tx, GFP_KERNEL)) {
res = -ENOMEM;
goto out;
}
#endif
res = register_pernet_subsys(&bond_net_ops); res = register_pernet_subsys(&bond_net_ops);
if (res) if (res)
goto out; goto out;
...@@ -5334,9 +5327,6 @@ static int __init bonding_init(void) ...@@ -5334,9 +5327,6 @@ static int __init bonding_init(void)
rtnl_link_unregister(&bond_link_ops); rtnl_link_unregister(&bond_link_ops);
err_link: err_link:
unregister_pernet_subsys(&bond_net_ops); unregister_pernet_subsys(&bond_net_ops);
#ifdef CONFIG_NET_POLL_CONTROLLER
free_cpumask_var(netpoll_block_tx);
#endif
goto out; goto out;
} }
...@@ -5353,7 +5343,10 @@ static void __exit bonding_exit(void) ...@@ -5353,7 +5343,10 @@ static void __exit bonding_exit(void)
unregister_pernet_subsys(&bond_net_ops); unregister_pernet_subsys(&bond_net_ops);
#ifdef CONFIG_NET_POLL_CONTROLLER #ifdef CONFIG_NET_POLL_CONTROLLER
free_cpumask_var(netpoll_block_tx); /*
* Make sure we don't have an imbalance on our netpoll blocking
*/
WARN_ON(atomic_read(&netpoll_block_tx));
#endif #endif
} }
......
...@@ -119,26 +119,22 @@ ...@@ -119,26 +119,22 @@
#ifdef CONFIG_NET_POLL_CONTROLLER #ifdef CONFIG_NET_POLL_CONTROLLER
extern cpumask_var_t netpoll_block_tx; extern atomic_t netpoll_block_tx;
static inline void block_netpoll_tx(void) static inline void block_netpoll_tx(void)
{ {
preempt_disable(); atomic_inc(&netpoll_block_tx);
BUG_ON(cpumask_test_and_set_cpu(smp_processor_id(),
netpoll_block_tx));
} }
static inline void unblock_netpoll_tx(void) static inline void unblock_netpoll_tx(void)
{ {
BUG_ON(!cpumask_test_and_clear_cpu(smp_processor_id(), atomic_dec(&netpoll_block_tx);
netpoll_block_tx));
preempt_enable();
} }
static inline int is_netpoll_tx_blocked(struct net_device *dev) static inline int is_netpoll_tx_blocked(struct net_device *dev)
{ {
if (unlikely(dev->priv_flags & IFF_IN_NETPOLL)) if (unlikely(dev->priv_flags & IFF_IN_NETPOLL))
return cpumask_test_cpu(smp_processor_id(), netpoll_block_tx); return atomic_read(&netpoll_block_tx);
return 0; return 0;
} }
#else #else
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment