aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorNeil Horman <nhorman@tuxdriver.com>2010-12-06 04:05:50 -0500
committerDavid S. Miller <davem@davemloft.net>2010-12-09 23:33:46 -0500
commitfb4fa76a1fa59340154c42d998d700e1f8bf21e0 (patch)
treec9d66f1bd9468708ad5a241aa818c075f93f071f /drivers
parent4e085e76cbe558b79b54cbab772f61185879bc64 (diff)
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: Liang 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: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/net/bonding/bond_main.c17
-rw-r--r--drivers/net/bonding/bonding.h12
2 files changed, 9 insertions, 20 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2fee00a4c9ef..d0ea760ce419 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -171,7 +171,7 @@ MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on link
171/*----------------------------- Global variables ----------------------------*/ 171/*----------------------------- Global variables ----------------------------*/
172 172
173#ifdef CONFIG_NET_POLL_CONTROLLER 173#ifdef CONFIG_NET_POLL_CONTROLLER
174cpumask_var_t netpoll_block_tx; 174atomic_t netpoll_block_tx = ATOMIC_INIT(0);
175#endif 175#endif
176 176
177static const char * const version = 177static const char * const version =
@@ -5299,13 +5299,6 @@ static int __init bonding_init(void)
5299 if (res) 5299 if (res)
5300 goto out; 5300 goto out;
5301 5301
5302#ifdef CONFIG_NET_POLL_CONTROLLER
5303 if (!alloc_cpumask_var(&netpoll_block_tx, GFP_KERNEL)) {
5304 res = -ENOMEM;
5305 goto out;
5306 }
5307#endif
5308
5309 res = register_pernet_subsys(&bond_net_ops); 5302 res = register_pernet_subsys(&bond_net_ops);
5310 if (res) 5303 if (res)
5311 goto out; 5304 goto out;
@@ -5334,9 +5327,6 @@ err:
5334 rtnl_link_unregister(&bond_link_ops); 5327 rtnl_link_unregister(&bond_link_ops);
5335err_link: 5328err_link:
5336 unregister_pernet_subsys(&bond_net_ops); 5329 unregister_pernet_subsys(&bond_net_ops);
5337#ifdef CONFIG_NET_POLL_CONTROLLER
5338 free_cpumask_var(netpoll_block_tx);
5339#endif
5340 goto out; 5330 goto out;
5341 5331
5342} 5332}
@@ -5353,7 +5343,10 @@ static void __exit bonding_exit(void)
5353 unregister_pernet_subsys(&bond_net_ops); 5343 unregister_pernet_subsys(&bond_net_ops);
5354 5344
5355#ifdef CONFIG_NET_POLL_CONTROLLER 5345#ifdef CONFIG_NET_POLL_CONTROLLER
5356 free_cpumask_var(netpoll_block_tx); 5346 /*
5347 * Make sure we don't have an imbalance on our netpoll blocking
5348 */
5349 WARN_ON(atomic_read(&netpoll_block_tx));
5357#endif 5350#endif
5358} 5351}
5359 5352
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4eedb12df6ca..c2f081352a03 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -119,26 +119,22 @@
119 119
120 120
121#ifdef CONFIG_NET_POLL_CONTROLLER 121#ifdef CONFIG_NET_POLL_CONTROLLER
122extern cpumask_var_t netpoll_block_tx; 122extern atomic_t netpoll_block_tx;
123 123
124static inline void block_netpoll_tx(void) 124static inline void block_netpoll_tx(void)
125{ 125{
126 preempt_disable(); 126 atomic_inc(&netpoll_block_tx);
127 BUG_ON(cpumask_test_and_set_cpu(smp_processor_id(),
128 netpoll_block_tx));
129} 127}
130 128
131static inline void unblock_netpoll_tx(void) 129static inline void unblock_netpoll_tx(void)
132{ 130{
133 BUG_ON(!cpumask_test_and_clear_cpu(smp_processor_id(), 131 atomic_dec(&netpoll_block_tx);
134 netpoll_block_tx));
135 preempt_enable();
136} 132}
137 133
138static inline int is_netpoll_tx_blocked(struct net_device *dev) 134static inline int is_netpoll_tx_blocked(struct net_device *dev)
139{ 135{
140 if (unlikely(dev->priv_flags & IFF_IN_NETPOLL)) 136 if (unlikely(dev->priv_flags & IFF_IN_NETPOLL))
141 return cpumask_test_cpu(smp_processor_id(), netpoll_block_tx); 137 return atomic_read(&netpoll_block_tx);
142 return 0; 138 return 0;
143} 139}
144#else 140#else