* [patch 1/1] Convert the semaphore to a mutex in net/tipc/socket.c
[not found] <20071210011741.039692330@gmail.com>
@ 2007-12-10 1:17 ` Kevin Winchester
2007-12-12 0:44 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Kevin Winchester @ 2007-12-10 1:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Ingo Molnar, Per Liden, Jon Maloy, Allan Stephens,
tipc-discussion, linux-kernel, Kevin Winchester
[-- Attachment #1: tipc-socket-sem-to-mutex.patch --]
[-- Type: text/plain, Size: 6700 bytes --]
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Per Liden <per.liden@ericsson.com>
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Allan Stephens <allan.stephens@windriver.com>
Cc: tipc-discussion@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Note also that in the release method, down_interruptible() was being called
without checking the return value. I converted it to mutex_lock_interruptible()
and made the interrupted case return -ERESTARTSYS, as was done for all other
calls to down_interruptible() in the file.
Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
net/tipc/socket.c | 56 +++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 28 deletions(-)
Index: v2.6.24-rc4/net/tipc/socket.c
===================================================================
--- v2.6.24-rc4.orig/net/tipc/socket.c
+++ v2.6.24-rc4/net/tipc/socket.c
@@ -63,7 +63,7 @@
struct tipc_sock {
struct sock sk;
struct tipc_port *p;
- struct semaphore sem;
+ struct mutex lock;
};
#define tipc_sk(sk) ((struct tipc_sock*)sk)
@@ -217,7 +217,7 @@ static int tipc_create(struct net *net,
tsock->p = port;
port->usr_handle = tsock;
- init_MUTEX(&tsock->sem);
+ mutex_init(&tsock->lock);
dbg("sock_create: %x\n",tsock);
@@ -253,9 +253,10 @@ static int release(struct socket *sock)
dbg("sock_delete: %x\n",tsock);
if (!tsock)
return 0;
- down_interruptible(&tsock->sem);
+ if (mutex_lock_interruptible(&tsock->lock))
+ return -ERESTARTSYS;
if (!sock->sk) {
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return 0;
}
@@ -288,7 +289,7 @@ static int release(struct socket *sock)
atomic_dec(&tipc_queue_size);
}
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
sock_put(sk);
@@ -315,7 +316,7 @@ static int bind(struct socket *sock, str
struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr;
int res;
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
if (unlikely(!uaddr_len)) {
@@ -346,7 +347,7 @@ static int bind(struct socket *sock, str
res = tipc_withdraw(tsock->p->ref, -addr->scope,
&addr->addr.nameseq);
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -367,7 +368,7 @@ static int get_name(struct socket *sock,
struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr;
u32 res;
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
*uaddr_len = sizeof(*addr);
@@ -380,7 +381,7 @@ static int get_name(struct socket *sock,
res = tipc_ownidentity(tsock->p->ref, &addr->addr.id);
addr->addr.name.domain = 0;
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -477,7 +478,7 @@ static int send_msg(struct kiocb *iocb,
}
}
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
if (needs_conn) {
@@ -523,7 +524,7 @@ static int send_msg(struct kiocb *iocb,
}
if (likely(res != -ELINKCONG)) {
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
if (m->msg_flags & MSG_DONTWAIT) {
@@ -562,9 +563,8 @@ static int send_packet(struct kiocb *ioc
if (unlikely(dest))
return send_msg(iocb, sock, m, total_len);
- if (down_interruptible(&tsock->sem)) {
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
- }
do {
if (unlikely(sock->state != SS_CONNECTED)) {
@@ -578,7 +578,7 @@ static int send_packet(struct kiocb *ioc
res = tipc_send(tsock->p->ref, m->msg_iovlen, m->msg_iov);
if (likely(res != -ELINKCONG)) {
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
if (m->msg_flags & MSG_DONTWAIT) {
@@ -846,7 +846,7 @@ static int recv_msg(struct kiocb *iocb,
/* Look for a message in receive queue; wait if necessary */
- if (unlikely(down_interruptible(&tsock->sem)))
+ if (unlikely(mutex_lock_interruptible(&tsock->lock)))
return -ERESTARTSYS;
restart:
@@ -930,7 +930,7 @@ restart:
advance_queue(tsock);
}
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -981,7 +981,7 @@ static int recv_stream(struct kiocb *ioc
/* Look for a message in receive queue; wait if necessary */
- if (unlikely(down_interruptible(&tsock->sem)))
+ if (unlikely(mutex_lock_interruptible(&tsock->lock)))
return -ERESTARTSYS;
restart:
@@ -1077,7 +1077,7 @@ restart:
goto restart;
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return sz_copied ? sz_copied : res;
}
@@ -1293,7 +1293,7 @@ static int connect(struct socket *sock,
return res;
}
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
/* Wait for destination's 'ACK' response */
@@ -1317,7 +1317,7 @@ static int connect(struct socket *sock,
sock->state = SS_DISCONNECTING;
}
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -1365,7 +1365,7 @@ static int accept(struct socket *sock, s
(flags & O_NONBLOCK)))
return -EWOULDBLOCK;
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
if (wait_event_interruptible(*sock->sk->sk_sleep,
@@ -1412,7 +1412,7 @@ static int accept(struct socket *sock, s
}
}
exit:
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -1434,7 +1434,7 @@ static int shutdown(struct socket *sock,
/* Could return -EINVAL for an invalid "how", but why bother? */
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
sock_lock(tsock);
@@ -1484,7 +1484,7 @@ restart:
sock_unlock(tsock);
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -1518,7 +1518,7 @@ static int setsockopt(struct socket *soc
if ((res = get_user(value, (u32 __user *)ov)))
return res;
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
switch (opt) {
@@ -1541,7 +1541,7 @@ static int setsockopt(struct socket *soc
res = -EINVAL;
}
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
@@ -1574,7 +1574,7 @@ static int getsockopt(struct socket *soc
if ((res = get_user(len, ol)))
return res;
- if (down_interruptible(&tsock->sem))
+ if (mutex_lock_interruptible(&tsock->lock))
return -ERESTARTSYS;
switch (opt) {
@@ -1607,7 +1607,7 @@ static int getsockopt(struct socket *soc
res = put_user(sizeof(value), ol);
}
- up(&tsock->sem);
+ mutex_unlock(&tsock->lock);
return res;
}
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/1] Convert the semaphore to a mutex in net/tipc/socket.c
2007-12-10 1:17 ` [patch 1/1] Convert the semaphore to a mutex in net/tipc/socket.c Kevin Winchester
@ 2007-12-12 0:44 ` Andrew Morton
2007-12-12 19:24 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2007-12-12 0:44 UTC (permalink / raw)
To: Kevin Winchester
Cc: mingo, per.liden, jon.maloy, allan.stephens, tipc-discussion,
linux-kernel, kjwinchester, netdev, David S. Miller
On Sun, 09 Dec 2007 21:17:42 -0400
Kevin Winchester <kjwinchester@gmail.com> wrote:
> Note also that in the release method, down_interruptible() was being called
> without checking the return value. I converted it to mutex_lock_interruptible()
> and made the interrupted case return -ERESTARTSYS, as was done for all other
> calls to down_interruptible() in the file.
That's an outright bug.
static int release(struct socket *sock)
{
struct tipc_sock *tsock = tipc_sk(sock->sk);
struct sock *sk = sock->sk;
int res = TIPC_OK;
struct sk_buff *buf;
dbg("sock_delete: %x\n",tsock);
if (!tsock)
return 0;
down_interruptible(&tsock->sem);
if (!sock->sk) {
up(&tsock->sem);
return 0;
}
...
up(&tsock->sem);
...
}
So if the calling process has signal_pending(), down_interruptible() will
return without having downed the semaphore and then we merrily proceed to
do up() on it, so a subsequent down() won't actually take the lock and
things will deteriorate from there.
So I'd propose this:
--- a/net/tipc/socket.c~a
+++ a/net/tipc/socket.c
@@ -253,7 +253,7 @@ static int release(struct socket *sock)
dbg("sock_delete: %x\n",tsock);
if (!tsock)
return 0;
- down_interruptible(&tsock->sem);
+ down(&tsock->sem);
if (!sock->sk) {
up(&tsock->sem);
return 0;
_
as a for-2.6.24 bugfix. And for 2.6.23. But someone who knows what
they're doing should take a look at this...
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/1] Convert the semaphore to a mutex in net/tipc/socket.c
2007-12-12 0:44 ` Andrew Morton
@ 2007-12-12 19:24 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2007-12-12 19:24 UTC (permalink / raw)
To: akpm
Cc: kjwinchester, mingo, per.liden, jon.maloy, allan.stephens,
tipc-discussion, linux-kernel, netdev
From: Andrew Morton <akpm@linux-foundation.org>
Date: Tue, 11 Dec 2007 16:44:45 -0800
> So I'd propose this:
I've applied this to net-2.6, thanks Andrew.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-12-12 19:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20071210011741.039692330@gmail.com>
2007-12-10 1:17 ` [patch 1/1] Convert the semaphore to a mutex in net/tipc/socket.c Kevin Winchester
2007-12-12 0:44 ` Andrew Morton
2007-12-12 19:24 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).