[patch] Use rwsems instead of custom locking scheme in net/socket.c and net/dccp/ccid.c

[Frederik Deweerdt]

On Sun, Aug 06, 2006 at 03:08:09AM -0700, Andrew Morton wrote:
> 
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc3/2.6.18-rc3-mm2/
> 
Hi Andrew,

This patch aims at removing two implementations (spotted by Masatake YAMATO) of
pseudo-rwlocks using a spinlock_t and an atomic_t. One in net/socket.c
and another in net/bluetooth/af_bluetooth.c. I think that both could be
converted to rwsems, saving some lines of code.

Regards,
Frederik


Signed-off-by: Frederik Deweerdt <frederik.deweerdt@xxxxxxxxx>

 net/dccp/ccid.c |   63 ++++++++++++------------------------------------------------
 net/socket.c    |   58 +++++++------------------------------------------------
 2 files changed, 21 insertions(+), 100 deletions(-)

diff --git a/net/dccp/ccid.c b/net/dccp/ccid.c
--- a/net/dccp/ccid.c
+++ b/net/dccp/ccid.c
@@ -12,48 +12,11 @@
  */
 
 #include "ccid.h"
+#include <linux/rwsem.h>
 
 static struct ccid_operations *ccids[CCID_MAX];
-#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
-static atomic_t ccids_lockct = ATOMIC_INIT(0);
-static DEFINE_SPINLOCK(ccids_lock);
+static DECLARE_RWSEM(ccids_sem);
 
