[ovs-dev] [netlink v5 51/61] datapath: Eliminate vport_mutex by protecting vport table with RCU.
Ben Pfaff
blp at nicira.com
Thu Jan 27 00:23:34 UTC 2011
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 <blp at nicira.com>
---
datapath/datapath.c | 20 +++------
datapath/vport-internal_dev.c | 1 +
datapath/vport.c | 94 ++++++----------------------------------
datapath/vport.h | 8 ++--
4 files changed, 25 insertions(+), 98 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 94f908a..599a20b 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 b5406c7..1df6180 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 8289c01..574e3ec 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -16,6 +16,7 @@
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/percpu.h>
+#include <linux/rcupdate.h>
#include <linux/rtnetlink.h>
#include <linux/compat.h>
#include <linux/version.h>
@@ -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 87d5615..ee3b127 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/<devname>/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
--
1.7.1
More information about the dev
mailing list