Skip to contentSkip to conversation

Clean up owner field in sock_lock_t

PATCH In review John Heffner 2 participants 4 messages Sep 11, 2007 – Sep 12, 2007

Subsystems (5)
Networking Network drivers NFS client NFS server +1 more subsystems
lore
Share & tools
lore

John Heffner

author
#0 lore raw

cover Clean up owner field in sock_lock_t

I don't know why the owner field is a (struct sock_iocb *). I'm assuming it's historical. Can someone check this out? Did I miss some alternate usage?

These patches are against net-2.6.24.

author
#1 lore raw

PATCH 1/2 Cleanup: Use sock_owned_by_user() macro

Changes asserts in sunrpc to use sock_owned_by_user() macro instead of referencing sock_lock.owner directly.

Signed-off-by John Heffner <[email protected]>
---
 net/sunrpc/svcsock.c  |    2 +-
 net/sunrpc/xprtsock.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
net/sunrpc/svcsock.c +1 -1
index ed17a50..3a95612 100644
@@ -104,7 +104,7 @@ static struct lock_class_key svc_slock_key[2];
 static inline void svc_reclassify_socket(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
-	BUG_ON(sk->sk_lock.owner != NULL);
+	BUG_ON(sock_owned_by_user(sk));
 	switch (sk->sk_family) {
 	case AF_INET:
 		sock_lock_init_class_and_name(sk, "slock-AF_INET-NFSD",
net/sunrpc/xprtsock.c +1 -1
index 4ae7eed..282efd4 100644
@@ -1186,7 +1186,7 @@ static struct lock_class_key xs_slock_key[2];
 static inline void xs_reclassify_socket(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
-	BUG_ON(sk->sk_lock.owner != NULL);
+	BUG_ON(sock_owned_by_user(sk));
 	switch (sk->sk_family) {
 	case AF_INET:
 		sock_lock_init_class_and_name(sk, "slock-AF_INET-NFS",
Signature · 1 line
1.5.3.rc7.30.g947ad2
author
#2 lore raw

PATCH 2/2 Change type of owner in sock_lock_t to int, rename

The type of owner in sock_lock_t is currently (struct sock_iocb *), presumably for historical reasons. It is never used as this type, only tested as NULL or set to (void *)1. For clarity, this changes it to type int, and renames to owned, to avoid any possible type casting errors.

Signed-off-by John Heffner <[email protected]>
---
 include/net/sock.h |    7 +++----
 net/core/sock.c    |    6 +++---
 2 files changed, 6 insertions(+), 7 deletions(-)
include/net/sock.h +3 -4
index 802c670..5ed9fa4 100644
@@ -76,10 +76,9 @@
  * between user contexts and software interrupt processing, whereas the
  * mini-semaphore synchronizes multiple users amongst themselves.
  */
-struct sock_iocb;
 typedef struct {
 	spinlock_t		slock;
-	struct sock_iocb	*owner;
+	int			owned;
 	wait_queue_head_t	wq;
 	/*
 	 * We express the mutex-alike socket_lock semantics

@@ -737,7 +736,7 @@ static inline int sk_stream_wmem_schedule(struct sock *sk, int size)
  * Since ~2.3.5 it is also exclusive sleep lock serializing
  * accesses from user process context.
  */
-#define sock_owned_by_user(sk)	((sk)->sk_lock.owner)
+#define sock_owned_by_user(sk)	((sk)->sk_lock.owned)
 
 /*
  * Macro so as to not evaluate some arguments when

@@ -748,7 +747,7 @@ static inline int sk_stream_wmem_schedule(struct sock *sk, int size)
  */
 #define sock_lock_init_class_and_name(sk, sname, skey, name, key) 	\
 do {									\
-	sk->sk_lock.owner = NULL;					\
+	sk->sk_lock.owned = 0;					\
 	init_waitqueue_head(&sk->sk_lock.wq);				\
 	spin_lock_init(&(sk)->sk_lock.slock);				\
 	debug_check_no_locks_freed((void *)&(sk)->sk_lock,		\
net/core/sock.c +3 -3
index cfed7d4..edbc562 100644
@@ -1575,9 +1575,9 @@ void fastcall lock_sock_nested(struct sock *sk, int subclass)
 {
 	might_sleep();
 	spin_lock_bh(&sk->sk_lock.slock);
-	if (sk->sk_lock.owner)
+	if (sk->sk_lock.owned)
 		__lock_sock(sk);
-	sk->sk_lock.owner = (void *)1;
+	sk->sk_lock.owned = 1;
 	spin_unlock(&sk->sk_lock.slock);
 	/*
 	 * The sk_lock has mutex_lock() semantics here:

@@ -1598,7 +1598,7 @@ void fastcall release_sock(struct sock *sk)
 	spin_lock_bh(&sk->sk_lock.slock);
 	if (sk->sk_backlog.tail)
 		__release_sock(sk);
-	sk->sk_lock.owner = NULL;
+	sk->sk_lock.owned = 0;
 	if (waitqueue_active(&sk->sk_lock.wq))
 		wake_up(&sk->sk_lock.wq);
 	spin_unlock_bh(&sk->sk_lock.slock);
Signature · 1 line
1.5.3.rc7.30.g947ad2
maintainer
#3 lore raw
In reply to John Heffner

From: John Heffner <[email protected]>
Date: Tue, 11 Sep 2007 14:01:31 -0400

Quoted 3 lines

I don't know why the owner field is a (struct sock_iocb *). I'm assuming it's historical. Can someone check this out? Did I miss some alternate usage?

AIO used it somehow in net/socket.c and I believe there was some intention to access this sock_iocb deeper in the call chain.

None of that materialized of course :)

Quoted 1 line

These patches are against net-2.6.24.

Thanks a lot, I'll add these patches.