-/*
- * The strategy is: modifications ccids vector are short, do not sleep and
- * veeery rare, but read access should be free of any exclusive locks.
- */
-static void ccids_write_lock(void)
-{
-	spin_lock(&ccids_lock);
-	while (atomic_read(&ccids_lockct) != 0) {
-		spin_unlock(&ccids_lock);
-		yield();
-		spin_lock(&ccids_lock);
-	}
-}
-
-static inline void ccids_write_unlock(void)
-{
-	spin_unlock(&ccids_lock);
-}
-
-static inline void ccids_read_lock(void)
-{
-	atomic_inc(&ccids_lockct);
-	spin_unlock_wait(&ccids_lock);
-}
-
-static inline void ccids_read_unlock(void)
-{
-	atomic_dec(&ccids_lockct);
-}
-
-#else
-#define ccids_write_lock() do { } while(0)
-#define ccids_write_unlock() do { } while(0)
-#define ccids_read_lock() do { } while(0)
-#define ccids_read_unlock() do { } while(0)
-#endif
 
 static kmem_cache_t *ccid_kmem_cache_create(int obj_size, const char *fmt,...)
 {
@@ -103,13 +66,13 @@ int ccid_register(struct ccid_operations
 	if (ccid_ops->ccid_hc_tx_slab == NULL)
 		goto out_free_rx_slab;
 
-	ccids_write_lock();
+	down_write(&ccids_sem);
 	err = -EEXIST;
 	if (ccids[ccid_ops->ccid_id] == NULL) {
 		ccids[ccid_ops->ccid_id] = ccid_ops;
 		err = 0;
 	}
-	ccids_write_unlock();
+	up_write(&ccids_sem);
 	if (err != 0)
 		goto out_free_tx_slab;
 
@@ -131,9 +94,9 @@ EXPORT_SYMBOL_GPL(ccid_register);
 
 int ccid_unregister(struct ccid_operations *ccid_ops)
 {
-	ccids_write_lock();
+	down_write(&ccids_sem);
 	ccids[ccid_ops->ccid_id] = NULL;
-	ccids_write_unlock();
+	up_write(&ccids_sem);
 
 	ccid_kmem_cache_destroy(ccid_ops->ccid_hc_tx_slab);
 	ccid_ops->ccid_hc_tx_slab = NULL;
@@ -152,15 +115,15 @@ struct ccid *ccid_new(unsigned char id, 
 	struct ccid_operations *ccid_ops;
 	struct ccid *ccid = NULL;
 
-	ccids_read_lock();
+	down_read(&ccids_sem);
 #ifdef CONFIG_KMOD
 	if (ccids[id] == NULL) {
 		/* We only try to load if in process context */
-		ccids_read_unlock();
+		up_read(&ccids_sem);
 		if (gfp & GFP_ATOMIC)
 			goto out;
 		request_module("net-dccp-ccid-%d", id);
-		ccids_read_lock();
+		down_read(&ccids_sem);
 	}
 #endif
 	ccid_ops = ccids[id];
@@ -170,7 +133,7 @@ #endif
 	if (!try_module_get(ccid_ops->ccid_owner))
 		goto out_unlock;
 
-	ccids_read_unlock();
+	up_read(&ccids_sem);
 
 	ccid = kmem_cache_alloc(rx ? ccid_ops->ccid_hc_rx_slab :
 				     ccid_ops->ccid_hc_tx_slab, gfp);
@@ -191,7 +154,7 @@ #endif
 out:
 	return ccid;
 out_unlock:
-	ccids_read_unlock();
+	up_read(&ccids_sem);
 	goto out;
 out_free_ccid:
 	kmem_cache_free(rx ? ccid_ops->ccid_hc_rx_slab :
@@ -235,10 +198,10 @@ static void ccid_delete(struct ccid *cci
 			ccid_ops->ccid_hc_tx_exit(sk);
 		kmem_cache_free(ccid_ops->ccid_hc_tx_slab,  ccid);
 	}
-	ccids_read_lock();
+	down_read(&ccids_sem);
 	if (ccids[ccid_ops->ccid_id] != NULL)
 		module_put(ccid_ops->ccid_owner);
-	ccids_read_unlock();
+	up_read(&ccids_sem);
 }
 
 void ccid_hc_rx_delete(struct ccid *ccid, struct sock *sk)
diff --git a/net/socket.c b/net/socket.c
index 53cb85b..bc52aeb 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -85,6 +85,7 @@ #include <linux/compat.h>
 #include <linux/kmod.h>
 #include <linux/audit.h>
 #include <linux/wireless.h>
+#include <linux/rwsem.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -143,50 +144,7 @@ #endif
 
 static struct net_proto_family *net_families[NPROTO];
 
-#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
-static atomic_t net_family_lockct = ATOMIC_INIT(0);
-static DEFINE_SPINLOCK(net_family_lock);
-
-/* The strategy is: modifications net_family vector are short, do not
-   sleep and veeery rare, but read access should be free of any exclusive
-   locks.
- */
-
-static void net_family_write_lock(void)
-{
-	spin_lock(&net_family_lock);
-	while (atomic_read(&net_family_lockct) != 0) {
-		spin_unlock(&net_family_lock);
-
-		yield();
-
-		spin_lock(&net_family_lock);
-	}
-}
-
-static __inline__ void net_family_write_unlock(void)
-{
-	spin_unlock(&net_family_lock);
-}
-
-static __inline__ void net_family_read_lock(void)
-{
-	atomic_inc(&net_family_lockct);
-	spin_unlock_wait(&net_family_lock);
-}
-
-static __inline__ void net_family_read_unlock(void)
-{
-	atomic_dec(&net_family_lockct);
-}
-
-#else
-#define net_family_write_lock() do { } while(0)
-#define net_family_write_unlock() do { } while(0)
-#define net_family_read_lock() do { } while(0)
-#define net_family_read_unlock() do { } while(0)
-#endif
-
+static DECLARE_RWSEM(net_family_sem);
 
 /*
  *	Statistics counters of the socket lists
@@ -1132,7 +1090,7 @@ #if defined(CONFIG_KMOD)
 	}
 #endif
 
-	net_family_read_lock();
+	down_read(&net_family_sem);
 	if (net_families[family] == NULL) {
 		err = -EAFNOSUPPORT;
 		goto out;
@@ -1185,7 +1143,7 @@ #endif
 		goto out_release;
 
 out:
-	net_family_read_unlock();
+	up_read(&net_family_sem);
 	return err;
 out_module_put:
 	module_put(net_families[family]->owner);
@@ -2034,13 +1992,13 @@ int sock_register(struct net_proto_famil
 		printk(KERN_CRIT "protocol %d >= NPROTO(%d)\n", ops->family, NPROTO);
 		return -ENOBUFS;
 	}
-	net_family_write_lock();
+	down_write(&net_family_sem);
 	err = -EEXIST;
 	if (net_families[ops->family] == NULL) {
 		net_families[ops->family]=ops;
 		err = 0;
 	}
-	net_family_write_unlock();
+	up_write(&net_family_sem);
 	printk(KERN_INFO "NET: Registered protocol family %d\n",
 	       ops->family);
 	return err;
@@ -2057,9 +2015,9 @@ int sock_unregister(int family)
 	if (family < 0 || family >= NPROTO)
 		return -1;
 
-	net_family_write_lock();
+	down_write(&net_family_sem);
 	net_families[family]=NULL;
-	net_family_write_unlock();
+	up_write(&net_family_sem);
 	printk(KERN_INFO "NET: Unregistered protocol family %d\n",
 	       family);
 	return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/