[ovs-dev] [PATCH 09/10] datapath: Allocate vports in more RCU friendly manner.

[Jesse Gross]

In a few places, when creating a new vport we also need to allocate
some memory for configuration that can change.  This data is protected
by RCU but we directly access the memory when initializing it.  This
is fine, since the vport has not yet been published and we use the
apropriate memory barriers when doing so.  However, it makes tools
like sparse unhappy and is also asymmetric since we use RCU to
derefence the pointers but not to assign them.  This cleans that
up somewhat by initializing the memory first and then using RCU
to assign it, which makes everyone happy.

Found with sparse.

Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 datapath/tunnel.c      |   17 ++++++++++-------
 datapath/vport-patch.c |   18 ++++++++++--------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index d49d928..e42e0e3 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -1372,6 +1372,7 @@ struct vport *tnl_create(const struct vport_parms *parms,
 {
 	struct vport *vport;
 	struct tnl_vport *tnl_vport;
+	struct tnl_mutable_config *mutable;
 	int initial_frag_id;
 	int err;
 
@@ -1386,19 +1387,19 @@ struct vport *tnl_create(const struct vport_parms *parms,
 	strcpy(tnl_vport->name, parms->name);
 	tnl_vport->tnl_ops = tnl_ops;
 
-	tnl_vport->mutable = kzalloc(sizeof(struct tnl_mutable_config), GFP_KERNEL);
-	if (!tnl_vport->mutable) {
+	mutable = kzalloc(sizeof(struct tnl_mutable_config), GFP_KERNEL);
+	if (!mutable) {
 		err = -ENOMEM;
 		goto error_free_vport;
 	}
 
-	vport_gen_rand_ether_addr(tnl_vport->mutable->eth_addr);
-	tnl_vport->mutable->mtu = ETH_DATA_LEN;
+	vport_gen_rand_ether_addr(mutable->eth_addr);
+	mutable->mtu = ETH_DATA_LEN;
 
 	get_random_bytes(&initial_frag_id, sizeof(int));
 	atomic_set(&tnl_vport->frag_id, initial_frag_id);
 
-	err = set_config(parms->config, tnl_ops, NULL, tnl_vport->mutable);
+	err = set_config(parms->config, tnl_ops, NULL, mutable);
 	if (err)
 		goto error_free_mutable;
 
@@ -1406,9 +1407,11 @@ struct vport *tnl_create(const struct vport_parms *parms,
 
 #ifdef NEED_CACHE_TIMEOUT
 	tnl_vport->cache_exp_interval = MAX_CACHE_EXP -
-					(net_random() % (MAX_CACHE_EXP / 2));
+				       (net_random() % (MAX_CACHE_EXP / 2));
 #endif
 
+	rcu_assign_pointer(tnl_vport->mutable, mutable);
+
 	err = add_port(vport);
 	if (err)
 		goto error_free_mutable;
@@ -1416,7 +1419,7 @@ struct vport *tnl_create(const struct vport_parms *parms,
 	return vport;
 
 error_free_mutable:
-	kfree(tnl_vport->mutable);
+	kfree(mutable);
 error_free_vport:
 	vport_free(vport);
 error:
diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c
index 67b68a3..8a3b465 100644
--- a/datapath/vport-patch.c
+++ b/datapath/vport-patch.c
@@ -106,6 +106,7 @@ static struct vport *patch_create(const struct vport_parms *parms)
 	struct vport *vport;
 	struct patch_vport *patch_vport;
 	const char *peer_name;
+	struct device_config *devconf;
 	int err;
 
 	vport = vport_alloc(sizeof(struct patch_vport), &patch_vport_ops, parms);
@@ -118,32 +119,33 @@ static struct vport *patch_create(const struct vport_parms *parms)
 
 	strcpy(patch_vport->name, parms->name);
 
-	patch_vport->devconf = kmalloc(sizeof(struct device_config), GFP_KERNEL);
-	if (!patch_vport->devconf) {
+	devconf = kmalloc(sizeof(struct device_config), GFP_KERNEL);
+	if (!devconf) {
 		err = -ENOMEM;
 		goto error_free_vport;
 	}
 
-	err = set_config(vport, parms->config, patch_vport->devconf);
+	err = set_config(vport, parms->config, devconf);
 	if (err)
 		goto error_free_devconf;
 
-	vport_gen_rand_ether_addr(patch_vport->devconf->eth_addr);
+	vport_gen_rand_ether_addr(devconf->eth_addr);
 
 	/* Make the default MTU fairly large so that it doesn't become the
 	 * bottleneck on systems using jumbo frames. */
-	patch_vport->devconf->mtu = 65535;
+	devconf->mtu = 65535;
 
-	peer_name = patch_vport->devconf->peer_name;
-	hlist_add_head(&patch_vport->hash_node, hash_bucket(peer_name));
+	rcu_assign_pointer(patch_vport->devconf, devconf);
 
+	peer_name = devconf->peer_name;
+	hlist_add_head(&patch_vport->hash_node, hash_bucket(peer_name));
 	rcu_assign_pointer(patch_vport->peer, vport_locate(peer_name));
 	update_peers(patch_vport->name, vport);
 
 	return vport;
 
 error_free_devconf:
-	kfree(patch_vport->devconf);
+	kfree(devconf);
 error_free_vport:
 	vport_free(vport);
 error:
-- 
1.7.1