[PATCH net-next-2.6] garp: remove one synchronize_rcu() call

[PATCH net-next-2.6] garp: remove one synchronize_rcu() call

[Eric Dumazet]

Speedup vlan dismantling in CONFIG_VLAN_8021Q_GVRP=y cases,
by using a call_rcu() to free the memory instead of waiting with
expensive synchronize_rcu() [ while RTNL is held ]

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ben Greear <greearb@candelatech.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
Note: I'll take care of using kfree_rcu() when available in net-next-2.6

 include/net/garp.h |    1 +
 net/802/garp.c     |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/net/garp.h b/include/net/garp.h
index f4c2959..8cabbf08 100644
--- a/include/net/garp.h
+++ b/include/net/garp.h
@@ -108,6 +108,7 @@ struct garp_applicant {
 
 struct garp_port {
 	struct garp_applicant __rcu	*applicants[GARP_APPLICATION_MAX + 1];
+	struct rcu_head			rcu;
 };
 
 extern int	garp_register_application(struct garp_application *app);
diff --git a/net/802/garp.c b/net/802/garp.c
index c1df2da..5dbe896 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -544,6 +544,11 @@ static int garp_init_port(struct net_device *dev)
 	return 0;
 }
 
+static void garp_kfree_rcu(struct rcu_head *head)
+{
+	kfree(container_of(head, struct garp_port, rcu));
+}
+
 static void garp_release_port(struct net_device *dev)
 {
 	struct garp_port *port = rtnl_dereference(dev->garp_port);
@@ -554,8 +559,7 @@ static void garp_release_port(struct net_device *dev)
 			return;
 	}
 	rcu_assign_pointer(dev->garp_port, NULL);
-	synchronize_rcu();
-	kfree(port);
+	call_rcu(&port->rcu, garp_kfree_rcu);
 }
 
 int garp_init_applicant(struct net_device *dev, struct garp_application *appl)

[PATCH net-next-2.6] vlan: remove one synchronize_net() call

[Eric Dumazet]

At VLAN dismantle phase, unregister_vlan_dev() makes one
synchronize_net() call after vlan_group_set_device(grp, vlan_id, NULL).

