Attachment 312200 Details for Bug 441716 – [BRIDGE]: eliminate workqueue for carrier check

[patch] [BRIDGE]: eliminate workqueue for carrier check

p (text/plain), 4.92 KB, created by Herbert Xu on 2008-07-19 12:13:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Herbert Xu

Created: 2008-07-19 12:13:25 UTC

Size: 4.92 KB

patch

obsolete

>commit 269def7c505b4d229f9ad49bf88543d1e605533e
>Author: Stephen Hemminger <shemminger@linux-foundation.org>
>Date:   Thu Feb 22 01:10:18 2007 -0800
>
>    [BRIDGE]: eliminate workqueue for carrier check
>    
>    Having a work queue for checking carrier leads to lots of race issues.
>    Simpler to just get the cost when data structure is created and
>    update on change.
>    
>    Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>    Signed-off-by: David S. Miller <davem@davemloft.net>
>
>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>index aff6a77..6845a25 100644
>--- a/net/bridge/br_if.c
>+++ b/net/bridge/br_if.c
>@@ -77,26 +77,15 @@ static int port_cost(struct net_device *dev)
>  * Called from work queue to allow for calling functions that
>  * might sleep (such as speed check), and to debounce.
>  */
>-static void port_carrier_check(struct work_struct *work)
>+void br_port_carrier_check(struct net_bridge_port *p)
> {
>-	struct net_bridge_port *p;
>-	struct net_device *dev;
>-	struct net_bridge *br;
>-
>-	dev = container_of(work, struct net_bridge_port,
>-			   carrier_check.work)->dev;
>-	work_release(work);
>-
>-	rtnl_lock();
>-	p = dev->br_port;
>-	if (!p)
>-		goto done;
>-	br = p->br;
>+	struct net_device *dev = p->dev;
>+	struct net_bridge *br = p->br;
> 
> 	if (netif_carrier_ok(dev))
> 		p->path_cost = port_cost(dev);
> 
>-	if (br->dev->flags & IFF_UP) {
>+	if (netif_running(br->dev)) {
> 		spin_lock_bh(&br->lock);
> 		if (netif_carrier_ok(dev)) {
> 			if (p->state == BR_STATE_DISABLED)
>@@ -107,9 +96,6 @@ static void port_carrier_check(struct work_struct *work)
> 		}
> 		spin_unlock_bh(&br->lock);
> 	}
>-done:
>-	dev_put(dev);
>-	rtnl_unlock();
> }
> 
> static void release_nbp(struct kobject *kobj)
>@@ -162,9 +148,6 @@ static void del_nbp(struct net_bridge_port *p)
> 
> 	dev_set_promiscuity(dev, -1);
> 
>-	if (cancel_delayed_work(&p->carrier_check))
>-		dev_put(dev);
>-
> 	spin_lock_bh(&br->lock);
> 	br_stp_disable_port(p);
> 	spin_unlock_bh(&br->lock);
>@@ -282,7 +265,6 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
> 	p->port_no = index;
> 	br_init_port(p);
> 	p->state = BR_STATE_DISABLED;
>-	INIT_DELAYED_WORK_NAR(&p->carrier_check, port_carrier_check);
> 	br_stp_port_timer_init(p);
> 
> 	kobject_init(&p->kobj);
>@@ -446,12 +428,10 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
> 	spin_lock_bh(&br->lock);
> 	br_stp_recalculate_bridge_id(br);
> 	br_features_recompute(br);
>-	if (schedule_delayed_work(&p->carrier_check, BR_PORT_DEBOUNCE))
>-		dev_hold(dev);
>-
> 	spin_unlock_bh(&br->lock);
> 
> 	dev_set_mtu(br->dev, br_min_mtu(br));
>+
> 	kobject_uevent(&p->kobj, KOBJ_ADD);
> 
> 	return 0;
>diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
>index 3311c4e..37357ed 100644
>--- a/net/bridge/br_notify.c
>+++ b/net/bridge/br_notify.c
>@@ -42,51 +42,48 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
> 
> 	br = p->br;
> 
>-	spin_lock_bh(&br->lock);
> 	switch (event) {
> 	case NETDEV_CHANGEMTU:
> 		dev_set_mtu(br->dev, br_min_mtu(br));
> 		break;
> 
> 	case NETDEV_CHANGEADDR:
>+		spin_lock_bh(&br->lock);
> 		br_fdb_changeaddr(p, dev->dev_addr);
> 		br_ifinfo_notify(RTM_NEWLINK, p);
> 		br_stp_recalculate_bridge_id(br);
>+		spin_unlock_bh(&br->lock);
> 		break;
> 
> 	case NETDEV_CHANGE:
>-		if (br->dev->flags & IFF_UP)
>-			if (schedule_delayed_work(&p->carrier_check,
>-						BR_PORT_DEBOUNCE))
>-				dev_hold(dev);
>+		br_port_carrier_check(p);
> 		break;
> 
> 	case NETDEV_FEAT_CHANGE:
>-		if (br->dev->flags & IFF_UP)
>+		spin_lock_bh(&br->lock);
>+		if (netif_running(br->dev))
> 			br_features_recompute(br);
>-
>-		/* could do recursive feature change notification
>-		 * but who would care??
>-		 */
>+		spin_unlock_bh(&br->lock);
> 		break;
> 
> 	case NETDEV_DOWN:
>+		spin_lock_bh(&br->lock);
> 		if (br->dev->flags & IFF_UP)
> 			br_stp_disable_port(p);
>+		spin_unlock_bh(&br->lock);
> 		break;
> 
> 	case NETDEV_UP:
>+		spin_lock_bh(&br->lock);
> 		if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP))
> 			br_stp_enable_port(p);
>+		spin_unlock_bh(&br->lock);
> 		break;
> 
> 	case NETDEV_UNREGISTER:
>-		spin_unlock_bh(&br->lock);
> 		br_del_if(br, dev);
>-		goto done;
>+		break;
> 	}
>-	spin_unlock_bh(&br->lock);
> 
>- done:
> 	return NOTIFY_DONE;
> }
>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>index 16fc47a..cc3f1c9 100644
>--- a/net/bridge/br_private.h
>+++ b/net/bridge/br_private.h
>@@ -26,8 +26,6 @@
> #define BR_PORT_BITS	10
> #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
> 
>-#define BR_PORT_DEBOUNCE (HZ/10)
>-
> #define BR_VERSION	"2.2"
> 
> typedef struct bridge_id bridge_id;
>@@ -81,7 +79,6 @@ struct net_bridge_port
> 	struct timer_list		hold_timer;
> 	struct timer_list		message_age_timer;
> 	struct kobject			kobj;
>-	struct delayed_work		carrier_check;
> 	struct rcu_head			rcu;
> };
> 
>@@ -172,6 +169,7 @@ extern void br_flood_forward(struct net_bridge *br,
> 		      int clone);
> 
> /* br_if.c */
>+extern void br_port_carrier_check(struct net_bridge_port *p);
> extern int br_add_bridge(const char *name);
> extern int br_del_bridge(const char *name);
> extern void br_cleanup_bridges(void);

Actions: View | Diff

Attachments on bug 441716: 302697 | 303426 | 310783 | 310792 | 312200 | 313096 | 313197