From 77b2283604bdd7053494a97b0e2fee97148206c6 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sat, 22 Jan 2011 23:44:59 +0100 Subject: x25: remove the BKL This replaces all instances of lock_kernel in x25 with lock_sock, taking care to release the socket lock around sleeping functions (sock_alloc_send_skb and skb_recv_datagram). It is not clear whether this is a correct solution, but it seem to be what other protocols do in the same situation. Includes a fix suggested by Eric Dumazet. Signed-off-by: Arnd Bergmann Acked-by: David S. Miller Tested-by: Andrew Hendry Cc: linux-x25@vger.kernel.org Cc: netdev@vger.kernel.org Cc: Eric Dumazet --- net/x25/Kconfig | 1 - net/x25/af_x25.c | 58 +++++++++++++++++-------------------------------------- net/x25/x25_out.c | 7 +++++-- 3 files changed, 23 insertions(+), 43 deletions(-) (limited to 'net') diff --git a/net/x25/Kconfig b/net/x25/Kconfig index 2196e55e4f61..e6759c9660bb 100644 --- a/net/x25/Kconfig +++ b/net/x25/Kconfig @@ -5,7 +5,6 @@ config X25 tristate "CCITT X.25 Packet Layer (EXPERIMENTAL)" depends on EXPERIMENTAL - depends on BKL # should be fixable ---help--- X.25 is a set of standardized network protocols, similar in scope to frame relay; the one physical line from your box to the X.25 network diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c index ad96ee90fe27..4680b1e4c79c 100644 --- a/net/x25/af_x25.c +++ b/net/x25/af_x25.c @@ -40,7 +40,6 @@ #include #include #include -#include #include #include #include @@ -432,15 +431,6 @@ void x25_destroy_socket_from_timer(struct sock *sk) sock_put(sk); } -static void x25_destroy_socket(struct sock *sk) -{ - sock_hold(sk); - lock_sock(sk); - __x25_destroy_socket(sk); - release_sock(sk); - sock_put(sk); -} - /* * Handling for system calls applied via the various interfaces to a * X.25 socket object. @@ -647,18 +637,19 @@ static int x25_release(struct socket *sock) struct sock *sk = sock->sk; struct x25_sock *x25; - lock_kernel(); if (!sk) - goto out; + return 0; x25 = x25_sk(sk); + sock_hold(sk); + lock_sock(sk); switch (x25->state) { case X25_STATE_0: case X25_STATE_2: x25_disconnect(sk, 0, 0, 0); - x25_destroy_socket(sk); + __x25_destroy_socket(sk); goto out; case X25_STATE_1: @@ -678,7 +669,8 @@ static int x25_release(struct socket *sock) sock_orphan(sk); out: - unlock_kernel(); + release_sock(sk); + sock_put(sk); return 0; } @@ -1085,7 +1077,7 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock, size_t size; int qbit = 0, rc = -EINVAL; - lock_kernel(); + lock_sock(sk); if (msg->msg_flags & ~(MSG_DONTWAIT|MSG_OOB|MSG_EOR|MSG_CMSG_COMPAT)) goto out; @@ -1148,7 +1140,9 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock, size = len + X25_MAX_L2_LEN + X25_EXT_MIN_LEN; + release_sock(sk); skb = sock_alloc_send_skb(sk, size, noblock, &rc); + lock_sock(sk); if (!skb) goto out; X25_SKB_CB(skb)->flags = msg->msg_flags; @@ -1231,26 +1225,10 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock, len++; } - /* - * lock_sock() is currently only used to serialize this x25_kick() - * against input-driven x25_kick() calls. It currently only blocks - * incoming packets for this socket and does not protect against - * any other socket state changes and is not called from anywhere - * else. As x25_kick() cannot block and as long as all socket - * operations are BKL-wrapped, we don't need take to care about - * purging the backlog queue in x25_release(). - * - * Using lock_sock() to protect all socket operations entirely - * (and making the whole x25 stack SMP aware) unfortunately would - * require major changes to {send,recv}msg and skb allocation methods. - * -> 2.5 ;) - */ - lock_sock(sk); x25_kick(sk); - release_sock(sk); rc = len; out: - unlock_kernel(); + release_sock(sk); return rc; out_kfree_skb: kfree_skb(skb); @@ -1271,7 +1249,7 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, unsigned char *asmptr; int rc = -ENOTCONN; - lock_kernel(); + lock_sock(sk); /* * This works for seqpacket too. The receiver has ordered the queue for * us! We do one quick check first though @@ -1300,8 +1278,10 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, msg->msg_flags |= MSG_OOB; } else { /* Now we can treat all alike */ + release_sock(sk); skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT, flags & MSG_DONTWAIT, &rc); + lock_sock(sk); if (!skb) goto out; @@ -1338,14 +1318,12 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, msg->msg_namelen = sizeof(struct sockaddr_x25); - lock_sock(sk); x25_check_rbuf(sk); - release_sock(sk); rc = copied; out_free_dgram: skb_free_datagram(sk, skb); out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1581,18 +1559,18 @@ out_cud_release: case SIOCX25CALLACCPTAPPRV: { rc = -EINVAL; - lock_kernel(); + lock_sock(sk); if (sk->sk_state != TCP_CLOSE) break; clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags); - unlock_kernel(); + release_sock(sk); rc = 0; break; } case SIOCX25SENDCALLACCPT: { rc = -EINVAL; - lock_kernel(); + lock_sock(sk); if (sk->sk_state != TCP_ESTABLISHED) break; /* must call accptapprv above */ @@ -1600,7 +1578,7 @@ out_cud_release: break; x25_write_internal(sk, X25_CALL_ACCEPTED); x25->state = X25_STATE_3; - unlock_kernel(); + release_sock(sk); rc = 0; break; } diff --git a/net/x25/x25_out.c b/net/x25/x25_out.c index d00649fb251d..0144271d2184 100644 --- a/net/x25/x25_out.c +++ b/net/x25/x25_out.c @@ -68,8 +68,11 @@ int x25_output(struct sock *sk, struct sk_buff *skb) frontlen = skb_headroom(skb); while (skb->len > 0) { - if ((skbn = sock_alloc_send_skb(sk, frontlen + max_len, - noblock, &err)) == NULL){ + release_sock(sk); + skbn = sock_alloc_send_skb(sk, frontlen + max_len, + noblock, &err); + lock_sock(sk); + if (!skbn) { if (err == -EWOULDBLOCK && noblock){ kfree_skb(skb); return sent; -- cgit v1.2.3 From 60d9f461a20ba59219fdcdc30cbf8e3a4ad3f625 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sun, 23 Jan 2011 00:21:11 +0100 Subject: appletalk: remove the BKL This changes appletalk to use lock_sock instead of lock_kernel for serialization. I tried to make sure that we don't hold the socket lock during sleeping functions, but I did not try to prove whether the locks are necessary in the first place. Compile-tested only. Signed-off-by: Arnd Bergmann Acked-by: David S. Miller Cc: Arnaldo Carvalho de Melo Cc: David Miller Cc: netdev@vger.kernel.org --- net/appletalk/ddp.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) (limited to 'net') diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c index c410b93fda2e..3d4f4b043406 100644 --- a/net/appletalk/ddp.c +++ b/net/appletalk/ddp.c @@ -54,7 +54,6 @@ #include #include #include -#include #include /* For TIOCOUTQ/INQ */ #include #include @@ -1052,13 +1051,13 @@ static int atalk_release(struct socket *sock) { struct sock *sk = sock->sk; - lock_kernel(); + lock_sock(sk); if (sk) { sock_orphan(sk); sock->sk = NULL; atalk_destroy_socket(sk); } - unlock_kernel(); + release_sock(sk); return 0; } @@ -1143,7 +1142,7 @@ static int atalk_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) if (addr->sat_family != AF_APPLETALK) return -EAFNOSUPPORT; - lock_kernel(); + lock_sock(sk); if (addr->sat_addr.s_net == htons(ATADDR_ANYNET)) { struct atalk_addr *ap = atalk_find_primary(); @@ -1179,7 +1178,7 @@ static int atalk_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) sock_reset_flag(sk, SOCK_ZAPPED); err = 0; out: - unlock_kernel(); + release_sock(sk); return err; } @@ -1215,7 +1214,7 @@ static int atalk_connect(struct socket *sock, struct sockaddr *uaddr, #endif } - lock_kernel(); + lock_sock(sk); err = -EBUSY; if (sock_flag(sk, SOCK_ZAPPED)) if (atalk_autobind(sk) < 0) @@ -1233,7 +1232,7 @@ static int atalk_connect(struct socket *sock, struct sockaddr *uaddr, sk->sk_state = TCP_ESTABLISHED; err = 0; out: - unlock_kernel(); + release_sock(sk); return err; } @@ -1249,7 +1248,7 @@ static int atalk_getname(struct socket *sock, struct sockaddr *uaddr, struct atalk_sock *at = at_sk(sk); int err; - lock_kernel(); + lock_sock(sk); err = -ENOBUFS; if (sock_flag(sk, SOCK_ZAPPED)) if (atalk_autobind(sk) < 0) @@ -1277,17 +1276,7 @@ static int atalk_getname(struct socket *sock, struct sockaddr *uaddr, memcpy(uaddr, &sat, sizeof(sat)); out: - unlock_kernel(); - return err; -} - -static unsigned int atalk_poll(struct file *file, struct socket *sock, - poll_table *wait) -{ - int err; - lock_kernel(); - err = datagram_poll(file, sock, wait); - unlock_kernel(); + release_sock(sk); return err; } @@ -1596,7 +1585,7 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr if (len > DDP_MAXSZ) return -EMSGSIZE; - lock_kernel(); + lock_sock(sk); if (usat) { err = -EBUSY; if (sock_flag(sk, SOCK_ZAPPED)) @@ -1651,7 +1640,9 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr sk, size, dev->name); size += dev->hard_header_len; + release_sock(sk); skb = sock_alloc_send_skb(sk, size, (flags & MSG_DONTWAIT), &err); + lock_sock(sk); if (!skb) goto out; @@ -1738,7 +1729,7 @@ static int atalk_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr SOCK_DEBUG(sk, "SK %p: Done write (%Zd).\n", sk, len); out: - unlock_kernel(); + release_sock(sk); return err ? : len; } @@ -1753,9 +1744,10 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr int err = 0; struct sk_buff *skb; - lock_kernel(); skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT, flags & MSG_DONTWAIT, &err); + lock_sock(sk); + if (!skb) goto out; @@ -1787,7 +1779,7 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr skb_free_datagram(sk, skb); /* Free the datagram. */ out: - unlock_kernel(); + release_sock(sk); return err ? : copied; } @@ -1887,7 +1879,7 @@ static const struct proto_ops atalk_dgram_ops = { .socketpair = sock_no_socketpair, .accept = sock_no_accept, .getname = atalk_getname, - .poll = atalk_poll, + .poll = datagram_poll, .ioctl = atalk_ioctl, #ifdef CONFIG_COMPAT .compat_ioctl = atalk_compat_ioctl, -- cgit v1.2.3 From b0d0d915d1d1a0fe486849f3e41333c66df620c4 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 25 Jan 2011 21:49:56 +0100 Subject: ipx: remove the BKL This replaces all instances of lock_kernel in the IPX code with lock_sock. As far as I can tell, this is safe to do, because there is no global state that needs to be locked in IPX, and the code does not recursively take the lock or sleep indefinitely while holding it. Compile-tested only. Signed-off-by: Arnd Bergmann Acked-by: David S. Miller Cc: Arnaldo Carvalho de Melo Cc: netdev@vger.kernel.org --- net/ipx/Kconfig | 1 - net/ipx/af_ipx.c | 52 ++++++++++++++++++++-------------------------------- 2 files changed, 20 insertions(+), 33 deletions(-) (limited to 'net') diff --git a/net/ipx/Kconfig b/net/ipx/Kconfig index 02549cb2c328..e9ad0062fbb6 100644 --- a/net/ipx/Kconfig +++ b/net/ipx/Kconfig @@ -3,7 +3,6 @@ # config IPX tristate "The IPX protocol" - depends on BKL # should be fixable select LLC ---help--- This is support for the Novell networking protocol, IPX, commonly diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c index da3d21c41d90..2731b51923d1 100644 --- a/net/ipx/af_ipx.c +++ b/net/ipx/af_ipx.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include @@ -1299,7 +1298,7 @@ static int ipx_setsockopt(struct socket *sock, int level, int optname, int opt; int rc = -EINVAL; - lock_kernel(); + lock_sock(sk); if (optlen != sizeof(int)) goto out; @@ -1314,7 +1313,7 @@ static int ipx_setsockopt(struct socket *sock, int level, int optname, ipx_sk(sk)->type = opt; rc = 0; out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1326,7 +1325,7 @@ static int ipx_getsockopt(struct socket *sock, int level, int optname, int len; int rc = -ENOPROTOOPT; - lock_kernel(); + lock_sock(sk); if (!(level == SOL_IPX && optname == IPX_TYPE)) goto out; @@ -1347,7 +1346,7 @@ static int ipx_getsockopt(struct socket *sock, int level, int optname, rc = 0; out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1396,7 +1395,7 @@ static int ipx_release(struct socket *sock) if (!sk) goto out; - lock_kernel(); + lock_sock(sk); if (!sock_flag(sk, SOCK_DEAD)) sk->sk_state_change(sk); @@ -1404,7 +1403,7 @@ static int ipx_release(struct socket *sock) sock->sk = NULL; sk_refcnt_debug_release(sk); ipx_destroy_socket(sk); - unlock_kernel(); + release_sock(sk); out: return 0; } @@ -1530,11 +1529,12 @@ out: static int ipx_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) { + struct sock *sk = sock->sk; int rc; - lock_kernel(); + lock_sock(sk); rc = __ipx_bind(sock, uaddr, addr_len); - unlock_kernel(); + release_sock(sk); return rc; } @@ -1551,7 +1551,7 @@ static int ipx_connect(struct socket *sock, struct sockaddr *uaddr, sk->sk_state = TCP_CLOSE; sock->state = SS_UNCONNECTED; - lock_kernel(); + lock_sock(sk); if (addr_len != sizeof(*addr)) goto out; addr = (struct sockaddr_ipx *)uaddr; @@ -1598,7 +1598,7 @@ static int ipx_connect(struct socket *sock, struct sockaddr *uaddr, ipxrtr_put(rt); rc = 0; out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1614,7 +1614,7 @@ static int ipx_getname(struct socket *sock, struct sockaddr *uaddr, *uaddr_len = sizeof(struct sockaddr_ipx); - lock_kernel(); + lock_sock(sk); if (peer) { rc = -ENOTCONN; if (sk->sk_state != TCP_ESTABLISHED) @@ -1649,19 +1649,7 @@ static int ipx_getname(struct socket *sock, struct sockaddr *uaddr, rc = 0; out: - unlock_kernel(); - return rc; -} - -static unsigned int ipx_datagram_poll(struct file *file, struct socket *sock, - poll_table *wait) -{ - int rc; - - lock_kernel(); - rc = datagram_poll(file, sock, wait); - unlock_kernel(); - + release_sock(sk); return rc; } @@ -1736,7 +1724,7 @@ static int ipx_sendmsg(struct kiocb *iocb, struct socket *sock, int rc = -EINVAL; int flags = msg->msg_flags; - lock_kernel(); + lock_sock(sk); /* Socket gets bound below anyway */ /* if (sk->sk_zapped) return -EIO; */ /* Socket not bound */ @@ -1788,7 +1776,7 @@ static int ipx_sendmsg(struct kiocb *iocb, struct socket *sock, if (rc >= 0) rc = len; out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1803,7 +1791,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock, struct sk_buff *skb; int copied, rc; - lock_kernel(); + lock_sock(sk); /* put the autobinding in */ if (!ipxs->port) { struct sockaddr_ipx uaddr; @@ -1862,7 +1850,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock, out_free: skb_free_datagram(sk, skb); out: - unlock_kernel(); + release_sock(sk); return rc; } @@ -1874,7 +1862,7 @@ static int ipx_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) struct sock *sk = sock->sk; void __user *argp = (void __user *)arg; - lock_kernel(); + lock_sock(sk); switch (cmd) { case TIOCOUTQ: amount = sk->sk_sndbuf - sk_wmem_alloc_get(sk); @@ -1937,7 +1925,7 @@ static int ipx_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) rc = -ENOIOCTLCMD; break; } - unlock_kernel(); + release_sock(sk); return rc; } @@ -1984,7 +1972,7 @@ static const struct proto_ops ipx_dgram_ops = { .socketpair = sock_no_socketpair, .accept = sock_no_accept, .getname = ipx_getname, - .poll = ipx_datagram_poll, + .poll = datagram_poll, .ioctl = ipx_ioctl, #ifdef CONFIG_COMPAT .compat_ioctl = ipx_compat_ioctl, -- cgit v1.2.3