On Fri, Jul 27, 2012 at 11:37:59PM +0800, Cong Wang wrote:
> Like the previous patch, slave_disable_netpoll() and __netpoll_cleanup()
> may be called with read_lock() held too, so we should make them
> non-block, by moving the cleanup and kfree() to call_rcu_bh() callbacks.
>
> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> Signed-off-by: Cong Wang <amwang@xxxxxxxxxx>
> ---
> drivers/net/bonding/bond_main.c | 4 +--
> include/linux/netpoll.h | 3 ++
> net/8021q/vlan_dev.c | 6 +----
> net/bridge/br_device.c | 6 +----
> net/core/netpoll.c | 42 +++++++++++++++++++++++++++++---------
> 5 files changed, 38 insertions(+), 23 deletions(-)
><snip>
> struct netpoll_info *npinfo;
> @@ -903,20 +921,24 @@ void __netpoll_cleanup(struct netpoll *np)
> ops->ndo_netpoll_cleanup(np->dev);
>
> RCU_INIT_POINTER(np->dev->npinfo, NULL);
> + call_rcu_bh(&npinfo->rcu, rcu_cleanup_netpoll_info);
> + }
> +}
> +EXPORT_SYMBOL_GPL(__netpoll_cleanup);
>
> - /* avoid racing with NAPI reading npinfo */
> - synchronize_rcu_bh();
> +static void rcu_cleanup_netpoll(struct rcu_head *rcu_head)
> +{
> + struct netpoll *np = container_of(rcu_head, struct netpoll, rcu);
>
> - skb_queue_purge(&npinfo->arp_tx);
> - skb_queue_purge(&npinfo->txq);
> - cancel_delayed_work_sync(&npinfo->tx_work);
> + __netpoll_cleanup(np);
> + kfree(np);
> +}
>
> - /* clean after last, unfinished work */
> - __skb_queue_purge(&npinfo->txq);
> - kfree(npinfo);
> - }
> +void __netpoll_free_rcu(struct netpoll *np)
> +{
> + call_rcu_bh(&np->rcu, rcu_cleanup_netpoll);
Here, and above I see you using an rcu_head to defer cleanup, until after all
pointer uses are dropped, but I don't see any modification of code points that
dereference any struct netpoll pointers to include
rcu_read_lock()/rcu_read_unlock(). Without those using rcu to defer cleanup is
pointless, as the rcu code won't know when its safe to run. You're no better
off that you would be just calling __netpoll_cleanup directly.
Neil
> }
> -EXPORT_SYMBOL_GPL(__netpoll_cleanup);
> +EXPORT_SYMBOL_GPL(__netpoll_free_rcu);
>
> void netpoll_cleanup(struct netpoll *np)
> {
> --
> 1.7.7.6
>
>