This call can be safely removed because we are calling
unregister_netdevice_queue() to queue device for deletion, and this
process needs at least one rcu grace period to complete.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ben Greear <greearb@candelatech.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Jesse Gross <jesse@nicira.com>
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/if_vlan.h |    1 -
 net/8021q/vlan.c        |   10 ++++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 546d9d3..290bd8a 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -86,7 +86,6 @@ struct vlan_group {
 					    * the vlan is attached to.
 					    */
 	unsigned int		nr_vlans;
-	int			killall;
 	struct hlist_node	hlist;	/* linked list */
 	struct net_device **vlan_devices_arrays[VLAN_GROUP_ARRAY_SPLIT_PARTS];
 	struct rcu_head		rcu;
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 969e700..718d635d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -120,9 +120,10 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
 	grp->nr_vlans--;
 
 	vlan_group_set_device(grp, vlan_id, NULL);
-	if (!grp->killall)
-		synchronize_net();
-
+	/* Because unregister_netdevice_queue() makes sure at least one rcu
+	 * grace period is respected before device freeing,
+	 * we dont need to call synchronize_net() here.
+	 */
 	unregister_netdevice_queue(dev, head);
 
 	/* If the group is now empty, kill off the group. */
@@ -478,9 +479,6 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 		if (dev->reg_state != NETREG_UNREGISTERING)
 			break;
 
-		/* Delete all VLANs for this dev. */
-		grp->killall = 1;
-
 		for (i = 0; i < VLAN_N_VID; i++) {
 			vlandev = vlan_group_get_device(grp, i);
 			if (!vlandev)

Re: [PATCH net-next-2.6] vlan: remove one synchronize_net() call

[Jesse Gross]

On Mon, May 9, 2011 at 7:40 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> At VLAN dismantle phase, unregister_vlan_dev() makes one
> synchronize_net() call after vlan_group_set_device(grp, vlan_id, NULL).
>
> This call can be safely removed because we are calling
> unregister_netdevice_queue() to queue device for deletion, and this
> process needs at least one rcu grace period to complete.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ben Greear <greearb@candelatech.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Jesse Gross <jesse@nicira.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Acked-by: Jesse Gross <jesse@nicira.com>

Re: [PATCH net-next-2.6] garp: remove one synchronize_rcu() call

[Patrick McHardy]

Am 09.05.2011 15:35, schrieb Eric Dumazet:
> Speedup vlan dismantling in CONFIG_VLAN_8021Q_GVRP=y cases,
> by using a call_rcu() to free the memory instead of waiting with
> expensive synchronize_rcu() [ while RTNL is held ]
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ben Greear <greearb@candelatech.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> Note: I'll take care of using kfree_rcu() when available in net-next-2.6
> 

Looks good to me.

Re: [PATCH net-next-2.6] vlan: remove one synchronize_net() call

[Patrick McHardy]

Am 09.05.2011 16:40, schrieb Eric Dumazet:
> At VLAN dismantle phase, unregister_vlan_dev() makes one
> synchronize_net() call after vlan_group_set_device(grp, vlan_id, NULL).
> 
> This call can be safely removed because we are calling
> unregister_netdevice_queue() to queue device for deletion, and this
> process needs at least one rcu grace period to complete.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ben Greear <greearb@candelatech.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Jesse Gross <jesse@nicira.com>
> Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  include/linux/if_vlan.h |    1 -
>  net/8021q/vlan.c        |   10 ++++------
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 546d9d3..290bd8a 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -86,7 +86,6 @@ struct vlan_group {
>  					    * the vlan is attached to.
>  					    */
>  	unsigned int		nr_vlans;
> -	int			killall;

Looks good, thanks Eric.

Re: [PATCH net-next-2.6] garp: remove one synchronize_rcu() call

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 09 May 2011 15:35:55 +0200

> Speedup vlan dismantling in CONFIG_VLAN_8021Q_GVRP=y cases,
> by using a call_rcu() to free the memory instead of waiting with
> expensive synchronize_rcu() [ while RTNL is held ]
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

Re: [PATCH net-next-2.6] vlan: remove one synchronize_net() call

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 09 May 2011 16:40:44 +0200

> At VLAN dismantle phase, unregister_vlan_dev() makes one
> synchronize_net() call after vlan_group_set_device(grp, vlan_id, NULL).
> 
> This call can be safely removed because we are calling
> unregister_netdevice_queue() to queue device for deletion, and this
> process needs at least one rcu grace period to complete.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

[PATCH net-next-2.6] garp: remove last synchronize_rcu() call

[Eric Dumazet]

Le lundi 09 mai 2011 à 20:25 +0200, Patrick McHardy a écrit :
> Am 09.05.2011 15:35, schrieb Eric Dumazet:
> > Speedup vlan dismantling in CONFIG_VLAN_8021Q_GVRP=y cases,
> > by using a call_rcu() to free the memory instead of waiting with
> > expensive synchronize_rcu() [ while RTNL is held ]
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Ben Greear <greearb@candelatech.com>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> > Note: I'll take care of using kfree_rcu() when available in net-next-2.6
> > 
> 
> Looks good to me.
> 

Here is a followup on this patch, thanks !


[PATCH net-next-2.6] garp: remove last synchronize_rcu() call

When removing last vlan from a device, garp_uninit_applicant() calls
synchronize_rcu() to make sure no user can still manipulate struct
garp_applicant before we free it.

Use call_rcu() instead, as a step to further net_device dismantle
optimizations.

Add the temporary garp_cleanup_module() function to make sure no pending
call_rcu() are left at module unload time [ this will be removed when
kfree_rcu() is available ]

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ben Greear <greearb@candelatech.com>
---
 include/net/garp.h |    1 +
 net/802/garp.c     |   14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/garp.h b/include/net/garp.h
index 8cabbf08..834d8ad 100644
--- a/include/net/garp.h
+++ b/include/net/garp.h
@@ -104,6 +104,7 @@ struct garp_applicant {
 	struct sk_buff_head	queue;
 	struct sk_buff		*pdu;
 	struct rb_root		gid;
+	struct rcu_head		rcu;
 };
 
 struct garp_port {
diff --git a/net/802/garp.c b/net/802/garp.c
index 5dbe896..f8300a8 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -603,6 +603,11 @@ err1:
 }
 EXPORT_SYMBOL_GPL(garp_init_applicant);
 
+static void garp_app_kfree_rcu(struct rcu_head *head)
+{
+	kfree(container_of(head, struct garp_applicant, rcu));
+}
+
 void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl)
 {
 	struct garp_port *port = rtnl_dereference(dev->garp_port);
@@ -611,7 +616,6 @@ void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl
 	ASSERT_RTNL();
 
 	rcu_assign_pointer(port->applicants[appl->type], NULL);
-	synchronize_rcu();
 
 	/* Delete timer and generate a final TRANSMIT_PDU event to flush out
 	 * all pending messages before the applicant is gone. */
@@ -621,7 +625,7 @@ void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl
 	garp_queue_xmit(app);
 
 	dev_mc_del(dev, appl->proto.group_address);
-	kfree(app);
+	call_rcu(&app->rcu, garp_app_kfree_rcu);
 	garp_release_port(dev);
 }
 EXPORT_SYMBOL_GPL(garp_uninit_applicant);
@@ -639,3 +643,9 @@ void garp_unregister_application(struct garp_application *appl)
 	stp_proto_unregister(&appl->proto);
 }
 EXPORT_SYMBOL_GPL(garp_unregister_application);
+
+static void __exit garp_cleanup_module(void)
+{
+	rcu_barrier(); /* Wait for completion of call_rcu()'s */
+}
+module_exit(garp_cleanup_module);

Re: [PATCH net-next-2.6] garp: remove last synchronize_rcu() call

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 May 2011 15:29:17 +0200

> [PATCH net-next-2.6] garp: remove last synchronize_rcu() call
> 
> When removing last vlan from a device, garp_uninit_applicant() calls
> synchronize_rcu() to make sure no user can still manipulate struct
> garp_applicant before we free it.
> 
> Use call_rcu() instead, as a step to further net_device dismantle
> optimizations.
> 
> Add the temporary garp_cleanup_module() function to make sure no pending
> call_rcu() are left at module unload time [ this will be removed when
> kfree_rcu() is available ]
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Looks good, applied, thanks Eric.