* [RFC 00/20] Proposal for remaining BKL users
@ 2011-01-25 22:17 Arnd Bergmann
2011-01-25 22:17 ` [PATCH 12/20] x25: remove the BKL Arnd Bergmann
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-25 22:17 UTC (permalink / raw)
To: linux-kernel
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, dri-devel,
Mikulas Patocka, H. Peter Anvin, Ian Kent, linux-cifs,
Nick Bowler, Jeff Layton, Takahiro Hirofuchi, Ross Cohen,
Arnaldo Carvalho de Melo, Evgeniy Dushistov, Arnd Bergmann,
Stuart Swales, Thomas Gleixner, Arjan van de Ven, autofs,
linux-x25, netdev, Greg Kroah-Hartman, Palash Bandyopadhyay,
linux-fsdevel, A
I've gone through all the code in the kernel that
uses the big kernel lock and come up with a solution
that seems at least half-reasonable for each of them.
The decisions are somewhat arbitrary, but here is
what I'd suggest we do:
* Remove in 2.6.39:
i830, autofs3, smbfs
* Move to staging now, kill in 2.6.41 (or later):
appletalk, hpfs
* Work around in an ugly way, but keep alive:
* ufs, ipx, i810, cx25721
* Fix properly:
* usbip, go7007, adfs, x25
Some of the patches are rather tricky and I haven't
really done much proper testing, so I'd much prefer
the maintainers to pick up the patches and do the
necessary testing where possible, or even come up
with a better solution.
Arnd Bergmann (20):
drm/i810: remove the BKL
drm: remove i830 driver
staging/usbip: convert to kthread
staging/cx25721: serialize access to devlist
staging/go7007: remove the BKL
staging: Remove autofs3
staging: remove smbfs
adfs: remove the big kernel lock
hpfs: rename big kernel lock to hpfs_lock
hpfs: replace BKL with a global mutex
hpfs: move to drivers/staging
x25: remove the BKL
appletalk: move to staging
staging/appletalk: remove the BKL
ufs: remove the BKL
ipx: remove the BKL
tracing: don't trace the BKL
rtmutex-tester: remove BKL tests
drivers: remove extraneous includes of smp_lock.h
BKL: That's all, folks
MAINTAINERS | 11 +-
drivers/gpu/drm/Kconfig | 47 +-
drivers/gpu/drm/Makefile | 1 -
drivers/gpu/drm/i810/i810_dma.c | 18 +-
drivers/gpu/drm/i810/i810_drv.c | 6 +-
drivers/gpu/drm/i830/Makefile | 8 -
drivers/gpu/drm/i830/i830_dma.c | 1560 ---------
drivers/gpu/drm/i830/i830_drv.c | 107 -
drivers/gpu/drm/i830/i830_drv.h | 295 --
drivers/gpu/drm/i830/i830_irq.c | 186 --
drivers/net/Makefile | 1 -
drivers/net/appletalk/Makefile | 7 -
drivers/scsi/megaraid/megaraid_sas_fp.c | 1 -
drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 -
drivers/staging/Kconfig | 8 +-
drivers/staging/Makefile | 4 +-
drivers/{net => staging}/appletalk/Kconfig | 1 -
{net => drivers/staging}/appletalk/Makefile | 7 +-
{net => drivers/staging}/appletalk/aarp.c | 2 +-
.../linux => drivers/staging/appletalk}/atalk.h | 0
{net => drivers/staging}/appletalk/atalk_proc.c | 2 +-
drivers/{net => staging}/appletalk/cops.c | 2 +-
drivers/{net => staging}/appletalk/cops.h | 0
drivers/{net => staging}/appletalk/cops_ffdrv.h | 0
drivers/{net => staging}/appletalk/cops_ltdrv.h | 0
{net => drivers/staging}/appletalk/ddp.c | 44 +-
{net => drivers/staging}/appletalk/dev.c | 0
drivers/{net => staging}/appletalk/ipddp.c | 2 +-
drivers/{net => staging}/appletalk/ipddp.h | 0
drivers/{net => staging}/appletalk/ltpc.c | 2 +-
drivers/{net => staging}/appletalk/ltpc.h | 0
.../staging}/appletalk/sysctl_net_atalk.c | 2 +-
drivers/staging/autofs/Kconfig | 22 -
drivers/staging/autofs/Makefile | 7 -
drivers/staging/autofs/TODO | 8 -
drivers/staging/autofs/autofs_i.h | 165 -
drivers/staging/autofs/dirhash.c | 260 --
drivers/staging/autofs/init.c | 52 -
drivers/staging/autofs/inode.c | 288 --
drivers/staging/autofs/root.c | 648 ----
drivers/staging/autofs/symlink.c | 26 -
drivers/staging/autofs/waitq.c | 205 --
drivers/staging/cx25821/Kconfig | 1 -
drivers/staging/cx25821/cx25821-alsa.c | 2 +
drivers/staging/cx25821/cx25821-core.c | 16 +-
drivers/staging/cx25821/cx25821-video.c | 9 +-
drivers/staging/cx25821/cx25821.h | 3 +-
drivers/staging/easycap/easycap.h | 1 -
drivers/staging/easycap/easycap_ioctl.c | 1 -
drivers/staging/go7007/Kconfig | 1 -
drivers/staging/go7007/s2250-loader.c | 3 -
{fs => drivers/staging}/hpfs/Kconfig | 5 +-
{fs => drivers/staging}/hpfs/Makefile | 0
drivers/staging/hpfs/TODO | 5 +
{fs => drivers/staging}/hpfs/alloc.c | 0
{fs => drivers/staging}/hpfs/anode.c | 0
{fs => drivers/staging}/hpfs/buffer.c | 0
{fs => drivers/staging}/hpfs/dentry.c | 0
{fs => drivers/staging}/hpfs/dir.c | 23 +-
{fs => drivers/staging}/hpfs/dnode.c | 0
{fs => drivers/staging}/hpfs/ea.c | 0
{fs => drivers/staging}/hpfs/file.c | 9 +-
{fs => drivers/staging}/hpfs/hpfs.h | 0
{fs => drivers/staging}/hpfs/hpfs_fn.h | 36 +
{fs => drivers/staging}/hpfs/inode.c | 9 +-
{fs => drivers/staging}/hpfs/map.c | 0
{fs => drivers/staging}/hpfs/name.c | 0
{fs => drivers/staging}/hpfs/namei.c | 49 +-
{fs => drivers/staging}/hpfs/super.c | 21 +-
drivers/staging/smbfs/Kconfig | 56 -
drivers/staging/smbfs/Makefile | 18 -
drivers/staging/smbfs/TODO | 8 -
drivers/staging/smbfs/cache.c | 208 --
drivers/staging/smbfs/dir.c | 699 ----
drivers/staging/smbfs/file.c | 456 ---
drivers/staging/smbfs/getopt.c | 64 -
drivers/staging/smbfs/getopt.h | 14 -
drivers/staging/smbfs/inode.c | 854 -----
drivers/staging/smbfs/ioctl.c | 68 -
drivers/staging/smbfs/proc.c | 3502 --------------------
drivers/staging/smbfs/proto.h | 89 -
drivers/staging/smbfs/request.c | 817 -----
drivers/staging/smbfs/request.h | 70 -
drivers/staging/smbfs/smb.h | 118 -
drivers/staging/smbfs/smb_debug.h | 34 -
drivers/staging/smbfs/smb_fs.h | 153 -
drivers/staging/smbfs/smb_fs_i.h | 37 -
drivers/staging/smbfs/smb_fs_sb.h | 100 -
drivers/staging/smbfs/smb_mount.h | 65 -
drivers/staging/smbfs/smbfs.txt | 8 -
drivers/staging/smbfs/smbiod.c | 343 --
drivers/staging/smbfs/smbno.h | 363 --
drivers/staging/smbfs/sock.c | 385 ---
drivers/staging/smbfs/symlink.c | 67 -
drivers/staging/usbip/Kconfig | 2 +-
drivers/staging/usbip/stub.h | 4 +-
drivers/staging/usbip/stub_dev.c | 13 +-
drivers/staging/usbip/stub_rx.c | 13 +-
drivers/staging/usbip/stub_tx.c | 14 +-
drivers/staging/usbip/usbip_common.c | 105 -
drivers/staging/usbip/usbip_common.h | 20 +-
drivers/staging/usbip/usbip_event.c | 31 +-
drivers/staging/usbip/vhci.h | 4 +-
drivers/staging/usbip/vhci_hcd.c | 10 +-
drivers/staging/usbip/vhci_rx.c | 16 +-
drivers/staging/usbip/vhci_sysfs.c | 9 +-
drivers/staging/usbip/vhci_tx.c | 14 +-
drivers/target/target_core_device.c | 1 -
drivers/target/target_core_fabric_lib.c | 1 -
drivers/target/target_core_file.c | 1 -
drivers/target/target_core_hba.c | 1 -
drivers/target/target_core_iblock.c | 1 -
drivers/target/target_core_pscsi.c | 1 -
drivers/target/target_core_rd.c | 1 -
drivers/target/target_core_tpg.c | 1 -
drivers/target/target_core_transport.c | 1 -
drivers/tty/n_hdlc.c | 1 -
drivers/tty/n_r3964.c | 1 -
drivers/tty/pty.c | 1 -
drivers/tty/tty_io.c | 1 -
drivers/tty/tty_ldisc.c | 2 -
drivers/tty/vt/selection.c | 1 -
drivers/tty/vt/vc_screen.c | 1 -
drivers/tty/vt/vt.c | 1 -
drivers/tty/vt/vt_ioctl.c | 1 -
fs/Kconfig | 1 -
fs/Makefile | 1 -
fs/adfs/Kconfig | 1 -
fs/adfs/dir.c | 6 -
fs/adfs/inode.c | 6 -
fs/adfs/super.c | 13 +-
fs/compat_ioctl.c | 1 -
fs/ufs/Kconfig | 1 -
fs/ufs/inode.c | 78 +-
fs/ufs/namei.c | 35 +-
fs/ufs/super.c | 55 +-
fs/ufs/truncate.c | 5 +-
fs/ufs/ufs.h | 6 +-
include/drm/Kbuild | 1 -
include/drm/i830_drm.h | 342 --
include/linux/Kbuild | 1 -
include/linux/hardirq.h | 9 +-
include/linux/smp_lock.h | 65 -
include/trace/events/bkl.h | 61 -
init/Kconfig | 5 -
kernel/rtmutex-tester.c | 39 +-
kernel/sched.c | 7 -
lib/Makefile | 1 -
lib/kernel_lock.c | 143 -
net/Kconfig | 1 -
net/Makefile | 1 -
net/ipx/Kconfig | 1 -
net/ipx/af_ipx.c | 52 +-
net/socket.c | 1 -
net/x25/Kconfig | 1 -
net/x25/af_x25.c | 61 +-
net/x25/x25_out.c | 7 +-
157 files changed, 356 insertions(+), 13722 deletions(-)
delete mode 100644 drivers/gpu/drm/i830/Makefile
delete mode 100644 drivers/gpu/drm/i830/i830_dma.c
delete mode 100644 drivers/gpu/drm/i830/i830_drv.c
delete mode 100644 drivers/gpu/drm/i830/i830_drv.h
delete mode 100644 drivers/gpu/drm/i830/i830_irq.c
delete mode 100644 drivers/net/appletalk/Makefile
rename drivers/{net => staging}/appletalk/Kconfig (98%)
rename {net => drivers/staging}/appletalk/Makefile (56%)
rename {net => drivers/staging}/appletalk/aarp.c (99%)
rename {include/linux => drivers/staging/appletalk}/atalk.h (100%)
rename {net => drivers/staging}/appletalk/atalk_proc.c (99%)
rename drivers/{net => staging}/appletalk/cops.c (99%)
rename drivers/{net => staging}/appletalk/cops.h (100%)
rename drivers/{net => staging}/appletalk/cops_ffdrv.h (100%)
rename drivers/{net => staging}/appletalk/cops_ltdrv.h (100%)
rename {net => drivers/staging}/appletalk/ddp.c (98%)
rename {net => drivers/staging}/appletalk/dev.c (100%)
rename drivers/{net => staging}/appletalk/ipddp.c (99%)
rename drivers/{net => staging}/appletalk/ipddp.h (100%)
rename drivers/{net => staging}/appletalk/ltpc.c (99%)
rename drivers/{net => staging}/appletalk/ltpc.h (100%)
rename {net => drivers/staging}/appletalk/sysctl_net_atalk.c (98%)
delete mode 100644 drivers/staging/autofs/Kconfig
delete mode 100644 drivers/staging/autofs/Makefile
delete mode 100644 drivers/staging/autofs/TODO
delete mode 100644 drivers/staging/autofs/autofs_i.h
delete mode 100644 drivers/staging/autofs/dirhash.c
delete mode 100644 drivers/staging/autofs/init.c
delete mode 100644 drivers/staging/autofs/inode.c
delete mode 100644 drivers/staging/autofs/root.c
delete mode 100644 drivers/staging/autofs/symlink.c
delete mode 100644 drivers/staging/autofs/waitq.c
rename {fs => drivers/staging}/hpfs/Kconfig (80%)
rename {fs => drivers/staging}/hpfs/Makefile (100%)
create mode 100644 drivers/staging/hpfs/TODO
rename {fs => drivers/staging}/hpfs/alloc.c (100%)
rename {fs => drivers/staging}/hpfs/anode.c (100%)
rename {fs => drivers/staging}/hpfs/buffer.c (100%)
rename {fs => drivers/staging}/hpfs/dentry.c (100%)
rename {fs => drivers/staging}/hpfs/dir.c (97%)
rename {fs => drivers/staging}/hpfs/dnode.c (100%)
rename {fs => drivers/staging}/hpfs/ea.c (100%)
rename {fs => drivers/staging}/hpfs/file.c (97%)
rename {fs => drivers/staging}/hpfs/hpfs.h (100%)
rename {fs => drivers/staging}/hpfs/hpfs_fn.h (91%)
rename {fs => drivers/staging}/hpfs/inode.c (98%)
rename {fs => drivers/staging}/hpfs/map.c (100%)
rename {fs => drivers/staging}/hpfs/name.c (100%)
rename {fs => drivers/staging}/hpfs/namei.c (96%)
rename {fs => drivers/staging}/hpfs/super.c (98%)
delete mode 100644 drivers/staging/smbfs/Kconfig
delete mode 100644 drivers/staging/smbfs/Makefile
delete mode 100644 drivers/staging/smbfs/TODO
delete mode 100644 drivers/staging/smbfs/cache.c
delete mode 100644 drivers/staging/smbfs/dir.c
delete mode 100644 drivers/staging/smbfs/file.c
delete mode 100644 drivers/staging/smbfs/getopt.c
delete mode 100644 drivers/staging/smbfs/getopt.h
delete mode 100644 drivers/staging/smbfs/inode.c
delete mode 100644 drivers/staging/smbfs/ioctl.c
delete mode 100644 drivers/staging/smbfs/proc.c
delete mode 100644 drivers/staging/smbfs/proto.h
delete mode 100644 drivers/staging/smbfs/request.c
delete mode 100644 drivers/staging/smbfs/request.h
delete mode 100644 drivers/staging/smbfs/smb.h
delete mode 100644 drivers/staging/smbfs/smb_debug.h
delete mode 100644 drivers/staging/smbfs/smb_fs.h
delete mode 100644 drivers/staging/smbfs/smb_fs_i.h
delete mode 100644 drivers/staging/smbfs/smb_fs_sb.h
delete mode 100644 drivers/staging/smbfs/smb_mount.h
delete mode 100644 drivers/staging/smbfs/smbfs.txt
delete mode 100644 drivers/staging/smbfs/smbiod.c
delete mode 100644 drivers/staging/smbfs/smbno.h
delete mode 100644 drivers/staging/smbfs/sock.c
delete mode 100644 drivers/staging/smbfs/symlink.c
delete mode 100644 include/drm/i830_drm.h
delete mode 100644 include/linux/smp_lock.h
delete mode 100644 include/trace/events/bkl.h
delete mode 100644 lib/kernel_lock.c
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: autofs@linux.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Airlie <airlied@linux.ie>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Evgeniy Dushistov <dushistov@mail.ru>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ian Kent <raven@themaw.net>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jeff Layton <jlayton@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-x25@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: netdev@vger.kernel.org
Cc: Nick Bowler <nbowler@elliptictech.com>
Cc: Palash Bandyopadhyay <palash.bandyopadhyay@conexant.com>
Cc: Ross Cohen <rcohen@snurgle.org>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Stuart Swales <stuart.swales.croftnuisk@gmail.com>
Cc: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 12/20] x25: remove the BKL
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
@ 2011-01-25 22:17 ` Arnd Bergmann
2011-01-27 10:07 ` Andrew Hendry
2011-01-25 22:17 ` [PATCH 13/20] appletalk: move to staging Arnd Bergmann
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-25 22:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Andrew Hendry, linux-x25, netdev
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.
Compile-tested only.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: linux-x25@vger.kernel.org
Cc: netdev@vger.kernel.org
---
net/x25/Kconfig | 1 -
net/x25/af_x25.c | 61 ++++++++++++++++------------------------------------
net/x25/x25_out.c | 7 ++++-
3 files changed, 24 insertions(+), 45 deletions(-)
diff --git a/net/x25/Kconfig b/net/x25/Kconfig
index 2196e55..e6759c9 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 ad96ee9..8f5d1bb 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -40,7 +40,6 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/sched.h>
-#include <linux/smp_lock.h>
#include <linux/timer.h>
#include <linux/string.h>
#include <linux/net.h>
@@ -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,9 +1140,10 @@ 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);
- if (!skb)
- goto out;
+ lock_sock(sk);
+
X25_SKB_CB(skb)->flags = msg->msg_flags;
skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
@@ -1231,26 +1224,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 +1248,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 +1277,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 +1317,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 +1558,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 +1577,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 d00649f..f1a6ff1 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,
+ 1, &err);
+ lock_sock(sk);
+ if (!skbn) {
if (err == -EWOULDBLOCK && noblock){
kfree_skb(skb);
return sent;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 13/20] appletalk: move to staging
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
2011-01-25 22:17 ` [PATCH 12/20] x25: remove the BKL Arnd Bergmann
@ 2011-01-25 22:17 ` Arnd Bergmann
2011-01-25 22:17 ` [PATCH 14/20] staging/appletalk: remove the BKL Arnd Bergmann
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-25 22:17 UTC (permalink / raw)
To: linux-kernel
Cc: Arnd Bergmann, Arnaldo Carvalho de Melo, netdev,
Greg Kroah-Hartman
For all I know, Appletalk is dead, the only reasonable
use right now would be nostalgia, and that can be served
well enough by old kernels. The code is largely not
in a bad shape, but it still uses the big kernel lock,
and nobody seems motivated to change that.
FWIW, the last release of MacOS that supported Appletalk
was MacOS X 10.5, made in 2007, and it has been abandoned
by Apple with 10.6. Using TCP/IP instead of Appletalk has
been supported since MacOS 7.6, which was released in
1997 and is able to run on most of the legacy hardware.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
MAINTAINERS | 3 +--
drivers/net/Makefile | 1 -
drivers/net/appletalk/Makefile | 7 -------
drivers/staging/Kconfig | 2 ++
drivers/staging/Makefile | 1 +
drivers/{net => staging}/appletalk/Kconfig | 0
{net => drivers/staging}/appletalk/Makefile | 7 +++++--
{net => drivers/staging}/appletalk/aarp.c | 2 +-
.../linux => drivers/staging/appletalk}/atalk.h | 0
{net => drivers/staging}/appletalk/atalk_proc.c | 2 +-
drivers/{net => staging}/appletalk/cops.c | 2 +-
drivers/{net => staging}/appletalk/cops.h | 0
drivers/{net => staging}/appletalk/cops_ffdrv.h | 0
drivers/{net => staging}/appletalk/cops_ltdrv.h | 0
{net => drivers/staging}/appletalk/ddp.c | 4 ++--
{net => drivers/staging}/appletalk/dev.c | 0
drivers/{net => staging}/appletalk/ipddp.c | 2 +-
drivers/{net => staging}/appletalk/ipddp.h | 0
drivers/{net => staging}/appletalk/ltpc.c | 2 +-
drivers/{net => staging}/appletalk/ltpc.h | 0
.../staging}/appletalk/sysctl_net_atalk.c | 2 +-
fs/compat_ioctl.c | 1 -
include/linux/Kbuild | 1 -
net/Kconfig | 1 -
net/Makefile | 1 -
net/socket.c | 1 -
26 files changed, 17 insertions(+), 25 deletions(-)
delete mode 100644 drivers/net/appletalk/Makefile
rename drivers/{net => staging}/appletalk/Kconfig (100%)
rename {net => drivers/staging}/appletalk/Makefile (56%)
rename {net => drivers/staging}/appletalk/aarp.c (99%)
rename {include/linux => drivers/staging/appletalk}/atalk.h (100%)
rename {net => drivers/staging}/appletalk/atalk_proc.c (99%)
rename drivers/{net => staging}/appletalk/cops.c (99%)
rename drivers/{net => staging}/appletalk/cops.h (100%)
rename drivers/{net => staging}/appletalk/cops_ffdrv.h (100%)
rename drivers/{net => staging}/appletalk/cops_ltdrv.h (100%)
rename {net => drivers/staging}/appletalk/ddp.c (99%)
rename {net => drivers/staging}/appletalk/dev.c (100%)
rename drivers/{net => staging}/appletalk/ipddp.c (99%)
rename drivers/{net => staging}/appletalk/ipddp.h (100%)
rename drivers/{net => staging}/appletalk/ltpc.c (99%)
rename drivers/{net => staging}/appletalk/ltpc.h (100%)
rename {net => drivers/staging}/appletalk/sysctl_net_atalk.c (98%)
diff --git a/MAINTAINERS b/MAINTAINERS
index fe5ca5f..5021051 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -554,8 +554,7 @@ F: drivers/hwmon/applesmc.c
APPLETALK NETWORK LAYER
M: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
S: Maintained
-F: drivers/net/appletalk/
-F: net/appletalk/
+F: drivers/staging/appletalk/
ARC FRAMEBUFFER DRIVER
M: Jaya Kumar <jayalk@intworks.biz>
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index b90738d..11a9c05 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -265,7 +265,6 @@ obj-$(CONFIG_MACB) += macb.o
obj-$(CONFIG_S6GMAC) += s6gmac.o
obj-$(CONFIG_ARM) += arm/
-obj-$(CONFIG_DEV_APPLETALK) += appletalk/
obj-$(CONFIG_TR) += tokenring/
obj-$(CONFIG_WAN) += wan/
obj-$(CONFIG_ARCNET) += arcnet/
diff --git a/drivers/net/appletalk/Makefile b/drivers/net/appletalk/Makefile
deleted file mode 100644
index 6cfc705..0000000
--- a/drivers/net/appletalk/Makefile
+++ /dev/null
@@ -1,7 +0,0 @@
-#
-# Makefile for drivers/net/appletalk
-#
-
-obj-$(CONFIG_IPDDP) += ipddp.o
-obj-$(CONFIG_COPS) += cops.o
-obj-$(CONFIG_LTPC) += ltpc.o
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 9fc5aa6..04514b5 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -167,6 +167,8 @@ source "drivers/staging/bcm/Kconfig"
source "drivers/staging/ft1000/Kconfig"
+source "drivers/staging/appletalk/Kconfig"
+
source "drivers/staging/intel_sst/Kconfig"
source "drivers/staging/speakup/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 878f381..0ec94e8 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_ATH6K_LEGACY) += ath6kl/
obj-$(CONFIG_USB_ENESTORAGE) += keucr/
obj-$(CONFIG_BCM_WIMAX) += bcm/
obj-$(CONFIG_FT1000) += ft1000/
+obj-$(CONFIG_DEV_APPLETALK) += appletalk/
obj-$(CONFIG_SND_INTEL_SST) += intel_sst/
obj-$(CONFIG_SPEAKUP) += speakup/
obj-$(CONFIG_TOUCHSCREEN_CLEARPAD_TM1217) += cptm1217/
diff --git a/drivers/net/appletalk/Kconfig b/drivers/staging/appletalk/Kconfig
similarity index 100%
rename from drivers/net/appletalk/Kconfig
rename to drivers/staging/appletalk/Kconfig
diff --git a/net/appletalk/Makefile b/drivers/staging/appletalk/Makefile
similarity index 56%
rename from net/appletalk/Makefile
rename to drivers/staging/appletalk/Makefile
index 5cda56e..2a5129a 100644
--- a/net/appletalk/Makefile
+++ b/drivers/staging/appletalk/Makefile
@@ -1,9 +1,12 @@
#
-# Makefile for the Linux AppleTalk layer.
+# Makefile for drivers/staging/appletalk
#
-
obj-$(CONFIG_ATALK) += appletalk.o
appletalk-y := aarp.o ddp.o dev.o
appletalk-$(CONFIG_PROC_FS) += atalk_proc.o
appletalk-$(CONFIG_SYSCTL) += sysctl_net_atalk.o
+
+obj-$(CONFIG_IPDDP) += ipddp.o
+obj-$(CONFIG_COPS) += cops.o
+obj-$(CONFIG_LTPC) += ltpc.o
diff --git a/net/appletalk/aarp.c b/drivers/staging/appletalk/aarp.c
similarity index 99%
rename from net/appletalk/aarp.c
rename to drivers/staging/appletalk/aarp.c
index 50dce79..7163a1d 100644
--- a/net/appletalk/aarp.c
+++ b/drivers/staging/appletalk/aarp.c
@@ -34,7 +34,7 @@
#include <net/sock.h>
#include <net/datalink.h>
#include <net/psnap.h>
-#include <linux/atalk.h>
+#include "atalk.h"
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/proc_fs.h>
diff --git a/include/linux/atalk.h b/drivers/staging/appletalk/atalk.h
similarity index 100%
rename from include/linux/atalk.h
rename to drivers/staging/appletalk/atalk.h
diff --git a/net/appletalk/atalk_proc.c b/drivers/staging/appletalk/atalk_proc.c
similarity index 99%
rename from net/appletalk/atalk_proc.c
rename to drivers/staging/appletalk/atalk_proc.c
index 6ef0e76..d012ba2 100644
--- a/net/appletalk/atalk_proc.c
+++ b/drivers/staging/appletalk/atalk_proc.c
@@ -13,7 +13,7 @@
#include <linux/seq_file.h>
#include <net/net_namespace.h>
#include <net/sock.h>
-#include <linux/atalk.h>
+#include "atalk.h"
static __inline__ struct atalk_iface *atalk_get_interface_idx(loff_t pos)
diff --git a/drivers/net/appletalk/cops.c b/drivers/staging/appletalk/cops.c
similarity index 99%
rename from drivers/net/appletalk/cops.c
rename to drivers/staging/appletalk/cops.c
index 748c9f5..661d42e 100644
--- a/drivers/net/appletalk/cops.c
+++ b/drivers/staging/appletalk/cops.c
@@ -65,7 +65,6 @@ static const char *version =
#include <linux/if_arp.h>
#include <linux/if_ltalk.h>
#include <linux/delay.h> /* For udelay() */
-#include <linux/atalk.h>
#include <linux/spinlock.h>
#include <linux/bitops.h>
#include <linux/jiffies.h>
@@ -74,6 +73,7 @@ static const char *version =
#include <asm/io.h>
#include <asm/dma.h>
+#include "atalk.h"
#include "cops.h" /* Our Stuff */
#include "cops_ltdrv.h" /* Firmware code for Tangent type cards. */
#include "cops_ffdrv.h" /* Firmware code for Dayna type cards. */
diff --git a/drivers/net/appletalk/cops.h b/drivers/staging/appletalk/cops.h
similarity index 100%
rename from drivers/net/appletalk/cops.h
rename to drivers/staging/appletalk/cops.h
diff --git a/drivers/net/appletalk/cops_ffdrv.h b/drivers/staging/appletalk/cops_ffdrv.h
similarity index 100%
rename from drivers/net/appletalk/cops_ffdrv.h
rename to drivers/staging/appletalk/cops_ffdrv.h
diff --git a/drivers/net/appletalk/cops_ltdrv.h b/drivers/staging/appletalk/cops_ltdrv.h
similarity index 100%
rename from drivers/net/appletalk/cops_ltdrv.h
rename to drivers/staging/appletalk/cops_ltdrv.h
diff --git a/net/appletalk/ddp.c b/drivers/staging/appletalk/ddp.c
similarity index 99%
rename from net/appletalk/ddp.c
rename to drivers/staging/appletalk/ddp.c
index c410b93..940dd19 100644
--- a/net/appletalk/ddp.c
+++ b/drivers/staging/appletalk/ddp.c
@@ -63,8 +63,8 @@
#include <net/sock.h>
#include <net/tcp_states.h>
#include <net/route.h>
-#include <linux/atalk.h>
-#include "../core/kmap_skb.h"
+#include "atalk.h"
+#include "../../net/core/kmap_skb.h"
struct datalink_proto *ddp_dl, *aarp_dl;
static const struct proto_ops atalk_dgram_ops;
diff --git a/net/appletalk/dev.c b/drivers/staging/appletalk/dev.c
similarity index 100%
rename from net/appletalk/dev.c
rename to drivers/staging/appletalk/dev.c
diff --git a/drivers/net/appletalk/ipddp.c b/drivers/staging/appletalk/ipddp.c
similarity index 99%
rename from drivers/net/appletalk/ipddp.c
rename to drivers/staging/appletalk/ipddp.c
index 10d0dba..58b4e60 100644
--- a/drivers/net/appletalk/ipddp.c
+++ b/drivers/staging/appletalk/ipddp.c
@@ -29,12 +29,12 @@
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/ip.h>
-#include <linux/atalk.h>
#include <linux/if_arp.h>
#include <linux/slab.h>
#include <net/route.h>
#include <asm/uaccess.h>
+#include "atalk.h"
#include "ipddp.h" /* Our stuff */
static const char version[] = KERN_INFO "ipddp.c:v0.01 8/28/97 Bradford W. Johnson <johns393@maroon.tc.umn.edu>\n";
diff --git a/drivers/net/appletalk/ipddp.h b/drivers/staging/appletalk/ipddp.h
similarity index 100%
rename from drivers/net/appletalk/ipddp.h
rename to drivers/staging/appletalk/ipddp.h
diff --git a/drivers/net/appletalk/ltpc.c b/drivers/staging/appletalk/ltpc.c
similarity index 99%
rename from drivers/net/appletalk/ltpc.c
rename to drivers/staging/appletalk/ltpc.c
index e69eead..60caf89 100644
--- a/drivers/net/appletalk/ltpc.c
+++ b/drivers/staging/appletalk/ltpc.c
@@ -225,7 +225,6 @@ static int dma;
#include <linux/if_ltalk.h>
#include <linux/delay.h>
#include <linux/timer.h>
-#include <linux/atalk.h>
#include <linux/bitops.h>
#include <linux/gfp.h>
@@ -234,6 +233,7 @@ static int dma;
#include <asm/io.h>
/* our stuff */
+#include "atalk.h"
#include "ltpc.h"
static DEFINE_SPINLOCK(txqueue_lock);
diff --git a/drivers/net/appletalk/ltpc.h b/drivers/staging/appletalk/ltpc.h
similarity index 100%
rename from drivers/net/appletalk/ltpc.h
rename to drivers/staging/appletalk/ltpc.h
diff --git a/net/appletalk/sysctl_net_atalk.c b/drivers/staging/appletalk/sysctl_net_atalk.c
similarity index 98%
rename from net/appletalk/sysctl_net_atalk.c
rename to drivers/staging/appletalk/sysctl_net_atalk.c
index 04e9c0d..4c896b6 100644
--- a/net/appletalk/sysctl_net_atalk.c
+++ b/drivers/staging/appletalk/sysctl_net_atalk.c
@@ -8,7 +8,7 @@
#include <linux/sysctl.h>
#include <net/sock.h>
-#include <linux/atalk.h>
+#include "atalk.h"
static struct ctl_table atalk_table[] = {
{
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 61abb63..86a2d7d 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -56,7 +56,6 @@
#include <linux/syscalls.h>
#include <linux/i2c.h>
#include <linux/i2c-dev.h>
-#include <linux/atalk.h>
#include <linux/gfp.h>
#include <net/bluetooth/bluetooth.h>
diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index 2296d8b..362041b 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -43,7 +43,6 @@ header-y += agpgart.h
header-y += aio_abi.h
header-y += apm_bios.h
header-y += arcfb.h
-header-y += atalk.h
header-y += atm.h
header-y += atm_eni.h
header-y += atm_he.h
diff --git a/net/Kconfig b/net/Kconfig
index 7284062..082c8bc 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -204,7 +204,6 @@ source "net/8021q/Kconfig"
source "net/decnet/Kconfig"
source "net/llc/Kconfig"
source "net/ipx/Kconfig"
-source "drivers/net/appletalk/Kconfig"
source "net/x25/Kconfig"
source "net/lapb/Kconfig"
source "net/econet/Kconfig"
diff --git a/net/Makefile b/net/Makefile
index a3330eb..16d9947 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -27,7 +27,6 @@ obj-$(CONFIG_NET_KEY) += key/
obj-$(CONFIG_BRIDGE) += bridge/
obj-$(CONFIG_NET_DSA) += dsa/
obj-$(CONFIG_IPX) += ipx/
-obj-$(CONFIG_ATALK) += appletalk/
obj-$(CONFIG_WAN_ROUTER) += wanrouter/
obj-$(CONFIG_X25) += x25/
obj-$(CONFIG_LAPB) += lapb/
diff --git a/net/socket.c b/net/socket.c
index ac2219f..26f7bcf 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -103,7 +103,6 @@
#include <linux/ipv6_route.h>
#include <linux/route.h>
#include <linux/sockios.h>
-#include <linux/atalk.h>
static int sock_no_open(struct inode *irrelevant, struct file *dontcare);
static ssize_t sock_aio_read(struct kiocb *iocb, const struct iovec *iov,
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 14/20] staging/appletalk: remove the BKL
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
2011-01-25 22:17 ` [PATCH 12/20] x25: remove the BKL Arnd Bergmann
2011-01-25 22:17 ` [PATCH 13/20] appletalk: move to staging Arnd Bergmann
@ 2011-01-25 22:17 ` Arnd Bergmann
2011-01-25 22:29 ` David Miller
2011-01-25 22:17 ` [PATCH 16/20] ipx: " Arnd Bergmann
2011-01-26 2:22 ` [RFC 00/20] Proposal for remaining BKL users Greg KH
4 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-25 22:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Arnaldo Carvalho de Melo, netdev
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 <arnd@arndb.de>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: netdev@vger.kernel.org
---
drivers/staging/appletalk/Kconfig | 1 -
drivers/staging/appletalk/ddp.c | 40 ++++++++++++++----------------------
2 files changed, 16 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/appletalk/Kconfig b/drivers/staging/appletalk/Kconfig
index 0b376a9..f5a8916 100644
--- a/drivers/staging/appletalk/Kconfig
+++ b/drivers/staging/appletalk/Kconfig
@@ -3,7 +3,6 @@
#
config ATALK
tristate "Appletalk protocol support"
- depends on BKL # waiting to be removed from net/appletalk/ddp.c
select LLC
---help---
AppleTalk is the protocol that Apple computers can use to communicate
diff --git a/drivers/staging/appletalk/ddp.c b/drivers/staging/appletalk/ddp.c
index 940dd19..0e2240c 100644
--- a/drivers/staging/appletalk/ddp.c
+++ b/drivers/staging/appletalk/ddp.c
@@ -54,7 +54,6 @@
#include <linux/capability.h>
#include <linux/module.h>
#include <linux/if_arp.h>
-#include <linux/smp_lock.h>
#include <linux/termios.h> /* For TIOCOUTQ/INQ */
#include <linux/compat.h>
#include <linux/slab.h>
@@ -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,
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 16/20] ipx: remove the BKL
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
` (2 preceding siblings ...)
2011-01-25 22:17 ` [PATCH 14/20] staging/appletalk: remove the BKL Arnd Bergmann
@ 2011-01-25 22:17 ` Arnd Bergmann
2011-01-26 2:22 ` [RFC 00/20] Proposal for remaining BKL users Greg KH
4 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-25 22:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Arnd Bergmann, Arnaldo Carvalho de Melo, netdev
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 <arnd@arndb.de>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: netdev@vger.kernel.org
---
net/ipx/Kconfig | 1 -
net/ipx/af_ipx.c | 52 ++++++++++++++++++++--------------------------------
2 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/net/ipx/Kconfig b/net/ipx/Kconfig
index 02549cb..e9ad006 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 da3d21c..2731b51 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -42,7 +42,6 @@
#include <linux/uio.h>
#include <linux/slab.h>
#include <linux/skbuff.h>
-#include <linux/smp_lock.h>
#include <linux/socket.h>
#include <linux/sockios.h>
#include <linux/string.h>
@@ -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,
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 14/20] staging/appletalk: remove the BKL
2011-01-25 22:17 ` [PATCH 14/20] staging/appletalk: remove the BKL Arnd Bergmann
@ 2011-01-25 22:29 ` David Miller
2011-01-26 12:57 ` Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2011-01-25 22:29 UTC (permalink / raw)
To: arnd; +Cc: linux-kernel, acme, netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 25 Jan 2011 23:17:28 +0100
> 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 <arnd@arndb.de>
If you're moving appletalk to staging because "nobody is motivated
enough to remove the BKL" and then you actually do the work to remove
the BKL, I really don't see any point in doing the whole staging
thing.
We always keep an eye on every protocol that sits under the top-level
net/. Every socket API change propagates, as does every other
networking API change that matters for those protocols.
You can move appletalk down to staging if that stops happening, which
it won't.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC 00/20] Proposal for remaining BKL users
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
` (3 preceding siblings ...)
2011-01-25 22:17 ` [PATCH 16/20] ipx: " Arnd Bergmann
@ 2011-01-26 2:22 ` Greg KH
2011-01-26 11:31 ` Arnd Bergmann
4 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2011-01-26 2:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, dri-devel,
Mikulas Patocka, H. Peter Anvin, Ian Kent, linux-cifs,
Nick Bowler, linux-x25, Takahiro Hirofuchi, Ross Cohen,
Arnaldo Carvalho de Melo, Evgeniy Dushistov, Stuart Swales,
Thomas Gleixner, Arjan van de Ven, autofs, Jeff Layton, netdev,
linux-kernel, Palash Bandyopadhyay, linux-fsdevel, Andrew Morton
On Tue, Jan 25, 2011 at 11:17:14PM +0100, Arnd Bergmann wrote:
> I've gone through all the code in the kernel that
> uses the big kernel lock and come up with a solution
> that seems at least half-reasonable for each of them.
>
> The decisions are somewhat arbitrary, but here is
> what I'd suggest we do:
>
> * Remove in 2.6.39:
> i830, autofs3, smbfs
I thought some people really wanted to keep i830. Or was that i810?
I'll drop autofs3 and smbfs, thanks.
> * Move to staging now, kill in 2.6.41 (or later):
> appletalk, hpfs
Sounds good to me.
> * Work around in an ugly way, but keep alive:
> * ufs, ipx, i810, cx25721
>
> * Fix properly:
> * usbip, go7007, adfs, x25
Thanks for the usbip and go7007 patches, I'll queue them up.
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC 00/20] Proposal for remaining BKL users
2011-01-26 2:22 ` [RFC 00/20] Proposal for remaining BKL users Greg KH
@ 2011-01-26 11:31 ` Arnd Bergmann
2011-01-26 11:58 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-26 11:31 UTC (permalink / raw)
To: Greg KH
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, dri-devel,
Mikulas Patocka, H. Peter Anvin, Ian Kent, linux-cifs,
Nick Bowler, linux-x25, Takahiro Hirofuchi, Ross Cohen,
Arnaldo Carvalho de Melo, Evgeniy Dushistov, Stuart Swales,
Thomas Gleixner, Arjan van de Ven, autofs, Jeff Layton, netdev,
linux-kernel, Palash Bandyopadhyay, linux-fsdevel, Andrew Morton
On Wednesday 26 January 2011, Greg KH wrote:
> On Tue, Jan 25, 2011 at 11:17:14PM +0100, Arnd Bergmann wrote:
> > I've gone through all the code in the kernel that
> > uses the big kernel lock and come up with a solution
> > that seems at least half-reasonable for each of them.
> >
> > The decisions are somewhat arbitrary, but here is
> > what I'd suggest we do:
> >
> > * Remove in 2.6.39:
> > i830, autofs3, smbfs
>
> I thought some people really wanted to keep i830. Or was that i810?
> I'll drop autofs3 and smbfs, thanks.
i810 needs to be kept, i830 is obsolete, see the patch changelogs
there. I assume that Dave Airlie will take both patches (i810 BKL
removal and i830 removal) into his 2.6.39 queue.
> > * Work around in an ugly way, but keep alive:
> > * ufs, ipx, i810, cx25721
> >
> > * Fix properly:
> > * usbip, go7007, adfs, x25
>
> Thanks for the usbip and go7007 patches, I'll queue them up.
It would be good if you could also take the cx25721 patch. It's
not the nicest one I've done, but it's a bug fix nonetheless.
Thanks,
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC 00/20] Proposal for remaining BKL users
2011-01-26 11:31 ` Arnd Bergmann
@ 2011-01-26 11:58 ` Mauro Carvalho Chehab
2011-01-26 13:45 ` Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2011-01-26 11:58 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Frederic Weisbecker, dri-devel, Mikulas Patocka, H. Peter Anvin,
Ian Kent, linux-cifs, Nick Bowler, linux-x25, Takahiro Hirofuchi,
Palash Bandyopadhyay, Ross Cohen, Arnaldo Carvalho de Melo,
Evgeniy Dushistov, Stuart Swales, Thomas Gleixner,
Arjan van de Ven, autofs, Jeff Layton, netdev, Greg KH,
linux-kernel, Palash Bandyopadhyay, linux-fsdevel
Em 26-01-2011 09:31, Arnd Bergmann escreveu:
> On Wednesday 26 January 2011, Greg KH wrote:
>> On Tue, Jan 25, 2011 at 11:17:14PM +0100, Arnd Bergmann wrote:
>>> I've gone through all the code in the kernel that
>>> uses the big kernel lock and come up with a solution
>>> that seems at least half-reasonable for each of them.
>>>
>>> The decisions are somewhat arbitrary, but here is
>>> what I'd suggest we do:
>>>
>>> * Remove in 2.6.39:
>>> i830, autofs3, smbfs
>>
>> I thought some people really wanted to keep i830. Or was that i810?
>> I'll drop autofs3 and smbfs, thanks.
>
> i810 needs to be kept, i830 is obsolete, see the patch changelogs
> there. I assume that Dave Airlie will take both patches (i810 BKL
> removal and i830 removal) into his 2.6.39 queue.
>
>>> * Work around in an ugly way, but keep alive:
>>> * ufs, ipx, i810, cx25721
>>>
>>> * Fix properly:
>>> * usbip, go7007, adfs, x25
>>
>> Thanks for the usbip and go7007 patches, I'll queue them up.
>
> It would be good if you could also take the cx25721 patch. It's
> not the nicest one I've done, but it's a bug fix nonetheless.
I guess you're meaning cx25821, right?
Palash should take a look on it and review. This is a device that allows
12 simultaneous streams, so, I suspect that he'll need to do some
changes at the locking schema, to avoid performance bottlenecks.
Cheers,
Mauro
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 14/20] staging/appletalk: remove the BKL
2011-01-25 22:29 ` David Miller
@ 2011-01-26 12:57 ` Arnd Bergmann
0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-26 12:57 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, acme, netdev
On Tuesday 25 January 2011, David Miller wrote:
> If you're moving appletalk to staging because "nobody is motivated
> enough to remove the BKL" and then you actually do the work to remove
> the BKL, I really don't see any point in doing the whole staging
> thing.
>
> We always keep an eye on every protocol that sits under the top-level
> net/. Every socket API change propagates, as does every other
> networking API change that matters for those protocols.
Ok, fair enough. I've removed the "move to staging" patch now from my
series. I'm going to need some help with testing, or at least thoroughly
reviewing the patch, since I am not extremely confident in my own work
there.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC 00/20] Proposal for remaining BKL users
2011-01-26 11:58 ` Mauro Carvalho Chehab
@ 2011-01-26 13:45 ` Arnd Bergmann
2011-01-26 16:24 ` Palash Bandyopadhyay
0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-26 13:45 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Frederic Weisbecker, dri-devel, Mikulas Patocka, H. Peter Anvin,
Ian Kent, linux-cifs, Nick Bowler, linux-x25, Takahiro Hirofuchi,
Ross Cohen, Arnaldo Carvalho de Melo, Evgeniy Dushistov,
Stuart Swales, Thomas Gleixner, Arjan van de Ven, autofs,
Jeff Layton, netdev, Greg KH, linux-kernel, Palash Bandyopadhyay,
linux-fsdevel, Andrew Morton, Andrew
On Wednesday 26 January 2011, Mauro Carvalho Chehab wrote:
> I guess you're meaning cx25821, right?
Right, sorry for the typo.
> Palash should take a look on it and review. This is a device that allows
> 12 simultaneous streams, so, I suspect that he'll need to do some
> changes at the locking schema, to avoid performance bottlenecks.
I would be surprised if there was any measurable performance change.
The BKL was used only in the open() function to iterate the device
list, and that code is nowhere on a critical path, as far as I can tell.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [RFC 00/20] Proposal for remaining BKL users
2011-01-26 13:45 ` Arnd Bergmann
@ 2011-01-26 16:24 ` Palash Bandyopadhyay
0 siblings, 0 replies; 17+ messages in thread
From: Palash Bandyopadhyay @ 2011-01-26 16:24 UTC (permalink / raw)
To: Arnd Bergmann, Mauro Carvalho Chehab
Cc: Frederic Weisbecker, dri-devel@lists.freedesktop.org,
Mikulas Patocka, H. Peter Anvin, Ian Kent,
linux-cifs@vger.kernel.org, Nick Bowler,
linux-x25@vger.kernel.org, Takahiro Hirofuchi, Ross Cohen,
Arnaldo Carvalho de Melo, Evgeniy Dushistov, Stuart Swales,
Thomas Gleixner, Arjan van de Ven, autofs@linux.kernel.org,
Jeff Layton, netdev@vger.kernel.org, Greg KH,
"linux-kernel@vger.ker
I think it should be ok. If we do hit any performance issue, we'll revisit this.
Thanks,
Palash
-----Original Message-----
From: Arnd Bergmann [mailto:arnd@arndb.de]
Sent: Wednesday, January 26, 2011 5:45 AM
To: Mauro Carvalho Chehab
Cc: Greg KH; linux-kernel@vger.kernel.org; Andrew Hendry; Andrew Morton; Arjan van de Ven; Arnaldo Carvalho de Melo; autofs@linux.kernel.org; Chris Wilson; Dave Airlie; dri-devel@lists.freedesktop.org; Evgeniy Dushistov; Frederic Weisbecker; H. Peter Anvin; Ian Kent; Ingo Molnar; Jeff Layton; linux-cifs@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-x25@vger.kernel.org; Mikulas Patocka; netdev@vger.kernel.org; Nick Bowler; Palash Bandyopadhyay; Ross Cohen; Russell King; Stuart Swales; Takahiro Hirofuchi; Thomas Gleixner
Subject: Re: [RFC 00/20] Proposal for remaining BKL users
On Wednesday 26 January 2011, Mauro Carvalho Chehab wrote:
> I guess you're meaning cx25821, right?
Right, sorry for the typo.
> Palash should take a look on it and review. This is a device that allows
> 12 simultaneous streams, so, I suspect that he'll need to do some
> changes at the locking schema, to avoid performance bottlenecks.
I would be surprised if there was any measurable performance change.
The BKL was used only in the open() function to iterate the device
list, and that code is nowhere on a critical path, as far as I can tell.
Arnd
Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer ****************************
"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you."
**********************************************************************
---------------------------------------------------------------------
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 12/20] x25: remove the BKL
2011-01-25 22:17 ` [PATCH 12/20] x25: remove the BKL Arnd Bergmann
@ 2011-01-27 10:07 ` Andrew Hendry
2011-01-27 12:17 ` Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Hendry @ 2011-01-27 10:07 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, linux-x25, netdev
Left it running and put about 3.0G through x.25, it was running fine
until after about 20 hours.
I was stopping the test programs and hit this.
Jan 27 20:18:34 jaunty kernel: [80403.945790] PGD 1d8b00067 PUD 1ddec3067 PMD 0
Jan 27 20:18:34 jaunty kernel: [80403.945836] CPU 3
Jan 27 20:18:34 jaunty kernel: [80403.945842] Modules linked in: x25
nls_cp437 cifs binfmt_misc kvm_intel kvm snd_hda_codec_via
snd_usb_audio snd_hda_intel snd_hda_codec nouveau snd_pcm_oss
snd_mixer_oss snd_pcm snd_seq_dummy snd_hwdep snd_usbmidi_lib
snd_seq_oss snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq
psmouse snd_timer snd_seq_device serio_raw fbcon ttm tileblit font
bitblit softcursor xhci_hcd drm_kms_helper snd drm asus_atk0110
soundcore snd_page_alloc i2c_algo_bit video usbhid hid usb_storage
r8169 pata_jmicron ahci mii libahci
Jan 27 20:18:34 jaunty kernel: [80403.946026]
Jan 27 20:18:34 jaunty kernel: [80403.946034] Pid: 28187, comm:
x25echotest Not tainted 2.6.38-rc2+ #41 P7P55D-E PRO/System Product
Name
Jan 27 20:18:34 jaunty kernel: [80403.946050] RIP:
0010:[<ffffffffa026f197>] [<ffffffffa026f197>]
x25_sendmsg+0x1a7/0x530 [x25]
Jan 27 20:18:34 jaunty kernel: [80403.946072] RSP:
0018:ffff880228dbfcb8 EFLAGS: 00010246
Jan 27 20:18:34 jaunty kernel: [80403.946083] RAX: 0000000000000080
RBX: ffff880228dbfd70 RCX: ffff880228dbfce4
Jan 27 20:18:34 jaunty kernel: [80403.946096] RDX: 00000000fffffe00
RSI: 0000000000000000 RDI: ffff8801ba89f050
Jan 27 20:18:34 jaunty kernel: [80403.946109] RBP: ffff880228dbfd18
R08: ffff88022aa91000 R09: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946482] R10: 0000000000000000
R11: 0000000000000000 R12: ffff8801ba89f000
Jan 27 20:18:34 jaunty kernel: [80403.946495] R13: 0000000000000000
R14: 0000000000000000 R15: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946509] FS:
00007f09b3013700(0000) GS:ffff8800bf460000(0000)
knlGS:0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946523] CS: 0010 DS: 0000 ES:
0000 CR0: 000000008005003b
Jan 27 20:18:34 jaunty kernel: [80403.946534] CR2: 00000000000000b4
CR3: 00000001df992000 CR4: 00000000000006e0
Jan 27 20:18:34 jaunty kernel: [80403.946547] DR0: 0000000000000000
DR1: 0000000000000000 DR2: 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946560] DR3: 0000000000000000
DR6: 00000000ffff0ff0 DR7: 0000000000000400
Jan 27 20:18:34 jaunty kernel: [80403.946574] Process x25echotest
(pid: 28187, threadinfo ffff880228dbe000, task ffff8801d89bc320)
Jan 27 20:18:34 jaunty kernel: [80403.946594] ffff880200000008
0000000000000016 0000303030390009 0000000000000000
Jan 27 20:18:34 jaunty kernel: [80403.946616] ffff880228db0000
fffffe00d8832450 0000000000000000 ffff880228dbfd38
Jan 27 20:18:34 jaunty kernel: [80403.946638] ffff880228dbfec8
ffff880228dbfdf8 ffff8801de73b980 ffff880228dbfd70
Jan 27 20:18:34 jaunty kernel: [80403.946671] [<ffffffff8140cdd3>]
sock_aio_write+0x183/0x1a0
Jan 27 20:18:34 jaunty kernel: [80403.946686] [<ffffffff8110304c>] ?
__pte_alloc+0xdc/0x100
Jan 27 20:18:34 jaunty kernel: [80403.946700] [<ffffffff81138a5a>]
do_sync_write+0xda/0x120
Jan 27 20:18:34 jaunty kernel: [80403.946713] [<ffffffff8140d026>] ?
move_addr_to_user+0x86/0xa0
Jan 27 20:18:34 jaunty kernel: [80403.946729] [<ffffffff812431a3>] ?
security_file_permission+0x23/0x90
Jan 27 20:18:34 jaunty kernel: [80403.946743] [<ffffffff8113903e>]
vfs_write+0x15e/0x180
Jan 27 20:18:34 jaunty kernel: [80403.946757] [<ffffffff81139151>]
sys_write+0x51/0x90
Jan 27 20:18:34 jaunty kernel: [80403.946771] [<ffffffff8100bf42>]
system_call_fastpath+0x16/0x1b
Jan 27 20:18:34 jaunty kernel: [80403.946973] RSP <ffff880228dbfcb8>
Jan 27 20:18:34 jaunty kernel: [80403.950010] ---[ end trace
36cd53b6ce0d6f4b ]---
If i have done it right, x25_sendmsg+0x1a7/0x530 is the skb_reserve
which gets inlined here.
(af_x25.c)
/* Build a packet */
SOCK_DEBUG(sk, "x25_sendmsg: sendto: building packet.\n");
if ((msg->msg_flags & MSG_OOB) && len > 32)
len = 32;
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);
X25_SKB_CB(skb)->flags = msg->msg_flags;
skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
/*
* Put the data on the end
*/
SOCK_DEBUG(sk, "x25_sendmsg: Copying user data\n");
objdump -dS show it at 2197 here.
static inline void skb_reserve(struct sk_buff *skb, int len)
{
skb->data += len;
skb->tail += len;
2197: 41 83 87 b4 00 00 00 addl $0x16,0xb4(%r15) <---
219e: 16
219f: 41 89 47 28 mov %eax,0x28(%r15)
21a3: 49 8b 87 c8 00 00 00 mov 0xc8(%r15),%rax
21aa: 48 83 c0 16 add $0x16,%rax
skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
But im not sure where to go from there...
On Wed, Jan 26, 2011 at 9:17 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> 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.
>
> Compile-tested only.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> Cc: linux-x25@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> net/x25/Kconfig | 1 -
> net/x25/af_x25.c | 61 ++++++++++++++++------------------------------------
> net/x25/x25_out.c | 7 ++++-
> 3 files changed, 24 insertions(+), 45 deletions(-)
>
> diff --git a/net/x25/Kconfig b/net/x25/Kconfig
> index 2196e55..e6759c9 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 ad96ee9..8f5d1bb 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -40,7 +40,6 @@
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> -#include <linux/smp_lock.h>
> #include <linux/timer.h>
> #include <linux/string.h>
> #include <linux/net.h>
> @@ -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,9 +1140,10 @@ 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);
> - if (!skb)
> - goto out;
> + lock_sock(sk);
> +
> X25_SKB_CB(skb)->flags = msg->msg_flags;
>
> skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
> @@ -1231,26 +1224,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 +1248,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 +1277,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 +1317,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 +1558,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 +1577,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 d00649f..f1a6ff1 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,
> + 1, &err);
> + lock_sock(sk);
> + if (!skbn) {
> if (err == -EWOULDBLOCK && noblock){
> kfree_skb(skb);
> return sent;
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 12/20] x25: remove the BKL
2011-01-27 10:07 ` Andrew Hendry
@ 2011-01-27 12:17 ` Arnd Bergmann
2011-01-27 12:38 ` [PATCH v2] " Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-27 12:17 UTC (permalink / raw)
To: Andrew Hendry; +Cc: linux-kernel, linux-x25, netdev
On Thursday 27 January 2011, Andrew Hendry wrote:
> Left it running and put about 3.0G through x.25, it was running fine
> until after about 20 hours.
> I was stopping the test programs and hit this.
>
> Jan 27 20:18:34 jaunty kernel: [80403.945790] PGD 1d8b00067 PUD 1ddec3067 PMD 0
Is there no long above this about what problem was hit? There
is normally one saying things like "Bug: unable to handle ..."
Well, nevermind. It seems I could figure it out anyway:
> Jan 27 20:18:34 jaunty kernel: [80403.946083] RAX: 0000000000000080 RBX: ffff880228dbfd70 RCX: ffff880228dbfce4
> Jan 27 20:18:34 jaunty kernel: [80403.946096] RDX: 00000000fffffe00 RSI: 0000000000000000 RDI: ffff8801ba89f050
> Jan 27 20:18:34 jaunty kernel: [80403.946109] RBP: ffff880228dbfd18 R08: ffff88022aa91000 R09: 0000000000000000
> Jan 27 20:18:34 jaunty kernel: [80403.946482] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801ba89f000
> Jan 27 20:18:34 jaunty kernel: [80403.946495] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> ...
>
> If i have done it right, x25_sendmsg+0x1a7/0x530 is the skb_reserve
> which gets inlined here.
> (af_x25.c)
> /* Build a packet */
> SOCK_DEBUG(sk, "x25_sendmsg: sendto: building packet.\n");
>
> if ((msg->msg_flags & MSG_OOB) && len > 32)
> len = 32;
>
> 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);
>
> X25_SKB_CB(skb)->flags = msg->msg_flags;
ok.
> objdump -dS show it at 2197 here.
>
> static inline void skb_reserve(struct sk_buff *skb, int len)
> {
> skb->data += len;
> skb->tail += len;
> 2197: 41 83 87 b4 00 00 00 addl $0x16,0xb4(%r15) <---
> 219e: 16
> 219f: 41 89 47 28 mov %eax,0x28(%r15)
> 21a3: 49 8b 87 c8 00 00 00 mov 0xc8(%r15),%rax
> 21aa: 48 83 c0 16 add $0x16,%rax
> skb_reserve(skb, X25_MAX_L2_LEN + X25_EXT_MIN_LEN);
>
> But im not sure where to go from there...
It's pretty clear that %r15 is the skb in this, and from the registers in the dump,
you can see that it's NULL. skb has just been returned from sock_alloc_send_skb,
which means that this function failed.
And indeed:
> > @@ -1148,9 +1140,10 @@ 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);
> > - if (!skb)
> > - goto out;
> > + lock_sock(sk);
> > +
> > X25_SKB_CB(skb)->flags = msg->msg_flags;
I accidentally removed the error handling in my patch. No idea how that
happened, it certainly wasn't intentional. Thanks a lot for the thorough
testing and the detailed bug report!
I'll follow up with a fixed patch that puts the error path back in.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] x25: remove the BKL
2011-01-27 12:17 ` Arnd Bergmann
@ 2011-01-27 12:38 ` Arnd Bergmann
2011-01-27 13:20 ` Eric Dumazet
0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-27 12:38 UTC (permalink / raw)
To: Andrew Hendry; +Cc: linux-kernel, linux-x25, netdev
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.
Compile-tested only.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: linux-x25@vger.kernel.org
Cc: netdev@vger.kernel.org
---
v2: fix possible NULL-pointer dereference in x25_sendmsg
net/x25/Kconfig | 1 -
net/x25/af_x25.c | 58 ++++++++++++++++------------------------------------
net/x25/x25_out.c | 7 ++++-
3 files changed, 23 insertions(+), 43 deletions(-)
diff --git a/net/x25/Kconfig b/net/x25/Kconfig
index 2196e55..e6759c9 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 ad96ee9..4680b1e 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -40,7 +40,6 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/sched.h>
-#include <linux/smp_lock.h>
#include <linux/timer.h>
#include <linux/string.h>
#include <linux/net.h>
@@ -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 d00649f..f1a6ff1 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,
+ 1, &err);
+ lock_sock(sk);
+ if (!skbn) {
if (err == -EWOULDBLOCK && noblock){
kfree_skb(skb);
return sent;
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] x25: remove the BKL
2011-01-27 12:38 ` [PATCH v2] " Arnd Bergmann
@ 2011-01-27 13:20 ` Eric Dumazet
2011-01-27 13:43 ` Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2011-01-27 13:20 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Andrew Hendry, linux-kernel, linux-x25, netdev
Le jeudi 27 janvier 2011 à 13:38 +0100, Arnd Bergmann a écrit :
> diff --git a/net/x25/x25_out.c b/net/x25/x25_out.c
> index d00649f..f1a6ff1 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,
> + 1, &err);
> + lock_sock(sk);
> + if (!skbn) {
> if (err == -EWOULDBLOCK && noblock){
> kfree_skb(skb);
> return sent;
This part looks strange :
noblock variable became "const 1 : NOBLOCK"
Why releasing socket if you dont block in sock_alloc_send_skb() ?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] x25: remove the BKL
2011-01-27 13:20 ` Eric Dumazet
@ 2011-01-27 13:43 ` Arnd Bergmann
0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2011-01-27 13:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrew Hendry, linux-kernel, linux-x25, netdev
On Thursday 27 January 2011, Eric Dumazet wrote:
> Le jeudi 27 janvier 2011 à 13:38 +0100, Arnd Bergmann a écrit :
> > diff --git a/net/x25/x25_out.c b/net/x25/x25_out.c
> > index d00649f..f1a6ff1 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,
> > + 1, &err);
> > + lock_sock(sk);
> > + if (!skbn) {
> > if (err == -EWOULDBLOCK && noblock){
> > kfree_skb(skb);
> > return sent;
>
> This part looks strange :
>
> noblock variable became "const 1 : NOBLOCK"
>
> Why releasing socket if you dont block in sock_alloc_send_skb() ?
Leftover from an earlier version of the patch, thanks for catching this!
Originally, I wrote this as
long timeo = sock_sndtimeo(sk, noblock)
do {
skbn = sock_alloc_send_skb(sk, frontlen + max_len, 1, &err);
if (skbn)
break;
release_sock(sk);
timeo = sock_wait_for_wmem(sk, timeo);
lock_sock(sk);
} while (timeo);
Then I forgot to flip it back after I noticed that other protocols also just
call release_sock/lock_sock around sock_alloc_send_skb.
I think I'd better go over the whole series and see if there are more things
that got slightly broken...
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-01-27 13:43 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-25 22:17 [RFC 00/20] Proposal for remaining BKL users Arnd Bergmann
2011-01-25 22:17 ` [PATCH 12/20] x25: remove the BKL Arnd Bergmann
2011-01-27 10:07 ` Andrew Hendry
2011-01-27 12:17 ` Arnd Bergmann
2011-01-27 12:38 ` [PATCH v2] " Arnd Bergmann
2011-01-27 13:20 ` Eric Dumazet
2011-01-27 13:43 ` Arnd Bergmann
2011-01-25 22:17 ` [PATCH 13/20] appletalk: move to staging Arnd Bergmann
2011-01-25 22:17 ` [PATCH 14/20] staging/appletalk: remove the BKL Arnd Bergmann
2011-01-25 22:29 ` David Miller
2011-01-26 12:57 ` Arnd Bergmann
2011-01-25 22:17 ` [PATCH 16/20] ipx: " Arnd Bergmann
2011-01-26 2:22 ` [RFC 00/20] Proposal for remaining BKL users Greg KH
2011-01-26 11:31 ` Arnd Bergmann
2011-01-26 11:58 ` Mauro Carvalho Chehab
2011-01-26 13:45 ` Arnd Bergmann
2011-01-26 16:24 ` Palash Bandyopadhyay
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).