lkml.org 
[lkml]   [2012]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/7] netpoll: make __netpoll_cleanup non-block
    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@davemloft.net>
    > Signed-off-by: Cong Wang <amwang@redhat.com>
    > ---
    > 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
    >
    >


    \
     
     \ /
      Last update: 2012-07-27 21:21    [W:0.027 / U:2.548 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site