[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20181020113355.GA12669@hmswarspite.think-freely.org>
Date: Sat, 20 Oct 2018 07:33:55 -0400
From: Neil Horman <nhorman@...driver.com>
To: "Banerjee, Debabrata" <dbanerje@...mai.com>
Cc: "David S . Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2] netpoll: allow cleanup to be synchronous
On Fri, Oct 19, 2018 at 08:46:45PM +0000, Banerjee, Debabrata wrote:
> > From: Neil Horman <nhorman@...driver.com>
>
> > I presume you've tested this with some of the stacked devices? I think I'm
> > ok with this change, but I'd like confirmation that its worked.
> >
> > Neil
>
> Yes I've tested this on a bond device with vlan stacked on top.
>
> -Deb
>
Cool, thanks
Acked-by: Neil Horman <nhorman@...driver.com>
> >
> > > CC: Neil Horman <nhorman@...driver.com>
> > > CC: "David S. Miller" <davem@...emloft.net>
> > > Signed-off-by: Debabrata Banerjee <dbanerje@...mai.com>
> > > ---
> > > drivers/net/bonding/bond_main.c | 3 ++-
> > > drivers/net/macvlan.c | 2 +-
> > > drivers/net/team/team.c | 5 +----
> > > include/linux/netpoll.h | 4 +---
> > > net/8021q/vlan_dev.c | 3 +--
> > > net/bridge/br_device.c | 2 +-
> > > net/core/netpoll.c | 20 +++++---------------
> > > net/dsa/slave.c | 2 +-
> > > 8 files changed, 13 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/net/bonding/bond_main.c
> > > b/drivers/net/bonding/bond_main.c index ee28ec9e0aba..ffa37adb7681
> > > 100644
> > > --- a/drivers/net/bonding/bond_main.c
> > > +++ b/drivers/net/bonding/bond_main.c
> > > @@ -963,7 +963,8 @@ static inline void slave_disable_netpoll(struct slave
> > *slave)
> > > return;
> > >
> > > slave->np = NULL;
> > > - __netpoll_free_async(np);
> > > +
> > > + __netpoll_free(np);
> > > }
> > >
> > > static void bond_poll_controller(struct net_device *bond_dev) diff
> > > --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index
> > > cfda146f3b3b..fc8d5f1ee1ad 100644
> > > --- a/drivers/net/macvlan.c
> > > +++ b/drivers/net/macvlan.c
> > > @@ -1077,7 +1077,7 @@ static void macvlan_dev_netpoll_cleanup(struct
> > > net_device *dev)
> > >
> > > vlan->netpoll = NULL;
> > >
> > > - __netpoll_free_async(netpoll);
> > > + __netpoll_free(netpoll);
> > > }
> > > #endif /* CONFIG_NET_POLL_CONTROLLER */
> > >
> > > diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index
> > > d887016e54b6..db633ae9f784 100644
> > > --- a/drivers/net/team/team.c
> > > +++ b/drivers/net/team/team.c
> > > @@ -1104,10 +1104,7 @@ static void team_port_disable_netpoll(struct
> > team_port *port)
> > > return;
> > > port->np = NULL;
> > >
> > > - /* Wait for transmitting packets to finish before freeing. */
> > > - synchronize_rcu_bh();
> > > - __netpoll_cleanup(np);
> > > - kfree(np);
> > > + __netpoll_free(np);
> > > }
> > > #else
> > > static int team_port_enable_netpoll(struct team_port *port) diff
> > > --git a/include/linux/netpoll.h b/include/linux/netpoll.h index
> > > 3ef82d3a78db..676f1ff161a9 100644
> > > --- a/include/linux/netpoll.h
> > > +++ b/include/linux/netpoll.h
> > > @@ -31,8 +31,6 @@ struct netpoll {
> > > bool ipv6;
> > > u16 local_port, remote_port;
> > > u8 remote_mac[ETH_ALEN];
> > > -
> > > - struct work_struct cleanup_work;
> > > };
> > >
> > > struct netpoll_info {
> > > @@ -63,7 +61,7 @@ int netpoll_parse_options(struct netpoll *np, char
> > > *opt); int __netpoll_setup(struct netpoll *np, struct net_device
> > > *ndev); int netpoll_setup(struct netpoll *np); void
> > > __netpoll_cleanup(struct netpoll *np); -void
> > > __netpoll_free_async(struct netpoll *np);
> > > +void __netpoll_free(struct netpoll *np);
> > > void netpoll_cleanup(struct netpoll *np); void
> > > netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
> > > struct net_device *dev);
> > > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index
> > > 546af0e73ac3..ff720f1ebf73 100644
> > > --- a/net/8021q/vlan_dev.c
> > > +++ b/net/8021q/vlan_dev.c
> > > @@ -756,8 +756,7 @@ static void vlan_dev_netpoll_cleanup(struct
> > net_device *dev)
> > > return;
> > >
> > > vlan->netpoll = NULL;
> > > -
> > > - __netpoll_free_async(netpoll);
> > > + __netpoll_free(netpoll);
> > > }
> > > #endif /* CONFIG_NET_POLL_CONTROLLER */
> > >
> > > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index
> > > e053a4e43758..c6abf927f0c9 100644
> > > --- a/net/bridge/br_device.c
> > > +++ b/net/bridge/br_device.c
> > > @@ -344,7 +344,7 @@ void br_netpoll_disable(struct net_bridge_port *p)
> > >
> > > p->np = NULL;
> > >
> > > - __netpoll_free_async(np);
> > > + __netpoll_free(np);
> > > }
> > >
> > > #endif
> > > diff --git a/net/core/netpoll.c b/net/core/netpoll.c index
> > > de1d1ba92f2d..6ac71624ead4 100644
> > > --- a/net/core/netpoll.c
> > > +++ b/net/core/netpoll.c
> > > @@ -591,7 +591,6 @@ int __netpoll_setup(struct netpoll *np, struct
> > > net_device *ndev)
> > >
> > > np->dev = ndev;
> > > strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
> > > - INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);
> > >
> > > if (ndev->priv_flags & IFF_DISABLE_NETPOLL) {
> > > np_err(np, "%s doesn't support polling, aborting\n", @@ -
> > 790,10
> > > +789,6 @@ void __netpoll_cleanup(struct netpoll *np) {
> > > struct netpoll_info *npinfo;
> > >
> > > - /* rtnl_dereference would be preferable here but
> > > - * rcu_cleanup_netpoll path can put us in here safely without
> > > - * holding the rtnl, so plain rcu_dereference it is
> > > - */
> > > npinfo = rtnl_dereference(np->dev->npinfo);
> > > if (!npinfo)
> > > return;
> > > @@ -814,21 +809,16 @@ void __netpoll_cleanup(struct netpoll *np) }
> > > EXPORT_SYMBOL_GPL(__netpoll_cleanup);
> > >
> > > -static void netpoll_async_cleanup(struct work_struct *work)
> > > +void __netpoll_free(struct netpoll *np)
> > > {
> > > - struct netpoll *np = container_of(work, struct netpoll,
> > cleanup_work);
> > > + ASSERT_RTNL();
> > >
> > > - rtnl_lock();
> > > + /* Wait for transmitting packets to finish before freeing. */
> > > + synchronize_rcu_bh();
> > > __netpoll_cleanup(np);
> > > - rtnl_unlock();
> > > kfree(np);
> > > }
> > > -
> > > -void __netpoll_free_async(struct netpoll *np) -{
> > > - schedule_work(&np->cleanup_work);
> > > -}
> > > -EXPORT_SYMBOL_GPL(__netpoll_free_async);
> > > +EXPORT_SYMBOL_GPL(__netpoll_free);
> > >
> > > void netpoll_cleanup(struct netpoll *np) { diff --git
> > > a/net/dsa/slave.c b/net/dsa/slave.c index 3f840b6eea69..3679e13b2ead
> > > 100644
> > > --- a/net/dsa/slave.c
> > > +++ b/net/dsa/slave.c
> > > @@ -722,7 +722,7 @@ static void dsa_slave_netpoll_cleanup(struct
> > > net_device *dev)
> > >
> > > p->netpoll = NULL;
> > >
> > > - __netpoll_free_async(netpoll);
> > > + __netpoll_free(netpoll);
> > > }
> > >
> > > static void dsa_slave_poll_controller(struct net_device *dev)
> > > --
> > > 2.19.1
> > >
> > >
>
Powered by blists - more mailing lists