[openib-general] [PATCH applied] sdp_link locking

[Michael S. Tsirkin]

The following patch adds locking to sdp_link.
It does not yet include request cancellation,
and the workqueue handling code also seems buggy
(cancels must flush workqueue, and ideally we wouldnt need
a separate workqueue at all).

---

Add locking to sdp link code.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.c
===================================================================
--- linux-2.6.12.2.orig/drivers/infiniband/ulp/sdp/sdp_link.c	2005-09-01 17:35:49.000000000 +0300
+++ linux-2.6.12.2/drivers/infiniband/ulp/sdp/sdp_link.c	2005-09-01 17:38:44.000000000 +0300
@@ -128,6 +128,7 @@ struct sdp_link_arp {
 static kmem_cache_t *wait_cache;
 static kmem_cache_t *info_cache;
 
+static DECLARE_MUTEX(sdp_link_mutex);
 static LIST_HEAD(info_list);
 
 static struct workqueue_struct *link_wq;
@@ -150,7 +151,7 @@ static u64 path_lookup_id;
 /*
  * proto
  */
-static void do_link_path_lookup(void *data);
+static void retry_link_path_lookup(void *);
 
 /*
  * sdp_link_path_complete - generate a path record completion for user
@@ -219,8 +220,8 @@ static struct sdp_path_info *sdp_path_in
 	info->arp_time = SDP_LINK_ARP_TIME_MIN;
 
 	INIT_LIST_HEAD(&info->wait_list);
+	INIT_WORK(&info->timer, retry_link_path_lookup, info);
 	list_add(&info->info_list, &info_list);
-	INIT_WORK(&info->timer, do_link_path_lookup, info);
 
 	return info;
 }
@@ -252,6 +253,8 @@ static void sdp_link_path_rec_done(int s
 	struct sdp_path_wait *sweep;
 	int result;
 
+	down(&sdp_link_mutex);
+
 	info->query = NULL;
 
 	sdp_dbg_data(NULL, "Path Record status <%d>", status);
@@ -301,6 +304,7 @@ static void sdp_link_path_rec_done(int s
 			sdp_path_info_destroy(info, result);
 		}
 	}
+	up(&sdp_link_mutex);
 }
 
 /*
@@ -345,9 +349,8 @@ static int sdp_link_path_rec_get(struct 
 /*
  * do_link_path_lookup - resolve an ip address to a path record
  */
-static void do_link_path_lookup(void *data)
+static void do_link_path_lookup(struct sdp_path_info *info)
 {
-	struct sdp_path_info *info = data;
 	struct ipoib_dev_priv *priv;
 	struct net_device *dev = NULL;
 	struct rtable *rt;
@@ -376,11 +379,6 @@ static void do_link_path_lookup(void *da
 	 */
 	if (info->flags & SDP_LINK_F_PATH)
 		goto done;
-	/*
-	 * route information present, but no path query.
-	 */
-	if (info->ca)
-		goto path;
 
 	result = ip_route_output_key(&rt, &fl);
 	if (result < 0 || !rt) {
@@ -543,6 +541,13 @@ error:
 	ip_rt_put(rt);
 }
 
+static void retry_link_path_lookup(void *data)
+{
+	down(&sdp_link_mutex);
+	do_link_path_lookup(data);
+	up(&sdp_link_mutex);
+}
+
 /*
  * Public functions
  */
@@ -566,7 +571,9 @@ int sdp_link_path_lookup(u32 dst_addr,  
 {
 	struct sdp_path_info *info;
 	struct sdp_path_wait *wait;
-	int result;
+	int result = 0;
+
+	down(&sdp_link_mutex);
 
 	*id = _SDP_PATH_LOOKUP_ID();
 
@@ -589,7 +596,7 @@ int sdp_link_path_lookup(u32 dst_addr,  
 	 */
 	if (info->flags & SDP_LINK_F_VALID) {
 		sdp_link_path_complete(*id, 0, info, completion, arg);
-		return 0;
+		goto done;
 	}
 	/*
 	 * add request to list of lookups.
@@ -598,7 +605,7 @@ int sdp_link_path_lookup(u32 dst_addr,  
 	if (!wait) {
 		sdp_dbg_warn(NULL, "Failed to create path wait object");
 		result = -ENOMEM;
-		goto error;
+		goto done;
 	}
 
 	wait->id = *id;
@@ -613,8 +620,8 @@ int sdp_link_path_lookup(u32 dst_addr,  
 	if (!((SDP_LINK_F_ARP|SDP_LINK_F_PATH) & info->flags))
 		do_link_path_lookup(info);
 
-	return 0;
-error:
+done:
+	up(&sdp_link_mutex);
 	return result;
 }
 
@@ -630,6 +637,7 @@ static void sdp_link_sweep(void *data)
 	struct sdp_path_info *info;
 	struct sdp_path_info *sweep;
 
+	down(&sdp_link_mutex);
 	list_for_each_entry_safe(info, sweep, &info_list, info_list) {
 		if (jiffies > (info->use + SDP_LINK_INFO_TIMEOUT)) {
 			sdp_dbg_ctrl(NULL,
@@ -643,6 +651,7 @@ static void sdp_link_sweep(void *data)
 			sdp_path_info_destroy(info, -ETIMEDOUT);
 		}
 	}
+	up(&sdp_link_mutex);
 
 	queue_delayed_work(link_wq, &link_timer, SDP_LINK_SWEEP_INTERVAL);
 }
@@ -656,8 +665,8 @@ static void sdp_link_sweep(void *data)
  */
 static void sdp_link_arp_work(void *data)
 {
-	struct sdp_work *work = (struct sdp_work *)data;
-	struct sk_buff *skb = (struct sk_buff *)work->arg;
+	struct sdp_work *work = data;
+	struct sk_buff *skb = work->arg;
 	struct sdp_path_info *info;
 	struct sdp_link_arp *arp;
 	int result;
@@ -670,6 +679,8 @@ static void sdp_link_arp_work(void *data
 		     (arp->src_ip & 0x00ff0000) >> 16,
 		     (arp->src_ip & 0xff000000) >> 24,
 		     GID_ARG(arp->src_gid));
+
+	down(&sdp_link_mutex);
 	/*
 	 * find a path info structure for the source IP address.
 	 */
@@ -696,6 +707,7 @@ static void sdp_link_arp_work(void *data
 	}
 
 done:
+	up(&sdp_link_mutex);
 	kfree_skb(skb);
 	kfree(work);
 }
@@ -811,6 +823,7 @@ void sdp_link_addr_cleanup(void)
 	 * remove ARP packet processing.
 	 */
 	dev_remove_pack(&sdp_arp_type);
+	
 	/*
 	 * destroy work queue
 	 */

-- 
MST