From: Ben Pfaff Date: Wed, 26 Jan 2011 17:42:28 +0000 (-0800) Subject: datapath: Eliminate vport_mutex by protecting vport table with RCU. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=057dd6d279c20e88f98bbe8328383564391bbc8f;p=openvswitch datapath: Eliminate vport_mutex by protecting vport table with RCU. The vport_mutex really only protects the vport dev_table, which isn't very much. By getting rid of it we take one step toward simplifying the vswitch locking, which will necessarily have to be based mainly around the Generic Netlink genl_mutex once we switch to Generic Netlink. Signed-off-by: Ben Pfaff Acked-by: Jesse Gross --- diff --git a/datapath/datapath.c b/datapath/datapath.c index 94f908a0..599a20b3 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -241,7 +241,6 @@ static struct vport *new_vport(const struct vport_parms *parms) { struct vport *vport; - vport_lock(); vport = vport_add(parms); if (!IS_ERR(vport)) { struct datapath *dp = parms->dp; @@ -251,15 +250,12 @@ static struct vport *new_vport(const struct vport_parms *parms) dp_ifinfo_notify(RTM_NEWLINK, vport); } - vport_unlock(); return vport; } int dp_detach_port(struct vport *p) { - int err; - ASSERT_RTNL(); if (p->port_no != ODPP_LOCAL) @@ -271,11 +267,7 @@ int dp_detach_port(struct vport *p) rcu_assign_pointer(p->dp->ports[p->port_no], NULL); /* Then destroy it. */ - vport_lock(); - err = vport_del(p); - vport_unlock(); - - return err; + return vport_del(p); } /* Must be called with rcu_read_lock. */ @@ -1309,10 +1301,10 @@ static struct datapath *lookup_datapath(struct odp_datapath *odp_datapath, struc struct vport *vport; int dp_idx; - vport_lock(); + rcu_read_lock(); vport = vport_locate(nla_data(a[ODP_DP_ATTR_NAME])); dp_idx = vport && vport->port_no == ODPP_LOCAL ? vport->dp->dp_idx : -1; - vport_unlock(); + rcu_read_unlock(); if (dp_idx < 0) return ERR_PTR(-ENODEV); @@ -1698,15 +1690,15 @@ static struct vport *lookup_vport(struct odp_vport *odp_vport, int dp_idx, port_no; retry: - vport_lock(); + rcu_read_lock(); vport = vport_locate(nla_data(a[ODP_VPORT_ATTR_NAME])); if (!vport) { - vport_unlock(); + rcu_read_unlock(); return ERR_PTR(-ENODEV); } dp_idx = vport->dp->dp_idx; port_no = vport->port_no; - vport_unlock(); + rcu_read_unlock(); dp = get_dp_locked(dp_idx); if (!dp) diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c index b5406c73..1df61801 100644 --- a/datapath/vport-internal_dev.c +++ b/datapath/vport-internal_dev.c @@ -217,6 +217,7 @@ static int internal_dev_destroy(struct vport *vport) dev_set_promiscuity(netdev_vport->dev, -1); unregister_netdevice(netdev_vport->dev); + /* unregister_netdevice() waits for an RCU grace period. */ vport_free(vport); return 0; diff --git a/datapath/vport.c b/datapath/vport.c index 8289c01a..574e3ec6 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -38,55 +39,10 @@ static const struct vport_ops *base_vport_ops_list[] = { static const struct vport_ops **vport_ops_list; static int n_vport_types; +/* Protected by RCU read lock for reading, RTNL lock for writing. */ static struct hlist_head *dev_table; #define VPORT_HASH_BUCKETS 1024 -/* Both RTNL lock and vport_mutex need to be held when updating dev_table. - * - * If you use vport_locate and then perform some operations, you need to hold - * one of these locks if you don't want the vport to be deleted out from under - * you. - * - * If you get a reference to a vport through a datapath, it is protected - * by RCU and you need to hold rcu_read_lock instead when reading. - * - * If multiple locks are taken, the hierarchy is: - * 1. RTNL - * 2. DP - * 3. vport - */ -static DEFINE_MUTEX(vport_mutex); - -/** - * vport_lock - acquire vport lock - * - * Acquire global vport lock. See above comment about locking requirements - * and specific function definitions. May sleep. - */ -void vport_lock(void) -{ - mutex_lock(&vport_mutex); -} - -/** - * vport_unlock - release vport lock - * - * Release lock acquired with vport_lock. - */ -void vport_unlock(void) -{ - mutex_unlock(&vport_mutex); -} - -#define ASSERT_VPORT() \ -do { \ - if (unlikely(!mutex_is_locked(&vport_mutex))) { \ - pr_err("vport lock not held at %s (%d)\n", \ - __FILE__, __LINE__); \ - dump_stack(); \ - } \ -} while (0) - /** * vport_init - initialize vport subsystem * @@ -166,9 +122,7 @@ static struct hlist_head *hash_bucket(const char *name) * * @name: name of port to find * - * Either RTNL or vport lock must be acquired before calling this function - * and held while using the found port. See the locking comments at the - * top of the file. + * Must be called with RTNL or RCU read lock. */ struct vport *vport_locate(const char *name) { @@ -176,32 +130,11 @@ struct vport *vport_locate(const char *name) struct vport *vport; struct hlist_node *node; - if (unlikely(!mutex_is_locked(&vport_mutex) && !rtnl_is_locked())) { - pr_err("neither RTNL nor vport lock held in vport_locate\n"); - dump_stack(); - } - - rcu_read_lock(); - - hlist_for_each_entry(vport, node, bucket, hash_node) + hlist_for_each_entry_rcu(vport, node, bucket, hash_node) if (!strcmp(name, vport_get_name(vport))) - goto out; - - vport = NULL; - -out: - rcu_read_unlock(); - return vport; -} - -static void register_vport(struct vport *vport) -{ - hlist_add_head(&vport->hash_node, hash_bucket(vport_get_name(vport))); -} + return vport; -static void unregister_vport(struct vport *vport) -{ - hlist_del(&vport->hash_node); + return NULL; } static void release_vport(struct kobject *kobj) @@ -270,6 +203,9 @@ struct vport *vport_alloc(int priv_size, const struct vport_ops *ops, const stru * @vport: vport to free * * Frees a vport allocated with vport_alloc() when it is no longer needed. + * + * The caller must ensure that an RCU grace period has passed since the last + * time @vport was in a datapath. */ void vport_free(struct vport *vport) { @@ -285,8 +221,7 @@ void vport_free(struct vport *vport) * @parms: Information about new vport. * * Creates a new vport with the specified configuration (which is dependent on - * device type) and attaches it to a datapath. Both RTNL and vport locks must - * be held. + * device type) and attaches it to a datapath. RTNL lock must be held. */ struct vport *vport_add(const struct vport_parms *parms) { @@ -295,7 +230,6 @@ struct vport *vport_add(const struct vport_parms *parms) int i; ASSERT_RTNL(); - ASSERT_VPORT(); for (i = 0; i < n_vport_types; i++) { if (vport_ops_list[i]->type == parms->type) { @@ -305,7 +239,8 @@ struct vport *vport_add(const struct vport_parms *parms) goto out; } - register_vport(vport); + hlist_add_head_rcu(&vport->hash_node, + hash_bucket(vport_get_name(vport))); return vport; } } @@ -340,14 +275,13 @@ int vport_set_options(struct vport *vport, struct nlattr *options) * @vport: vport to delete. * * Detaches @vport from its datapath and destroys it. It is possible to fail - * for reasons such as lack of memory. Both RTNL and vport locks must be held. + * for reasons such as lack of memory. RTNL lock must be held. */ int vport_del(struct vport *vport) { ASSERT_RTNL(); - ASSERT_VPORT(); - unregister_vport(vport); + hlist_del_rcu(&vport->hash_node); return vport->ops->destroy(vport); } diff --git a/datapath/vport.h b/datapath/vport.h index 87d56153..ee3b127d 100644 --- a/datapath/vport.h +++ b/datapath/vport.h @@ -22,9 +22,6 @@ struct vport_parms; /* The following definitions are for users of the vport subsytem: */ -void vport_lock(void); -void vport_unlock(void); - int vport_init(void); void vport_exit(void); @@ -77,6 +74,7 @@ struct vport_err_stats { /** * struct vport - one port within a datapath + * @rcu: RCU callback head for deferred destruction. * @port_no: Index into @dp's @ports array. * @dp: Datapath to which this port belongs. * @kobj: Represents /sys/class/net//brport. @@ -97,6 +95,7 @@ struct vport_err_stats { * XAPI for Citrix XenServer. Deprecated. */ struct vport { + struct rcu_head rcu; u16 port_no; struct datapath *dp; struct kobject kobj; @@ -151,7 +150,8 @@ struct vport_parms { * @exit: Called at module unload. * @create: Create a new vport configured as specified. On success returns * a new vport allocated with vport_alloc(), otherwise an ERR_PTR() value. - * @destroy: Detach and destroy a vport. + * @destroy: Destroys a vport. Must call vport_free() on the vport but not + * before an RCU grace period has elapsed. * @set_options: Modify the configuration of an existing vport. May be %NULL * if modification is not supported. * @get_options: Appends vport-specific attributes for the configuration of an