Skip to contentSkip to conversation

Proposal for remaining BKL users

RFC In review Arnd Bergmann 21 participants 60 messages Jan 25, 2011 – Mar 1, 2011

Subsystems (18)
lore
Share & tools
lore
#0 lore raw

cover Proposal for remaining BKL users

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 <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Dave Airlie <[email protected]>
Cc David Airlie <[email protected]>
Cc Evgeniy Dushistov <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ian Kent <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jeff Layton <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Mikulas Patocka <[email protected]>
Cc Nick Bowler <[email protected]>
Cc Palash Bandyopadhyay <[email protected]>
Cc Ross Cohen <[email protected]>
Cc Russell King <[email protected]>
Cc Stuart Swales <[email protected]>
Cc Takahiro Hirofuchi <[email protected]>
Cc Thomas Gleixner <[email protected]>
#1 lore raw

PATCH 1/20 drm/i810: remove the BKL

SMP i810 systems were practically nonexistent and the configuration was not officially supported by Intel at the time when Pentium-III was common.

With this change, it is still possible to build a distribution kernel that has support for SMP and includes the i810 driver without the BKL. As a precaution, check for the theoretical SMP case at run time and refuse to load the driver.

We also need to disable CONFIG_PREEMPT builds for this driver.

---
 drivers/gpu/drm/Kconfig         |    4 ++--
 drivers/gpu/drm/i810/i810_dma.c |   18 +-----------------
 drivers/gpu/drm/i810/i810_drv.c |    6 +++++-
 3 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/Kconfig +2 -2
index bea966f..9fc650d 100644
@@ -73,8 +73,8 @@ source "drivers/gpu/drm/radeon/Kconfig"
 
 config DRM_I810
 	tristate "Intel I810"
-	# BKL usage in order to avoid AB-BA deadlocks, may become BROKEN_ON_SMP
-	depends on DRM && AGP && AGP_INTEL && BKL
+	# !PREEMPT because of missing ioctl locking
+	depends on DRM && AGP && AGP_INTEL && (!PREEMPT || BROKEN)
 	help
 	  Choose this option if you have an Intel I810 graphics card.  If M is
 	  selected, the module will be called i810.  AGP support is required
drivers/gpu/drm/i810/i810_dma.c +1 -17
index ff33e53..8f371e8 100644
@@ -37,7 +37,6 @@
 #include <linux/interrupt.h>	/* For task queue support */
 #include <linux/delay.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/pagemap.h>
 
 #define I810_BUF_FREE		2
@@ -94,7 +93,6 @@ static int i810_mmap_buffers(struct file *filp, struct vm_area_struct *vma)
 	struct drm_buf *buf;
 	drm_i810_buf_priv_t *buf_priv;
 
-	lock_kernel();
 	dev = priv->minor->dev;
 	dev_priv = dev->dev_private;
 	buf = dev_priv->mmap_buffer;
@@ -104,7 +102,6 @@ static int i810_mmap_buffers(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_file = filp;
 
 	buf_priv->currently_mapped = I810_BUF_MAPPED;
-	unlock_kernel();
 
 	if (io_remap_pfn_range(vma, vma->vm_start,
 			       vma->vm_pgoff,
@@ -116,7 +113,7 @@ static int i810_mmap_buffers(struct file *filp, struct vm_area_struct *vma)
 static const struct file_operations i810_buffer_fops = {
 	.open = drm_open,
 	.release = drm_release,
-	.unlocked_ioctl = i810_ioctl,
+	.unlocked_ioctl = drm_ioctl,
 	.mmap = i810_mmap_buffers,
 	.fasync = drm_fasync,
 	.llseek = noop_llseek,
@@ -1242,19 +1239,6 @@ int i810_driver_dma_quiescent(struct drm_device *dev)
 	return 0;
 }
 
-/*
- * call the drm_ioctl under the big kernel lock because
- * to lock against the i810_mmap_buffers function.
- */
-long i810_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	int ret;
-	lock_kernel();
-	ret = drm_ioctl(file, cmd, arg);
-	unlock_kernel();
-	return ret;
-}
-
 struct drm_ioctl_desc i810_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I810_INIT, i810_dma_init, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I810_VERTEX, i810_dma_vertex, DRM_AUTH|DRM_UNLOCKED),
drivers/gpu/drm/i810/i810_drv.c +5 -1
index 88bcd33..0152fa2 100644
@@ -57,7 +57,7 @@ static struct drm_driver driver = {
 		 .owner = THIS_MODULE,
 		 .open = drm_open,
 		 .release = drm_release,
-		 .unlocked_ioctl = i810_ioctl,
+		 .unlocked_ioctl = drm_ioctl,
 		 .mmap = drm_mmap,
 		 .poll = drm_poll,
 		 .fasync = drm_fasync,
@@ -79,6 +79,10 @@ static struct drm_driver driver = {
 
 static int __init i810_init(void)
 {
+	if (num_possible_cpus() > 1) {
+		pr_err("drm/i810 does not support SMP\n");
+		return -EINVAL;
+	}
 	driver.num_ioctls = i810_max_ioctl;
 	return drm_init(&driver);
 }
Signature · 1 line
1.7.1
#2 lore raw

PATCH 2/20 drm: remove i830 driver

In reply to Arnd Bergmann

This driver is one of the last users of the big kernel lock, which is going away. All the hardware supported by this driver also works with the newer i915 driver, and recent X.org releases only work with that driver anyway.

---
 drivers/gpu/drm/Kconfig         |   43 +-
 drivers/gpu/drm/Makefile        |    1 -
 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 -----
 include/drm/Kbuild              |    1 -
 include/drm/i830_drm.h          |  342 ---------
 9 files changed, 17 insertions(+), 2526 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 include/drm/i830_drm.h
drivers/gpu/drm/Kconfig +17 -26
index 9fc650d..eac02c7 100644
@@ -80,25 +80,10 @@ config DRM_I810
 	  selected, the module will be called i810.  AGP support is required
 	  for this driver to work.
 
-choice
-	prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
-	depends on DRM && AGP && AGP_INTEL
-	optional
-
-config DRM_I830
-	tristate "i830 driver"
-	# BKL usage in order to avoid AB-BA deadlocks, i830 may get removed
-	depends on BKL
-	help
-	  Choose this option if you have a system that has Intel 830M, 845G,
-	  852GM, 855GM or 865G integrated graphics.  If M is selected, the
-	  module will be called i830.  AGP support is required for this driver
-	  to work. This driver is used by the older X releases X.org 6.7 and
-	  XFree86 4.3. If unsure, build this and i915 as modules and the X server
-	  will load the correct one.
-
 config DRM_I915
-	tristate "i915 driver"
+	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
+	depends on DRM
+	depends on AGP
 	depends on AGP_INTEL
 	select SHMEM
 	select DRM_KMS_HELPER
@@ -112,12 +97,20 @@ config DRM_I915
 	select ACPI_VIDEO if ACPI
 	select ACPI_BUTTON if ACPI
 	help
-	  Choose this option if you have a system that has Intel 830M, 845G,
-	  852GM, 855GM 865G or 915G integrated graphics.  If M is selected, the
-	  module will be called i915.  AGP support is required for this driver
-	  to work. This driver is used by the Intel driver in X.org 6.8 and
-	  XFree86 4.4 and above. If unsure, build this and i830 as modules and
-	  the X server will load the correct one.
+	  Choose this option if you have a system that has "Intel Graphics
+	  Media Accelerator" or "HD Graphics" integrated graphics,
+	  including 830M, 845G, 852GM, 855GM, 865G, 915G, 945G, 965G,
+	  G35, G41, G43, G45 chipsets and Celeron, Pentium, Core i3,
+	  Core i5, Core i7 as well as Atom CPUs with integrated graphics.
+	  If M is selected, the module will be called i915.  AGP support
+	  is required for this driver to work. This driver is used by
+	  the Intel driver in X.org 6.8 and XFree86 4.4 and above. It
+	  replaces the older i830 module that supported a subset of the
+	  hardware in older X.org releases.
+
+	  Note that the older i810/i815 chipsets require the use of the
+	  i810 driver instead, and the Atom z5xx series has an entirely
+	  different implementation.
 
 config DRM_I915_KMS
 	bool "Enable modesetting on intel by default"
@@ -129,8 +122,6 @@ config DRM_I915_KMS
 	  the driver to bind to PCI devices, which precludes loading things
 	  like intelfb.
 
-endchoice
-
 config DRM_MGA
 	tristate "Matrox g200/g400"
 	depends on DRM && PCI
drivers/gpu/drm/Makefile +0 -1
index 997c43d..d9cb3e3 100644
@@ -29,7 +29,6 @@ obj-$(CONFIG_DRM_R128)	+= r128/
 obj-$(CONFIG_DRM_RADEON)+= radeon/
 obj-$(CONFIG_DRM_MGA)	+= mga/
 obj-$(CONFIG_DRM_I810)	+= i810/
-obj-$(CONFIG_DRM_I830)	+= i830/
 obj-$(CONFIG_DRM_I915)  += i915/
 obj-$(CONFIG_DRM_SIS)   += sis/
 obj-$(CONFIG_DRM_SAVAGE)+= savage/
drivers/gpu/drm/i830/Makefile deleted +0 -8
deleted file mode 100644
index c642ee0..0000000
@@ -1,8 +0,0 @@
-#
-# Makefile for the drm device driver.  This driver provides support for the
-# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
-
-ccflags-y := -Iinclude/drm
-i830-y := i830_drv.o i830_dma.o i830_irq.o
-
-obj-$(CONFIG_DRM_I830) += i830.o
drivers/gpu/drm/i830/i830_dma.c deleted +0 -1560

1,560 lines not on this page · raw message

drivers/gpu/drm/i830/i830_drv.c deleted +0 -107

107 lines not on this page · raw message

drivers/gpu/drm/i830/i830_drv.h deleted +0 -295

295 lines not on this page · raw message

drivers/gpu/drm/i830/i830_irq.c deleted +0 -186

186 lines not on this page · raw message

include/drm/Kbuild +0 -1
index ffec177..3a60ac8 100644
@@ -2,7 +2,6 @@ header-y += drm.h
 header-y += drm_mode.h
 header-y += drm_sarea.h
 header-y += i810_drm.h
-header-y += i830_drm.h
 header-y += i915_drm.h
 header-y += mga_drm.h
 header-y += nouveau_drm.h
include/drm/i830_drm.h deleted +0 -342

342 lines not on this page · raw message

Signature · 1 line
1.7.1
#3 lore raw

PATCH 3/20 staging/usbip: convert to kthread

In reply to Arnd Bergmann

usbip has its own infrastructure for managing kernel threads, similar to kthread. By changing it to use the standard functions, we can simplify the code and get rid of one of the last BKL users at the same time.

Compile-tested only, please verify.

Cc Greg Kroah-Hartman <[email protected]>
Cc Takahiro Hirofuchi <[email protected]>
---
 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 ++---
 13 files changed, 58 insertions(+), 197 deletions(-)
drivers/staging/usbip/Kconfig +1 -1
index b11ec37..2c1d10a 100644
@@ -1,6 +1,6 @@
 config USB_IP_COMMON
 	tristate "USB IP support (EXPERIMENTAL)"
-	depends on USB && NET && EXPERIMENTAL && BKL
+	depends on USB && NET && EXPERIMENTAL
 	default N
 	---help---
 	  This enables pushing USB packets over IP to allow remote
drivers/staging/usbip/stub.h +2 -2
index 30dbfb6..5b44e7b 100644
@@ -94,13 +94,13 @@ extern struct kmem_cache *stub_priv_cache;
 
 /* stub_tx.c */
 void stub_complete(struct urb *);
-void stub_tx_loop(struct usbip_task *);
+int stub_tx_loop(void *data);
 
 /* stub_dev.c */
 extern struct usb_driver stub_driver;
 
 /* stub_rx.c */
-void stub_rx_loop(struct usbip_task *);
+int stub_rx_loop(void *data);
 void stub_enqueue_ret_unlink(struct stub_device *, __u32, __u32);
 
 /* stub_main.c */
drivers/staging/usbip/stub_dev.c +8 -5
index b186b5f..4ee70d4 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -138,7 +139,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,
 
 		spin_unlock(&sdev->ud.lock);
 
-		usbip_start_threads(&sdev->ud);
+		wake_up_process(sdev->ud.tcp_rx);
+		wake_up_process(sdev->ud.tcp_tx);
 
 		spin_lock(&sdev->ud.lock);
 		sdev->ud.status = SDEV_ST_USED;
@@ -218,7 +220,8 @@ static void stub_shutdown_connection(struct usbip_device *ud)
 	}
 
 	/* 1. stop threads */
-	usbip_stop_threads(ud);
+	kthread_stop(ud->tcp_rx);
+	kthread_stop(ud->tcp_tx);
 
 	/* 2. close the socket */
 	/*
@@ -333,8 +336,8 @@ static struct stub_device *stub_device_alloc(struct usb_interface *interface)
 	 */
 	sdev->devid     = (busnum << 16) | devnum;
 
-	usbip_task_init(&sdev->ud.tcp_rx, "stub_rx", stub_rx_loop);
-	usbip_task_init(&sdev->ud.tcp_tx, "stub_tx", stub_tx_loop);
+	sdev->ud.tcp_rx = kthread_create(stub_rx_loop, &sdev->ud, "stub_rx");
+	sdev->ud.tcp_tx = kthread_create(stub_tx_loop, &sdev->ud, "stub_tx");
 
 	sdev->ud.side = USBIP_STUB;
 	sdev->ud.status = SDEV_ST_AVAILABLE;
@@ -537,7 +540,7 @@ static void stub_disconnect(struct usb_interface *interface)
 	stub_remove_files(&interface->dev);
 
 	/*If usb reset called from event handler*/
-	if (busid_priv->sdev->ud.eh.thread == current) {
+	if (busid_priv->sdev->ud.eh == current) {
 		busid_priv->interf_count--;
 		return;
 	}
drivers/staging/usbip/stub_rx.c +5 -8
index 3de6fd2..4ecc2bb 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -616,19 +617,15 @@ static void stub_rx_pdu(struct usbip_device *ud)
 
 }
 
-void stub_rx_loop(struct usbip_task *ut)
+int stub_rx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_rx);
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_stub_rx("signal caught!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 
 		stub_rx_pdu(ud);
 	}
+	return 0;
 }
drivers/staging/usbip/stub_tx.c +6 -8
index d7136e2..2477481 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -333,17 +334,12 @@ static int stub_send_ret_unlink(struct stub_device *sdev)
 
 /*-------------------------------------------------------------------------*/
 
-void stub_tx_loop(struct usbip_task *ut)
+int stub_tx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_tx);
+	struct usbip_device *ud = data;
 	struct stub_device *sdev = container_of(ud, struct stub_device, ud);
 
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_stub_tx("signal catched\n");
-			break;
-		}
-
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 
@@ -371,4 +367,6 @@ void stub_tx_loop(struct usbip_task *ut)
 				(!list_empty(&sdev->priv_tx) ||
 				 !list_empty(&sdev->unlink_tx)));
 	}
+
+	return 0;
 }
drivers/staging/usbip/usbip_common.c +0 -105

117 lines not on this page · raw message

drivers/staging/usbip/usbip_common.h +3 -17
index d280e23..9f809c3 100644
@@ -307,13 +307,6 @@ void usbip_dump_header(struct usbip_header *pdu);
 
 struct usbip_device;
 
-struct usbip_task {
-	struct task_struct *thread;
-	struct completion thread_done;
-	char *name;
-	void (*loop_ops)(struct usbip_task *);
-};
-
 enum usbip_side {
 	USBIP_VHCI,
 	USBIP_STUB,
@@ -346,8 +339,8 @@ struct usbip_device {
 
 	struct socket *tcp_socket;
 
-	struct usbip_task tcp_rx;
-	struct usbip_task tcp_tx;
+	struct task_struct *tcp_rx;
+	struct task_struct *tcp_tx;
 
 	/* event handler */
 #define USBIP_EH_SHUTDOWN	(1 << 0)
@@ -367,7 +360,7 @@ struct usbip_device {
 #define	VDEV_EVENT_ERROR_MALLOC	(USBIP_EH_SHUTDOWN | USBIP_EH_UNUSABLE)
 
 	unsigned long event;
-	struct usbip_task eh;
+	struct task_struct *eh;
 	wait_queue_head_t eh_waitq;
 
 	struct eh_ops {
@@ -378,13 +371,6 @@ struct usbip_device {
 };
 
 
-void usbip_task_init(struct usbip_task *ut, char *,
-				void (*loop_ops)(struct usbip_task *));
-
-int usbip_start_threads(struct usbip_device *ud);
-void usbip_stop_threads(struct usbip_device *ud);
-int usbip_thread(void *param);
-
 void usbip_pack_pdu(struct usbip_header *pdu, struct urb *urb, int cmd,
 								int pack);
 
drivers/staging/usbip/usbip_event.c +9 -22
index af3832b..89aecec 100644
@@ -62,16 +62,11 @@ static int event_handler(struct usbip_device *ud)
 	return 0;
 }
 
-static void event_handler_loop(struct usbip_task *ut)
+static int event_handler_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, eh);
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_eh("signal catched!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
+	while (!kthread_should_stop()) {
 		if (event_handler(ud) < 0)
 			break;
 
@@ -79,38 +74,30 @@ static void event_handler_loop(struct usbip_task *ut)
 					usbip_event_happened(ud));
 		usbip_dbg_eh("wakeup\n");
 	}
+	return 0;
 }
 
 int usbip_start_eh(struct usbip_device *ud)
 {
-	struct usbip_task *eh = &ud->eh;
-	struct task_struct *th;
-
 	init_waitqueue_head(&ud->eh_waitq);
 	ud->event = 0;
 
-	usbip_task_init(eh, "usbip_eh", event_handler_loop);
-
-	th = kthread_run(usbip_thread, (void *)eh, "usbip");
-	if (IS_ERR(th)) {
+	ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
+	if (IS_ERR(ud->eh)) {
 		printk(KERN_WARNING
 			"Unable to start control thread\n");
-		return PTR_ERR(th);
+		return PTR_ERR(ud->eh);
 	}
-
-	wait_for_completion(&eh->thread_done);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usbip_start_eh);
 
 void usbip_stop_eh(struct usbip_device *ud)
 {
-	struct usbip_task *eh = &ud->eh;
-
-	if (eh->thread == current)
+	if (ud->eh == current)
 		return; /* do not wait for myself */
 
-	wait_for_completion(&eh->thread_done);
+	kthread_stop(ud->eh);
 	usbip_dbg_eh("usbip_eh has finished\n");
 }
 EXPORT_SYMBOL_GPL(usbip_stop_eh);
drivers/staging/usbip/vhci.h +2 -2
index 41a1fe5..ed51983 100644
@@ -116,8 +116,8 @@ extern struct attribute_group dev_attr_group;
 /* vhci_hcd.c */
 void rh_port_connect(int rhport, enum usb_device_speed speed);
 void rh_port_disconnect(int rhport);
-void vhci_rx_loop(struct usbip_task *ut);
-void vhci_tx_loop(struct usbip_task *ut);
+int vhci_rx_loop(void *data);
+int vhci_tx_loop(void *data);
 
 #define hardware		(&the_controller->pdev.dev)
 
drivers/staging/usbip/vhci_hcd.c +7 -3
index 08bd26a..f048b47 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -840,7 +841,10 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
 		kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR);
 	}
 
-	usbip_stop_threads(&vdev->ud);
+	/* kill threads related to this sdev, if v.c. exists */
+	kthread_stop(vdev->ud.tcp_rx);
+	kthread_stop(vdev->ud.tcp_tx);
+
 	usbip_uinfo("stop threads\n");
 
 	/* active connection is closed */
@@ -907,8 +911,8 @@ static void vhci_device_init(struct vhci_device *vdev)
 {
 	memset(vdev, 0, sizeof(*vdev));
 
-	usbip_task_init(&vdev->ud.tcp_rx, "vhci_rx", vhci_rx_loop);
-	usbip_task_init(&vdev->ud.tcp_tx, "vhci_tx", vhci_tx_loop);
+	vdev->ud.tcp_rx = kthread_create(vhci_rx_loop, &vdev->ud, "vhci_rx");
+	vdev->ud.tcp_tx = kthread_create(vhci_tx_loop, &vdev->ud, "vhci_tx");
 
 	vdev->ud.side   = USBIP_VHCI;
 	vdev->ud.status = VDEV_ST_NULL;
drivers/staging/usbip/vhci_rx.c +6 -10
index 8147d72..b03b277 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -235,22 +236,17 @@ static void vhci_rx_pdu(struct usbip_device *ud)
 
 /*-------------------------------------------------------------------------*/
 
-void vhci_rx_loop(struct usbip_task *ut)
+int vhci_rx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_rx);
-
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_vhci_rx("signal catched!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
 
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 
 		vhci_rx_pdu(ud);
 	}
-}
 
+	return 0;
+}
drivers/staging/usbip/vhci_sysfs.c +3 -6
index f6e34e0..3f2459f 100644
@@ -220,16 +220,13 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	vdev->ud.tcp_socket = socket;
 	vdev->ud.status     = VDEV_ST_NOTASSIGNED;
 
+	wake_up_process(vdev->ud.tcp_rx);
+	wake_up_process(vdev->ud.tcp_tx);
+
 	spin_unlock(&vdev->ud.lock);
 	spin_unlock(&the_controller->lock);
 	/* end the lock */
 
-	/*
-	 * this function will sleep, so should be out of the lock. but, it's ok
-	 * because we already marked vdev as being used. really?
-	 */
-	usbip_start_threads(&vdev->ud);
-
 	rh_port_connect(rhport, speed);
 
 	return count;
drivers/staging/usbip/vhci_tx.c +6 -8
index e1c1f71..6d065b9 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -215,17 +216,12 @@ static int vhci_send_cmd_unlink(struct vhci_device *vdev)
 
 /*-------------------------------------------------------------------------*/
 
-void vhci_tx_loop(struct usbip_task *ut)
+int vhci_tx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_tx);
+	struct usbip_device *ud = data;
 	struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
 
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_uinfo("vhci_tx signal catched\n");
-			break;
-		}
-
+	while (!kthread_should_stop()) {
 		if (vhci_send_cmd_submit(vdev) < 0)
 			break;
 
@@ -238,4 +234,6 @@ void vhci_tx_loop(struct usbip_task *ut)
 
 		usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
 	}
+
+	return 0;
 }
Signature · 1 line
1.7.1
#4 lore raw

Re: [PATCH 03/20] staging/usbip: convert to kthread

Hi Arnd,

Quoted 7 lines · Arnd Bergmann

usbip has its own infrastructure for managing kernel threads, similar to kthread. By changing it to use the standard functions, we can simplify the code and get rid of one of the last BKL users at the same time.

Compile-tested only, please verify.

I started reviewing and testing, but need to postpone proper testing until mid next week. I did notice a few problems (see below)

Quoted 18 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc Takahiro Hirofuchi <[email protected]>
---
 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 ++---
 13 files changed, 58 insertions(+), 197 deletions(-)

What is not to like :)

Quoted 45 lines
drivers/staging/usbip/Kconfig +1 -1
index b11ec37..2c1d10a 100644
@@ -1,6 +1,6 @@
 config USB_IP_COMMON
 	tristate "USB IP support (EXPERIMENTAL)"
-	depends on USB && NET && EXPERIMENTAL && BKL
+	depends on USB && NET && EXPERIMENTAL
 	default N
 	---help---
 	  This enables pushing USB packets over IP to allow remote
drivers/staging/usbip/stub.h +2 -2
index 30dbfb6..5b44e7b 100644
@@ -94,13 +94,13 @@ extern struct kmem_cache *stub_priv_cache;
 
 /* stub_tx.c */
 void stub_complete(struct urb *);
-void stub_tx_loop(struct usbip_task *);
+int stub_tx_loop(void *data);
 
 /* stub_dev.c */
 extern struct usb_driver stub_driver;
 
 /* stub_rx.c */
-void stub_rx_loop(struct usbip_task *);
+int stub_rx_loop(void *data);
 void stub_enqueue_ret_unlink(struct stub_device *, __u32, __u32);
 
 /* stub_main.c */
drivers/staging/usbip/stub_dev.c +1 -0
index b186b5f..4ee70d4 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"

drivers/staging/usbip/stub_dev.c static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, · line 143

Quoted 6 lines
drivers/staging/usbip/stub_dev.c +2 -1
@@ -138,7 +139,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,
 
 		spin_unlock(&sdev->ud.lock);
 
-		usbip_start_threads(&sdev->ud);
+		wake_up_process(sdev->ud.tcp_rx);
+		wake_up_process(sdev->ud.tcp_tx);

The threads may have exited already if the
device was in use then then shut down.

Can we create or kthread_run() the threads
here as I think happened before?

This is what I saw from a quick test:

[  405.674068] usbip 1-1:1.0: stub up
[  405.674110] general protection fault: 0000 [#1] SMP 
[  405.674876] last sysfs file: /sys/devices/pci0000:00/0000:00:01.2/usb1/1-1/1-1:1.0/usbip_sockfd
[  405.676045] CPU 0 
[  405.676045] Modules linked in: usbip(C) usbip_common_mod(C) snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device edd nfs lockd fscache nfs_acl auth_rpcgss sunrpc ipv6 af_packet mperf microcode configfs fuse loop dm_mod snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_timer tpm_tis tpm snd soundcore tpm_bios rtc_cmos rtc_core i2c_piix4 i2c_core virtio_net snd_page_alloc pcspkr rtc_lib floppy virtio_balloon button uhci_hcd ehci_hcd usbcore ext3 mbcache jbd fan processor virtio_blk virtio_pci virtio_ring virtio ide_pci_generic piix ide_core ata_generic ata_piix libata scsi_mod thermal thermal_sys hwmon
[  405.676045] 
[  405.676045] Pid: 3360, comm: usbipd Tainted: G         C  2.6.38-rc2-0.5-default+ #1 /Bochs
[  405.676045] RIP: 0010:[<ffffffff8104c41f>]  [<ffffffff8104c41f>] task_rq_lock+0x4f/0xb0
[  405.676045] RSP: 0018:ffff88003b687db8  EFLAGS: 00010006
[  405.676045] RAX: 6b6b6b6b6b6b6b6b RBX: 00000000001d54c0 RCX: 0000000000000001
[  405.676045] RDX: ffff880037da1310 RSI: 0000000000000000 RDI: ffffffff8104c41a
[  405.676045] RBP: ffff88003b687dd8 R08: 0000000000000000 R09: 0000000000000001
[  405.676045] R10: 0000000000000008 R11: 0000000000000001 R12: ffff880038fdd2d0
[  405.676045] R13: ffff88003b687e10 R14: 00000000001d54c0 R15: 000000000000000f
[  405.676045] FS:  00007f6a43816700(0000) GS:ffff88003e200000(0000) knlGS:0000000000000000
[  405.676045] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  405.676045] CR2: 00007fae487ab000 CR3: 000000003b439000 CR4: 00000000000006f0
[  405.676045] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  405.676045] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  405.676045] Process usbipd (pid: 3360, threadinfo ffff88003b686000, task ffff880037da1310)
[  405.676045] Stack:
[  405.676045]  ffff880038fdd2d0 ffff880039be54b0 0000000000000000 0000000000000000
[  405.676045]  ffff88003b687e48 ffffffff8105985c ffff880037da1380 ffffffffa013f0a2
[  405.676045]  ffff88003b687e38 0000000000000246 0000000000000002 0000000000000286
[  405.676045] Call Trace:
[  405.676045]  [<ffffffff8105985c>] try_to_wake_up+0x3c/0x430
[  405.676045]  [<ffffffffa013f0a2>] ? store_sockfd+0xa2/0x1a0 [usbip]
[  405.676045]  [<ffffffff81059ca5>] wake_up_process+0x15/0x20
[  405.676045]  [<ffffffffa013f0ab>] store_sockfd+0xab/0x1a0 [usbip]
[  405.676045]  [<ffffffff8130b390>] dev_attr_store+0x20/0x30
[  405.676045]  [<ffffffff811c4186>] sysfs_write_file+0xe6/0x170
[  405.676045]  [<ffffffff81156e80>] vfs_write+0xd0/0x1a0
[  405.676045]  [<ffffffff81157054>] sys_write+0x54/0xa0
[  405.676045]  [<ffffffff8100bf42>] system_call_fastpath+0x16/0x1b
Quoted 102 lines
drivers/staging/usbip/stub_dev.c +5 -4
 		spin_lock(&sdev->ud.lock);
 		sdev->ud.status = SDEV_ST_USED;
@@ -218,7 +220,8 @@ static void stub_shutdown_connection(struct usbip_device *ud)
 	}
 
 	/* 1. stop threads */
-	usbip_stop_threads(ud);
+	kthread_stop(ud->tcp_rx);
+	kthread_stop(ud->tcp_tx);
 
 	/* 2. close the socket */
 	/*
@@ -333,8 +336,8 @@ static struct stub_device *stub_device_alloc(struct usb_interface *interface)
 	 */
 	sdev->devid     = (busnum << 16) | devnum;
 
-	usbip_task_init(&sdev->ud.tcp_rx, "stub_rx", stub_rx_loop);
-	usbip_task_init(&sdev->ud.tcp_tx, "stub_tx", stub_tx_loop);
+	sdev->ud.tcp_rx = kthread_create(stub_rx_loop, &sdev->ud, "stub_rx");
+	sdev->ud.tcp_tx = kthread_create(stub_tx_loop, &sdev->ud, "stub_tx");
 
 	sdev->ud.side = USBIP_STUB;
 	sdev->ud.status = SDEV_ST_AVAILABLE;
@@ -537,7 +540,7 @@ static void stub_disconnect(struct usb_interface *interface)
 	stub_remove_files(&interface->dev);
 
 	/*If usb reset called from event handler*/
-	if (busid_priv->sdev->ud.eh.thread == current) {
+	if (busid_priv->sdev->ud.eh == current) {
 		busid_priv->interf_count--;
 		return;
 	}
drivers/staging/usbip/stub_rx.c +5 -8
index 3de6fd2..4ecc2bb 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -616,19 +617,15 @@ static void stub_rx_pdu(struct usbip_device *ud)
 
 }
 
-void stub_rx_loop(struct usbip_task *ut)
+int stub_rx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_rx);
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_stub_rx("signal caught!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 
 		stub_rx_pdu(ud);
 	}
+	return 0;
 }
drivers/staging/usbip/stub_tx.c +4 -8
index d7136e2..2477481 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"
@@ -333,17 +334,12 @@ static int stub_send_ret_unlink(struct stub_device *sdev)
 
 /*-------------------------------------------------------------------------*/
 
-void stub_tx_loop(struct usbip_task *ut)
+int stub_tx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_tx);
+	struct usbip_device *ud = data;
 	struct stub_device *sdev = container_of(ud, struct stub_device, ud);
 
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_stub_tx("signal catched\n");
-			break;
-		}
-
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 

drivers/staging/usbip/stub_tx.c void stub_tx_loop(struct usbip_task *ut) · line 369

Quoted 3 lines
drivers/staging/usbip/stub_tx.c +0 -0
@@ -371,4 +367,6 @@ void stub_tx_loop(struct usbip_task *ut)
 				(!list_empty(&sdev->priv_tx) ||
 				 !list_empty(&sdev->unlink_tx)));
 	}

I think this needs kthread_should_stop() as
condition for the wait_event_interruptible() or
the thread may not actually stop.

drivers/staging/usbip/stub_tx.c +2 -1
@@ -365,7 +365,8 @@ int stub_tx_loop(void *data)
 
                wait_event_interruptible(sdev->tx_waitq,
                                (!list_empty(&sdev->priv_tx) ||
-                                !list_empty(&sdev->unlink_tx)));
+                                !list_empty(&sdev->unlink_tx)) ||
+                               kthread_should_stop());
        }
 
        return 0;
Quoted 200 lines
drivers/staging/usbip/usbip_common.c +0 -105
index 210ef16..337abc4 100644
@@ -18,7 +18,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/smp_lock.h>
 #include <linux/file.h>
 #include <linux/tcp.h>
 #include <linux/in.h>
@@ -349,110 +348,6 @@ void usbip_dump_header(struct usbip_header *pdu)
 }
 EXPORT_SYMBOL_GPL(usbip_dump_header);
 
-
-/*-------------------------------------------------------------------------*/
-/* thread routines */
-
-int usbip_thread(void *param)
-{
-	struct usbip_task *ut = param;
-
-	if (!ut)
-		return -EINVAL;
-
-	lock_kernel();
-	daemonize(ut->name);
-	allow_signal(SIGKILL);
-	ut->thread = current;
-	unlock_kernel();
-
-	/* srv.rb must wait for rx_thread starting */
-	complete(&ut->thread_done);
-
-	/* start of while loop */
-	ut->loop_ops(ut);
-
-	/* end of loop */
-	ut->thread = NULL;
-
-	complete_and_exit(&ut->thread_done, 0);
-}
-
-static void stop_rx_thread(struct usbip_device *ud)
-{
-	if (ud->tcp_rx.thread != NULL) {
-		send_sig(SIGKILL, ud->tcp_rx.thread, 1);
-		wait_for_completion(&ud->tcp_rx.thread_done);
-		usbip_udbg("rx_thread for ud %p has finished\n", ud);
-	}
-}
-
-static void stop_tx_thread(struct usbip_device *ud)
-{
-	if (ud->tcp_tx.thread != NULL) {
-		send_sig(SIGKILL, ud->tcp_tx.thread, 1);
-		wait_for_completion(&ud->tcp_tx.thread_done);
-		usbip_udbg("tx_thread for ud %p has finished\n", ud);
-	}
-}
-
-int usbip_start_threads(struct usbip_device *ud)
-{
-	/*
-	 * threads are invoked per one device (per one connection).
-	 */
-	struct task_struct *th;
-	int err = 0;
-
-	th = kthread_run(usbip_thread, (void *)&ud->tcp_rx, "usbip");
-	if (IS_ERR(th)) {
-		printk(KERN_WARNING
-			"Unable to start control thread\n");
-		err = PTR_ERR(th);
-		goto ust_exit;
-	}
-
-	th = kthread_run(usbip_thread, (void *)&ud->tcp_tx, "usbip");
-	if (IS_ERR(th)) {
-		printk(KERN_WARNING
-			"Unable to start control thread\n");
-		err = PTR_ERR(th);
-		goto tx_thread_err;
-	}
-
-	/* confirm threads are starting */
-	wait_for_completion(&ud->tcp_rx.thread_done);
-	wait_for_completion(&ud->tcp_tx.thread_done);
-
-	return 0;
-
-tx_thread_err:
-	stop_rx_thread(ud);
-
-ust_exit:
-	return err;
-}
-EXPORT_SYMBOL_GPL(usbip_start_threads);
-
-void usbip_stop_threads(struct usbip_device *ud)
-{
-	/* kill threads related to this sdev, if v.c. exists */
-	stop_rx_thread(ud);
-	stop_tx_thread(ud);
-}
-EXPORT_SYMBOL_GPL(usbip_stop_threads);
-
-void usbip_task_init(struct usbip_task *ut, char *name,
-		void (*loop_ops)(struct usbip_task *))
-{
-	ut->thread = NULL;
-	init_completion(&ut->thread_done);
-	ut->name = name;
-	ut->loop_ops = loop_ops;
-}
-EXPORT_SYMBOL_GPL(usbip_task_init);
-
-
 /*-------------------------------------------------------------------------*/
 /* socket routines */
 
drivers/staging/usbip/usbip_common.h +3 -17
index d280e23..9f809c3 100644
@@ -307,13 +307,6 @@ void usbip_dump_header(struct usbip_header *pdu);
 
 struct usbip_device;
 
-struct usbip_task {
-	struct task_struct *thread;
-	struct completion thread_done;
-	char *name;
-	void (*loop_ops)(struct usbip_task *);
-};
-
 enum usbip_side {
 	USBIP_VHCI,
 	USBIP_STUB,
@@ -346,8 +339,8 @@ struct usbip_device {
 
 	struct socket *tcp_socket;
 
-	struct usbip_task tcp_rx;
-	struct usbip_task tcp_tx;
+	struct task_struct *tcp_rx;
+	struct task_struct *tcp_tx;
 
 	/* event handler */
 #define USBIP_EH_SHUTDOWN	(1 << 0)
@@ -367,7 +360,7 @@ struct usbip_device {
 #define	VDEV_EVENT_ERROR_MALLOC	(USBIP_EH_SHUTDOWN | USBIP_EH_UNUSABLE)
 
 	unsigned long event;
-	struct usbip_task eh;
+	struct task_struct *eh;
 	wait_queue_head_t eh_waitq;
 
 	struct eh_ops {
@@ -378,13 +371,6 @@ struct usbip_device {
 };
 
 
-void usbip_task_init(struct usbip_task *ut, char *,
-				void (*loop_ops)(struct usbip_task *));
-
-int usbip_start_threads(struct usbip_device *ud);
-void usbip_stop_threads(struct usbip_device *ud);
-int usbip_thread(void *param);
-
 void usbip_pack_pdu(struct usbip_header *pdu, struct urb *urb, int cmd,
 								int pack);
 
drivers/staging/usbip/usbip_event.c +3 -8
index af3832b..89aecec 100644
@@ -62,16 +62,11 @@ static int event_handler(struct usbip_device *ud)
 	return 0;
 }
 
-static void event_handler_loop(struct usbip_task *ut)
+static int event_handler_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, eh);
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_eh("signal catched!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
+	while (!kthread_should_stop()) {
 		if (event_handler(ud) < 0)
 			break;
 

drivers/staging/usbip/usbip_event.c static void event_handler_loop(struct usbip_task *ut) · line 78

Quoted 5 lines
drivers/staging/usbip/usbip_event.c +1 -0
@@ -79,38 +74,30 @@ static void event_handler_loop(struct usbip_task *ut)
 					usbip_event_happened(ud));
 		usbip_dbg_eh("wakeup\n");
 	}
+	return 0;
 }

Same here, I think this is needed:

drivers/staging/usbip/usbip_event.c +3 -1
@@ -71,7 +71,9 @@ static int event_handler_loop(void *data)
                        break;
 
                wait_event_interruptible(ud->eh_waitq,
-                                       usbip_event_happened(ud));
+                                       usbip_event_happened(ud) ||
+                                       kthread_should_stop());
+
                usbip_dbg_eh("wakeup\n");
        }
        return 0;

The event handler also needs to get a chance to
run the shutdown functions before it exists.

This was implicitly the case before.

@@ -67,14 +67,14 @@ static int event_handler_loop(void *data)

        struct usbip_device *ud = data;
 
        while (!kthread_should_stop()) {
-               if (event_handler(ud) < 0)
-                       break;
-
                wait_event_interruptible(ud->eh_waitq,
                                        usbip_event_happened(ud) ||
                                        kthread_should_stop());
 
                usbip_dbg_eh("wakeup\n");
+
+               if (event_handler(ud) < 0)
+                       break;
        }
        return 0;

}

Quoted 133 lines
drivers/staging/usbip/usbip_event.c +5 -14
 int usbip_start_eh(struct usbip_device *ud)
 {
-	struct usbip_task *eh = &ud->eh;
-	struct task_struct *th;
-
 	init_waitqueue_head(&ud->eh_waitq);
 	ud->event = 0;
 
-	usbip_task_init(eh, "usbip_eh", event_handler_loop);
-
-	th = kthread_run(usbip_thread, (void *)eh, "usbip");
-	if (IS_ERR(th)) {
+	ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
+	if (IS_ERR(ud->eh)) {
 		printk(KERN_WARNING
 			"Unable to start control thread\n");
-		return PTR_ERR(th);
+		return PTR_ERR(ud->eh);
 	}
-
-	wait_for_completion(&eh->thread_done);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usbip_start_eh);
 
 void usbip_stop_eh(struct usbip_device *ud)
 {
-	struct usbip_task *eh = &ud->eh;
-
-	if (eh->thread == current)
+	if (ud->eh == current)
 		return; /* do not wait for myself */
 
-	wait_for_completion(&eh->thread_done);
+	kthread_stop(ud->eh);
 	usbip_dbg_eh("usbip_eh has finished\n");
 }
 EXPORT_SYMBOL_GPL(usbip_stop_eh);
drivers/staging/usbip/vhci.h +2 -2
index 41a1fe5..ed51983 100644
@@ -116,8 +116,8 @@ extern struct attribute_group dev_attr_group;
 /* vhci_hcd.c */
 void rh_port_connect(int rhport, enum usb_device_speed speed);
 void rh_port_disconnect(int rhport);
-void vhci_rx_loop(struct usbip_task *ut);
-void vhci_tx_loop(struct usbip_task *ut);
+int vhci_rx_loop(void *data);
+int vhci_tx_loop(void *data);
 
 #define hardware		(&the_controller->pdev.dev)
 
drivers/staging/usbip/vhci_hcd.c +7 -3
index 08bd26a..f048b47 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -840,7 +841,10 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
 		kernel_sock_shutdown(ud->tcp_socket, SHUT_RDWR);
 	}
 
-	usbip_stop_threads(&vdev->ud);
+	/* kill threads related to this sdev, if v.c. exists */
+	kthread_stop(vdev->ud.tcp_rx);
+	kthread_stop(vdev->ud.tcp_tx);
+
 	usbip_uinfo("stop threads\n");
 
 	/* active connection is closed */
@@ -907,8 +911,8 @@ static void vhci_device_init(struct vhci_device *vdev)
 {
 	memset(vdev, 0, sizeof(*vdev));
 
-	usbip_task_init(&vdev->ud.tcp_rx, "vhci_rx", vhci_rx_loop);
-	usbip_task_init(&vdev->ud.tcp_tx, "vhci_tx", vhci_tx_loop);
+	vdev->ud.tcp_rx = kthread_create(vhci_rx_loop, &vdev->ud, "vhci_rx");
+	vdev->ud.tcp_tx = kthread_create(vhci_tx_loop, &vdev->ud, "vhci_tx");
 
 	vdev->ud.side   = USBIP_VHCI;
 	vdev->ud.status = VDEV_ST_NULL;
drivers/staging/usbip/vhci_rx.c +6 -10
index 8147d72..b03b277 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -235,22 +236,17 @@ static void vhci_rx_pdu(struct usbip_device *ud)
 
 /*-------------------------------------------------------------------------*/
 
-void vhci_rx_loop(struct usbip_task *ut)
+int vhci_rx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_rx);
-
-
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_vhci_rx("signal catched!\n");
-			break;
-		}
+	struct usbip_device *ud = data;
 
 
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
 
 		vhci_rx_pdu(ud);
 	}
-}
 
+	return 0;
+}
drivers/staging/usbip/vhci_sysfs.c +0 -0
index f6e34e0..3f2459f 100644

drivers/staging/usbip/vhci_sysfs.c static ssize_t store_attach(struct device *dev, struct device_attribute *attr, · line 228

Quoted 9 lines
drivers/staging/usbip/vhci_sysfs.c +3 -0
@@ -220,16 +220,13 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	vdev->ud.tcp_socket = socket;
 	vdev->ud.status     = VDEV_ST_NOTASSIGNED;
 
+	wake_up_process(vdev->ud.tcp_rx);
+	wake_up_process(vdev->ud.tcp_tx);
+
 	spin_unlock(&vdev->ud.lock);
 	spin_unlock(&the_controller->lock);
 	/* end the lock */

Is it ok to call wake_up_process() while holding
the spinlocks? (I don't know - just noticed the
comment which used to be there)

Quoted 32 lines
drivers/staging/usbip/vhci_sysfs.c +0 -6
-	/*
-	 * this function will sleep, so should be out of the lock. but, it's ok
-	 * because we already marked vdev as being used. really?
-	 */
-	usbip_start_threads(&vdev->ud);
-
 	rh_port_connect(rhport, speed);
 
 	return count;
drivers/staging/usbip/vhci_tx.c +3 -2
index e1c1f71..6d065b9 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "vhci.h"
@@ -215,17 +216,12 @@ static int vhci_send_cmd_unlink(struct vhci_device *vdev)
 
 /*-------------------------------------------------------------------------*/
 
-void vhci_tx_loop(struct usbip_task *ut)
+int vhci_tx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_tx);
+	struct usbip_device *ud = data;
 	struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
 

drivers/staging/usbip/vhci_tx.c static int vhci_send_cmd_unlink(struct vhci_device *vdev) · line 227

Quoted 10 lines
drivers/staging/usbip/vhci_tx.c +1 -6
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_uinfo("vhci_tx signal catched\n");
-			break;
-		}
-
+	while (!kthread_should_stop()) {
 		if (vhci_send_cmd_submit(vdev) < 0)
 			break;
 

vhci_tx_loop() also needs the kthread_should_stop
in its wait_event_interruptible, I think.

drivers/staging/usbip/vhci_tx.c +2 -1
@@ -230,7 +230,8 @@ int vhci_tx_loop(void *data)
 
                wait_event_interruptible(vdev->waitq_tx,
                                (!list_empty(&vdev->priv_tx) ||
-                                !list_empty(&vdev->unlink_tx)));
+                                !list_empty(&vdev->unlink_tx)) ||
+                               kthread_should_stop());
 
                usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
        }

drivers/staging/usbip/vhci_tx.c void vhci_tx_loop(struct usbip_task *ut) · line 239

Quoted 7 lines
drivers/staging/usbip/vhci_tx.c +2 -0
@@ -238,4 +234,6 @@ void vhci_tx_loop(struct usbip_task *ut)
 
 		usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
 	}
+
+	return 0;
 }

I need to leave now for the next couple of days,
so this is a bit rushed.

I can take a closer look and do tests in different setups during the next week.

Max

#5 lore raw

Re: [PATCH 03/20] staging/usbip: convert to kthread

Quoted 9 lines · On Friday 28 January 2011 18:53:48 Max Vozeler wrote:

Hi Arnd,

Quoted 6 lines · Arnd Bergmann

usbip has its own infrastructure for managing kernel threads, similar to kthread. By changing it to use the standard functions, we can simplify the code and get rid of one of the last BKL users at the same time.

Quoted 5 lines
Quoted 1 line

Compile-tested only, please verify.

I started reviewing and testing, but need to postpone proper testing until mid next week. I did notice a few problems (see below)

Ok, thanks for the review!

Quoted 23 lines
Quoted 13 lines
drivers/staging/usbip/stub_dev.c +1 -0
index b186b5f..4ee70d4 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/slab.h>
+#include <linux/kthread.h>
 
 #include "usbip_common.h"
 #include "stub.h"

drivers/staging/usbip/stub_dev.c static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, · line 143

Quoted 6 lines
drivers/staging/usbip/stub_dev.c +2 -1
@@ -138,7 +139,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,
 
 		spin_unlock(&sdev->ud.lock);
 
-		usbip_start_threads(&sdev->ud);
+		wake_up_process(sdev->ud.tcp_rx);
+		wake_up_process(sdev->ud.tcp_tx);

The threads may have exited already if the device was in use then then shut down.

Quoted 2 lines

Can we create or kthread_run() the threads here as I think happened before?

Certainly. I just tried to find the semantics that match the previous behaviour the closest, but I could not find a reason why the threads were started in a different place from where they get created.

Quoted 10 lines

This is what I saw from a quick test:

[  405.674068] usbip 1-1:1.0: stub up
[  405.674110] general protection fault: 0000 [#1] SMP 
[  405.674876] last sysfs file: /sys/devices/pci0000:00/0000:00:01.2/usb1/1-1/1-1:1.0/usbip_sockfd
[  405.676045] CPU 0 
[  405.676045] Modules linked in: usbip(C) usbip_common_mod(C) snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device edd nfs lockd fscache nfs_acl auth_rpcgss sunrpc ipv6 af_packet mperf microcode configfs fuse loop dm_mod snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_timer tpm_tis tpm snd soundcore tpm_bios rtc_cmos rtc_core i2c_piix4 i2c_core virtio_net snd_page_alloc pcspkr rtc_lib floppy virtio_balloon button uhci_hcd ehci_hcd usbcore ext3 mbcache jbd fan processor virtio_blk virtio_pci virtio_ring virtio ide_pci_generic piix ide_core ata_generic ata_piix libata scsi_mod thermal thermal_sys hwmon
[  405.676045] 
[  405.676045] Pid: 3360, comm: usbipd Tainted: G         C  2.6.38-rc2-0.5-default+ #1 /Bochs
[  405.676045] RIP: 0010:[<ffffffff8104c41f>]  [<ffffffff8104c41f>] task_rq_lock+0x4f/0xb0

Right, that would fit the problem you described. It could also happen if tcp_rx is NULL at this point.

Quoted 17 lines
Quoted 17 lines
drivers/staging/usbip/stub_dev.c +2 -2
-void stub_tx_loop(struct usbip_task *ut)
+int stub_tx_loop(void *data)
 {
-	struct usbip_device *ud = container_of(ut, struct usbip_device, tcp_tx);
+	struct usbip_device *ud = data;
drivers/staging/usbip/stub_dev.c +1 -6
 	struct stub_device *sdev = container_of(ud, struct stub_device, ud);
 
-	while (1) {
-		if (signal_pending(current)) {
-			usbip_dbg_stub_tx("signal catched\n");
-			break;
-		}
-
+	while (!kthread_should_stop()) {
 		if (usbip_event_happened(ud))
 			break;
Quoted 8 lines

drivers/staging/usbip/stub_dev.c void stub_tx_loop(struct usbip_task *ut) · line 369

Quoted 4 lines
drivers/staging/usbip/stub_dev.c +0 -0
@@ -371,4 +367,6 @@ void stub_tx_loop(struct usbip_task *ut)
 				(!list_empty(&sdev->priv_tx) ||
 				 !list_empty(&sdev->unlink_tx)));
 	}

I think this needs kthread_should_stop() as condition for the wait_event_interruptible() or the thread may not actually stop.

Yes, good point.

drivers/staging/usbip/stub_tx.c int stub_tx_loop(void *data) · line 372

Quoted 12 lines
drivers/staging/usbip/stub_tx.c +2 -1
@@ -365,7 +365,8 @@ int stub_tx_loop(void *data)
 
                wait_event_interruptible(sdev->tx_waitq,
                                (!list_empty(&sdev->priv_tx) ||
-                                !list_empty(&sdev->unlink_tx)));
+                                !list_empty(&sdev->unlink_tx)) ||
+                               kthread_should_stop());
        }
 
        return 0;

Looks good, same for the others you found.

Quoted 8 lines
Quoted 8 lines
drivers/staging/usbip/vhci_sysfs.c +0 -0
index f6e34e0..3f2459f 100644
@@ -220,16 +220,13 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 	vdev->ud.tcp_socket = socket;
 	vdev->ud.status     = VDEV_ST_NOTASSIGNED;
 
Quoted 10 lines

drivers/staging/usbip/vhci_sysfs.c static ssize_t store_attach(struct device *dev, struct device_attribute *attr, · line 228

Quoted 6 lines
drivers/staging/usbip/vhci_sysfs.c +3 -0
+	wake_up_process(vdev->ud.tcp_rx);
+	wake_up_process(vdev->ud.tcp_tx);
+
 	spin_unlock(&vdev->ud.lock);
 	spin_unlock(&the_controller->lock);
 	/* end the lock */

Is it ok to call wake_up_process() while holding the spinlocks? (I don't know - just noticed the comment which used to be there)

I'm pretty sure you can call it almost everywhere, yes.

Quoted 11 lines

drivers/staging/usbip/vhci_sysfs.c void vhci_tx_loop(struct usbip_task *ut) · line 239

Quoted 7 lines
drivers/staging/usbip/vhci_sysfs.c +2 -0
@@ -238,4 +234,6 @@ void vhci_tx_loop(struct usbip_task *ut)
 
 		usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
 	}
+
+	return 0;
 }

I need to leave now for the next couple of days, so this is a bit rushed.

Quoted 2 lines

I can take a closer look and do tests in different setups during the next week.

That would be great!

Thanks,

Arnd

#6 lore raw

Re: [PATCH 03/20] staging/usbip: convert to kthread

In reply to Max Vozeler
Quoted 6 lines · On Friday 28 January 2011 18:53:48 Max Vozeler wrote:

I need to leave now for the next couple of days,
so this is a bit rushed.

I can take a closer look and do tests in different setups during the next week.

It seems we both forgot about this. This is what I now added to my tree. Greg, I'll resubmit a folded version with the original patch.

Arnd
8<------

staging/usbip: fix breakage from BKL removal

wait_event_interruptible() in a kthread needs
to check if the kthread should stop.

Thread creation was racy, calling kthread_run
instead of wake_up on an existing kthread
fixes this.

---

drivers/staging/usbip/stub_dev.c +2 -5
index f2c6148..8214c35 100644
@@ -139,8 +139,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,
 
 		spin_unlock(&sdev->ud.lock);
 
-		wake_up_process(sdev->ud.tcp_rx);
-		wake_up_process(sdev->ud.tcp_tx);
+		sdev->ud.tcp_rx = kthread_run(stub_rx_loop, &sdev->ud, "stub_rx");
+		sdev->ud.tcp_tx = kthread_run(stub_tx_loop, &sdev->ud, "stub_tx");
 
 		spin_lock(&sdev->ud.lock);
 		sdev->ud.status = SDEV_ST_USED;
@@ -339,9 +339,6 @@ static struct stub_device *stub_device_alloc(struct usb_device *udev,
 	 */
 	sdev->devid     = (busnum << 16) | devnum;
 
-	sdev->ud.tcp_rx = kthread_create(stub_rx_loop, &sdev->ud, "stub_rx");
-	sdev->ud.tcp_tx = kthread_create(stub_tx_loop, &sdev->ud, "stub_tx");
-
 	sdev->ud.side = USBIP_STUB;
 	sdev->ud.status = SDEV_ST_AVAILABLE;
 	/* sdev->ud.lock = SPIN_LOCK_UNLOCKED; */
drivers/staging/usbip/stub_tx.c +2 -1
index 2477481..5523f25 100644
@@ -365,7 +365,8 @@ int stub_tx_loop(void *data)
 
 		wait_event_interruptible(sdev->tx_waitq,
 				(!list_empty(&sdev->priv_tx) ||
-				 !list_empty(&sdev->unlink_tx)));
+				 !list_empty(&sdev->unlink_tx) ||
+				 kthread_should_stop()));
 	}
 
 	return 0;
drivers/staging/usbip/usbip_event.c +5 -4
index 89aecec..f4b287e 100644
@@ -67,12 +67,13 @@ static int event_handler_loop(void *data)
 	struct usbip_device *ud = data;
 
 	while (!kthread_should_stop()) {
-		if (event_handler(ud) < 0)
-			break;
-
 		wait_event_interruptible(ud->eh_waitq,
-					usbip_event_happened(ud));
+					usbip_event_happened(ud) ||
+					kthread_should_stop());
 		usbip_dbg_eh("wakeup\n");
+
+		if (event_handler(ud) < 0)
+			break;
 	}
 	return 0;
 }
drivers/staging/usbip/vhci_tx.c +2 -1
index 6d065b9..d9ab49d 100644
@@ -230,7 +230,8 @@ int vhci_tx_loop(void *data)
 
 		wait_event_interruptible(vdev->waitq_tx,
 				(!list_empty(&vdev->priv_tx) ||
-				 !list_empty(&vdev->unlink_tx)));
+				 !list_empty(&vdev->unlink_tx) ||
+				 kthread_should_stop()));
 
 		usbip_dbg_vhci_tx("pending urbs ?, now wake up\n");
 	}
#7 lore raw

PATCH 4/20 staging/cx25721: serialize access to devlist

In reply to Arnd Bergmann

Out of the three files accessing the device list, one uses a mutex, one uses the BKL and one does not have any locking. That is of course pointless, so let's make all of them use the same mutex, and get rid of one more BKL user.

Cc Mauro Carvalho Chehab <[email protected]>
Cc Palash Bandyopadhyay <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 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 ++-
 5 files changed, 15 insertions(+), 16 deletions(-)
drivers/staging/cx25821/Kconfig +0 -1
index b265695..5f6b542 100644
@@ -1,7 +1,6 @@
 config VIDEO_CX25821
 	tristate "Conexant cx25821 support"
 	depends on DVB_CORE && VIDEO_DEV && PCI && I2C
-	depends on BKL # please fix
 	select I2C_ALGOBIT
 	select VIDEO_BTCX
 	select VIDEO_TVEEPROM
drivers/staging/cx25821/cx25821-alsa.c +2 -0
index 160f669..ebdba7c 100644
@@ -770,10 +770,12 @@ static int cx25821_alsa_init(void)
 	struct cx25821_dev *dev = NULL;
 	struct list_head *list;
 
+	mutex_lock(&cx25821_devlist_mutex);
 	list_for_each(list, &cx25821_devlist) {
 		dev = list_entry(list, struct cx25821_dev, devlist);
 		cx25821_audio_initdev(dev);
 	}
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	if (dev == NULL)
 		pr_info("ERROR ALSA: no cx25821 cards found\n");
drivers/staging/cx25821/cx25821-core.c +7 -9
index a216b62..523ac5e 100644
@@ -33,9 +33,6 @@ MODULE_DESCRIPTION("Driver for Athena cards");
 MODULE_AUTHOR("Shu Lin - Hiep Huynh");
 MODULE_LICENSE("GPL");
 
-struct list_head cx25821_devlist;
-EXPORT_SYMBOL(cx25821_devlist);
-
 static unsigned int debug;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "enable debug messages");
@@ -46,8 +43,10 @@ MODULE_PARM_DESC(card, "card type");
 
 static unsigned int cx25821_devcount;
 
-static DEFINE_MUTEX(devlist);
+DEFINE_MUTEX(cx25821_devlist_mutex);
+EXPORT_SYMBOL(cx25821_devlist_mutex);
 LIST_HEAD(cx25821_devlist);
+EXPORT_SYMBOL(cx25821_devlist);
 
 struct sram_channel cx25821_sram_channels[] = {
 	[SRAM_CH00] = {
@@ -911,9 +910,9 @@ static int cx25821_dev_setup(struct cx25821_dev *dev)
 	dev->nr = ++cx25821_devcount;
 	sprintf(dev->name, "cx25821[%d]", dev->nr);
 
-	mutex_lock(&devlist);
+	mutex_lock(&cx25821_devlist_mutex);
 	list_add_tail(&dev->devlist, &cx25821_devlist);
-	mutex_unlock(&devlist);
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	strcpy(cx25821_boards[UNKNOWN_BOARD].name, "unknown");
 	strcpy(cx25821_boards[CX25821_BOARD].name, "cx25821");
@@ -1465,9 +1464,9 @@ static void __devexit cx25821_finidev(struct pci_dev *pci_dev)
 	if (pci_dev->irq)
 		free_irq(pci_dev->irq, dev);
 
-	mutex_lock(&devlist);
+	mutex_lock(&cx25821_devlist_mutex);
 	list_del(&dev->devlist);
-	mutex_unlock(&devlist);
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	cx25821_dev_unregister(dev);
 	v4l2_device_unregister(v4l2_dev);
@@ -1501,7 +1500,6 @@ static struct pci_driver cx25821_pci_driver = {
 
 static int __init cx25821_init(void)
 {
-	INIT_LIST_HEAD(&cx25821_devlist);
 	pr_info("driver version %d.%d.%d loaded\n",
 		(CX25821_VERSION_CODE >> 16) & 0xff,
 		(CX25821_VERSION_CODE >> 8) & 0xff,
drivers/staging/cx25821/cx25821-video.c +4 -5
index 0d8d756..ab05392 100644
@@ -27,7 +27,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include "cx25821-video.h"
-#include <linux/smp_lock.h>
 
 MODULE_DESCRIPTION("v4l2 driver module for cx25821 based TV cards");
 MODULE_AUTHOR("Hiep Huynh <[email protected]>");
@@ -815,7 +814,7 @@ static int video_open(struct file *file)
        if (NULL == fh)
 	       return -ENOMEM;
 
-       lock_kernel();
+	mutex_lock(&cx25821_devlist_mutex);
 
        list_for_each(list, &cx25821_devlist)
        {
@@ -832,8 +831,8 @@ static int video_open(struct file *file)
        }
 
        if (NULL == dev) {
-	       unlock_kernel();
-	       return -ENODEV;
+		mutex_unlock(&cx25821_devlist_mutex);
+		return -ENODEV;
        }
 
        file->private_data = fh;
@@ -862,7 +861,7 @@ static int video_open(struct file *file)
 			      sizeof(struct cx25821_buffer), fh, NULL);
 
        dprintk(1, "post videobuf_queue_init()\n");
-       unlock_kernel();
+	mutex_unlock(&cx25821_devlist_mutex);
 
        return 0;
 }
drivers/staging/cx25821/cx25821.h +2 -1
index 5511523..6230243 100644
@@ -31,7 +31,6 @@
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/kdev_t.h>
-#include <linux/smp_lock.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
@@ -445,6 +444,8 @@ static inline struct cx25821_dev *get_cx25821(struct v4l2_device *v4l2_dev)
 	v4l2_device_call_all(&dev->v4l2_dev, 0, o, f, ##args)
 
 extern struct list_head cx25821_devlist;
+extern struct mutex cx25821_devlist_mutex;
+
 extern struct cx25821_board cx25821_boards[];
 extern struct cx25821_subid cx25821_subids[];
 
Signature · 1 line
1.7.1
#8 lore raw

RE: [PATCH 04/20] staging/cx25721: serialize access to devlist

Thanks! Looks ok to me.

Signed off by Palash Bandyopadhyay

-----Original Message-----
From: Arnd Bergmann [mailto:[email protected]]
Sent: Tuesday, January 25, 2011 2:17 PM
To: [email protected]
Cc: Arnd Bergmann; Mauro Carvalho Chehab; Palash Bandyopadhyay; Greg Kroah-Hartman
Subject: [PATCH 04/20] staging/cx25721: serialize access to devlist

Out of the three files accessing the device list, one uses a mutex, one uses the BKL and one does not have any locking. That is of course pointless, so let's make all of them use the same mutex, and get rid of one more BKL user.

Cc Mauro Carvalho Chehab <[email protected]>
Cc Palash Bandyopadhyay <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 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 ++-
 5 files changed, 15 insertions(+), 16 deletions(-)
drivers/staging/cx25821/Kconfig +0 -1
index b265695..5f6b542 100644
@@ -1,7 +1,6 @@
 config VIDEO_CX25821
 	tristate "Conexant cx25821 support"
 	depends on DVB_CORE && VIDEO_DEV && PCI && I2C
-	depends on BKL # please fix
 	select I2C_ALGOBIT
 	select VIDEO_BTCX
 	select VIDEO_TVEEPROM
drivers/staging/cx25821/cx25821-alsa.c +2 -0
index 160f669..ebdba7c 100644
@@ -770,10 +770,12 @@ static int cx25821_alsa_init(void)
 	struct cx25821_dev *dev = NULL;
 	struct list_head *list;
 
+	mutex_lock(&cx25821_devlist_mutex);
 	list_for_each(list, &cx25821_devlist) {
 		dev = list_entry(list, struct cx25821_dev, devlist);
 		cx25821_audio_initdev(dev);
 	}
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	if (dev == NULL)
 		pr_info("ERROR ALSA: no cx25821 cards found\n");
drivers/staging/cx25821/cx25821-core.c +7 -9
index a216b62..523ac5e 100644
@@ -33,9 +33,6 @@ MODULE_DESCRIPTION("Driver for Athena cards");
 MODULE_AUTHOR("Shu Lin - Hiep Huynh");
 MODULE_LICENSE("GPL");
 
-struct list_head cx25821_devlist;
-EXPORT_SYMBOL(cx25821_devlist);
-
 static unsigned int debug;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "enable debug messages");
@@ -46,8 +43,10 @@ MODULE_PARM_DESC(card, "card type");
 
 static unsigned int cx25821_devcount;
 
-static DEFINE_MUTEX(devlist);
+DEFINE_MUTEX(cx25821_devlist_mutex);
+EXPORT_SYMBOL(cx25821_devlist_mutex);
 LIST_HEAD(cx25821_devlist);
+EXPORT_SYMBOL(cx25821_devlist);
 
 struct sram_channel cx25821_sram_channels[] = {
 	[SRAM_CH00] = {
@@ -911,9 +910,9 @@ static int cx25821_dev_setup(struct cx25821_dev *dev)
 	dev->nr = ++cx25821_devcount;
 	sprintf(dev->name, "cx25821[%d]", dev->nr);
 
-	mutex_lock(&devlist);
+	mutex_lock(&cx25821_devlist_mutex);
 	list_add_tail(&dev->devlist, &cx25821_devlist);
-	mutex_unlock(&devlist);
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	strcpy(cx25821_boards[UNKNOWN_BOARD].name, "unknown");
 	strcpy(cx25821_boards[CX25821_BOARD].name, "cx25821");
@@ -1465,9 +1464,9 @@ static void __devexit cx25821_finidev(struct pci_dev *pci_dev)
 	if (pci_dev->irq)
 		free_irq(pci_dev->irq, dev);
 
-	mutex_lock(&devlist);
+	mutex_lock(&cx25821_devlist_mutex);
 	list_del(&dev->devlist);
-	mutex_unlock(&devlist);
+	mutex_unlock(&cx25821_devlist_mutex);
 
 	cx25821_dev_unregister(dev);
 	v4l2_device_unregister(v4l2_dev);
@@ -1501,7 +1500,6 @@ static struct pci_driver cx25821_pci_driver = {
 
 static int __init cx25821_init(void)
 {
-	INIT_LIST_HEAD(&cx25821_devlist);
 	pr_info("driver version %d.%d.%d loaded\n",
 		(CX25821_VERSION_CODE >> 16) & 0xff,
 		(CX25821_VERSION_CODE >> 8) & 0xff,
drivers/staging/cx25821/cx25821-video.c +4 -5
index 0d8d756..ab05392 100644
@@ -27,7 +27,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include "cx25821-video.h"
-#include <linux/smp_lock.h>
 
 MODULE_DESCRIPTION("v4l2 driver module for cx25821 based TV cards");
 MODULE_AUTHOR("Hiep Huynh <[email protected]>");
@@ -815,7 +814,7 @@ static int video_open(struct file *file)
        if (NULL == fh)
 	       return -ENOMEM;
 
-       lock_kernel();
+	mutex_lock(&cx25821_devlist_mutex);
 
        list_for_each(list, &cx25821_devlist)
        {
@@ -832,8 +831,8 @@ static int video_open(struct file *file)
        }
 
        if (NULL == dev) {
-	       unlock_kernel();
-	       return -ENODEV;
+		mutex_unlock(&cx25821_devlist_mutex);
+		return -ENODEV;
        }
 
        file->private_data = fh;
@@ -862,7 +861,7 @@ static int video_open(struct file *file)
 			      sizeof(struct cx25821_buffer), fh, NULL);
 
        dprintk(1, "post videobuf_queue_init()\n");
-       unlock_kernel();
+	mutex_unlock(&cx25821_devlist_mutex);
 
        return 0;
 }
drivers/staging/cx25821/cx25821.h +2 -1
index 5511523..6230243 100644
@@ -31,7 +31,6 @@
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/kdev_t.h>
-#include <linux/smp_lock.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
@@ -445,6 +444,8 @@ static inline struct cx25821_dev *get_cx25821(struct v4l2_device *v4l2_dev)
 	v4l2_device_call_all(&dev->v4l2_dev, 0, o, f, ##args)
 
 extern struct list_head cx25821_devlist;
+extern struct mutex cx25821_devlist_mutex;
+
 extern struct cx25821_board cx25821_boards[];
 extern struct cx25821_subid cx25821_subids[];
 
Signature · 12 lines
1.7.1


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." 

********************************************************************** 

---------------------------------------------------------------------
Acked-by Greg Kroah-Hartman on v1 4/20
#9 lore raw

Re: [PATCH 04/20] staging/cx25721: serialize access to devlist

In reply to Arnd Bergmann
Quoted 8 lines · Arnd Bergmann

Out of the three files accessing the device list, one uses a mutex, one uses the BKL and one does not have any locking. That is of course pointless, so let's make all of them use the same mutex, and get rid of one more BKL user.

Cc Mauro Carvalho Chehab <[email protected]>

Mauro, you will take this one, right? If so, please add:

to it.

If not, please let me know and I will take it myself.

thanks,

greg k-h

#10 lore raw

PATCH 5/20 staging/go7007: remove the BKL

In reply to Arnd Bergmann

There is nothing that the BKL can possibly
protect here, so just remove it.

Cc Greg Kroah-Hartman <[email protected]>
Cc Ross Cohen <[email protected]>
---
 drivers/staging/go7007/Kconfig        |    1 -
 drivers/staging/go7007/s2250-loader.c |    3 ---
 2 files changed, 0 insertions(+), 4 deletions(-)
drivers/staging/go7007/Kconfig +0 -1
index 1da57df..7dfb281 100644
@@ -1,7 +1,6 @@
 config VIDEO_GO7007
 	tristate "WIS GO7007 MPEG encoder support"
 	depends on VIDEO_DEV && PCI && I2C
-	depends on BKL # please fix
 	depends on SND
 	select VIDEOBUF_DMA_SG
 	depends on RC_CORE
drivers/staging/go7007/s2250-loader.c +0 -3
index 7547a8f..4e13251 100644
@@ -18,7 +18,6 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/usb.h>
 #include <dvb-usb.h>
 
@@ -142,11 +141,9 @@ static void s2250loader_disconnect(struct usb_interface *interface)
 {
 	pdevice_extension_t s;
 	printk(KERN_INFO "s2250: disconnect\n");
-	lock_kernel();
 	s = usb_get_intfdata(interface);
 	usb_set_intfdata(interface, NULL);
 	kref_put(&(s->kref), s2250loader_delete);
-	unlock_kernel();
 }
 
 static const struct usb_device_id s2250loader_ids[] = {
Signature · 1 line
1.7.1
#11 lore raw

PATCH 6/20 staging: Remove autofs3

In reply to Arnd Bergmann

autofs3 was moved to staging in 2.6.37, so we can
remove it in the 2.6.39 merge window. If we have
a reason to bring it back after that, this patch
can get reverted.

Cc Greg Kroah-Hartman <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ian Kent <[email protected]>
---
 MAINTAINERS                       |    6 -
 drivers/staging/Kconfig           |    2 -
 drivers/staging/Makefile          |    1 -
 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 ------------
 13 files changed, 0 insertions(+), 1690 deletions(-)
 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
MAINTAINERS +0 -6
index 55592f8..246ee22 100644
@@ -3563,12 +3563,6 @@ W:	http://lse.sourceforge.net/kdump/
 S:	Maintained
 F:	Documentation/kdump/
 
-KERNEL AUTOMOUNTER (AUTOFS)
-M:	"H. Peter Anvin" <[email protected]>
-L:	[email protected]
-S:	Obsolete
-F:	drivers/staging/autofs/
-
 KERNEL AUTOMOUNTER v4 (AUTOFS4)
 M:	Ian Kent <[email protected]>
 L:	[email protected]
drivers/staging/Kconfig +0 -2
index 5c8fcfc..3957b2f 100644
@@ -91,8 +91,6 @@ source "drivers/staging/frontier/Kconfig"
 
 source "drivers/staging/pohmelfs/Kconfig"
 
-source "drivers/staging/autofs/Kconfig"
-
 source "drivers/staging/phison/Kconfig"
 
 source "drivers/staging/line6/Kconfig"
drivers/staging/Makefile +0 -1
index d538863..cbe013e 100644
@@ -30,7 +30,6 @@ obj-$(CONFIG_R8712U)		+= rtl8712/
 obj-$(CONFIG_SPECTRA)		+= spectra/
 obj-$(CONFIG_TRANZPORT)		+= frontier/
 obj-$(CONFIG_POHMELFS)		+= pohmelfs/
-obj-$(CONFIG_AUTOFS_FS)		+= autofs/
 obj-$(CONFIG_IDE_PHISON)	+= phison/
 obj-$(CONFIG_LINE6_USB)		+= line6/
 obj-$(CONFIG_USB_SERIAL_QUATECH2)	+= serqt_usb2/
drivers/staging/autofs/Kconfig deleted +0 -22
deleted file mode 100644
index 480e210..0000000
@@ -1,22 +0,0 @@
-config AUTOFS_FS
-	tristate "Kernel automounter support"
-	depends on BKL # unfixable, just use autofs4
-	help
-	  The automounter is a tool to automatically mount remote file systems
-	  on demand. This implementation is partially kernel-based to reduce
-	  overhead in the already-mounted case; this is unlike the BSD
-	  automounter (amd), which is a pure user space daemon.
-
-	  To use the automounter you need the user-space tools from the autofs
-	  package; you can find the location in <file:Documentation/Changes>.
-	  You also want to answer Y to "NFS file system support", below.
-
-	  If you want to use the newer version of the automounter with more
-	  features, say N here and say Y to "Kernel automounter v4 support",
-	  below.
-
-	  To compile this support as a module, choose M here: the module will be
-	  called autofs.
-
-	  If you are not a part of a fairly large, distributed network, you
-	  probably do not need an automounter, and can say N here.
drivers/staging/autofs/Makefile deleted +0 -7
deleted file mode 100644
index f48781c..0000000
@@ -1,7 +0,0 @@
-#
-# Makefile for the linux autofs-filesystem routines.
-#
-
-obj-$(CONFIG_AUTOFS_FS) += autofs.o
-
-autofs-y := dirhash.o init.o inode.o root.o symlink.o waitq.o
drivers/staging/autofs/TODO deleted +0 -8
deleted file mode 100644
index 543803d..0000000
@@ -1,8 +0,0 @@
-autofs version 3 is on its way out of the kernel,
-It has been replaced by autofs4 several years ago.
-
-The autofs3 code uses the big kernel lock which
-is getting deprecated.
-
-Users that find autofs3 to work but not autofs4
-should talk to Ian Kent <[email protected]>.
drivers/staging/autofs/autofs_i.h deleted +0 -165

165 lines not on this page · raw message

drivers/staging/autofs/dirhash.c deleted +0 -260

260 lines not on this page · raw message

drivers/staging/autofs/init.c deleted +0 -52

52 lines not on this page · raw message

drivers/staging/autofs/inode.c deleted +0 -288

288 lines not on this page · raw message

drivers/staging/autofs/root.c deleted +0 -648

648 lines not on this page · raw message

drivers/staging/autofs/symlink.c deleted +0 -26
deleted file mode 100644
index ff2c65c..0000000
@@ -1,26 +0,0 @@
-/* -*- linux-c -*- --------------------------------------------------------- *
- *
- * drivers/staging/autofs/symlink.c
- *
- *  Copyright 1997-1998 Transmeta Corporation -- All Rights Reserved
- *
- * This file is part of the Linux kernel and is made available under
- * the terms of the GNU General Public License, version 2, or at your
- * option, any later version, incorporated herein by reference.
- *
- * ------------------------------------------------------------------------- */
-
-#include "autofs_i.h"
-
-/* Nothing to release.. */
-static void *autofs_follow_link(struct dentry *dentry, struct nameidata *nd)
-{
-	char *s=((struct autofs_symlink *)dentry->d_inode->i_private)->data;
-	nd_set_link(nd, s);
-	return NULL;
-}
-
-const struct inode_operations autofs_symlink_inode_operations = {
-	.readlink	= generic_readlink,
-	.follow_link	= autofs_follow_link
-};
drivers/staging/autofs/waitq.c deleted +0 -205

205 lines not on this page · raw message

Signature · 1 line
1.7.1
#12 lore raw

Re: [PATCH 06/20] staging: Remove autofs3

Quoted 10 lines · Arnd Bergmann

autofs3 was moved to staging in 2.6.37, so we can remove it in the 2.6.39 merge window. If we have a reason to bring it back after that, this patch can get reverted.

Cc Greg Kroah-Hartman <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ian Kent <[email protected]>

;)

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#13 lore raw

PATCH 7/20 staging: remove smbfs

In reply to Arnd Bergmann

smbfs got moved to staging in 2.6.37, so we can
finally remove it in the 2.6.39 merge window.
All users should by now have migrated to cifs.

---
 drivers/staging/Kconfig           |    2 -
 drivers/staging/Makefile          |    1 -
 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 -
 27 files changed, 0 insertions(+), 8599 deletions(-)
 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
drivers/staging/Kconfig +0 -2
index 3957b2f..6ac0418 100644
@@ -147,8 +147,6 @@ source "drivers/staging/msm/Kconfig"
 
 source "drivers/staging/lirc/Kconfig"
 
-source "drivers/staging/smbfs/Kconfig"
-
 source "drivers/staging/easycap/Kconfig"
 
 source "drivers/staging/solo6x10/Kconfig"
drivers/staging/Makefile +0 -1
index cbe013e..a834d2e 100644
@@ -33,7 +33,6 @@ obj-$(CONFIG_POHMELFS)		+= pohmelfs/
 obj-$(CONFIG_IDE_PHISON)	+= phison/
 obj-$(CONFIG_LINE6_USB)		+= line6/
 obj-$(CONFIG_USB_SERIAL_QUATECH2)	+= serqt_usb2/
-obj-$(CONFIG_SMB_FS)		+= smbfs/
 obj-$(CONFIG_USB_SERIAL_QUATECH_USB2)	+= quatech_usb2/
 obj-$(CONFIG_OCTEON_ETHERNET)	+= octeon/
 obj-$(CONFIG_VT6655)		+= vt6655/
drivers/staging/smbfs/Kconfig deleted +0 -56

56 lines not on this page · raw message

drivers/staging/smbfs/Makefile deleted +0 -18
deleted file mode 100644
index d2a92c5..0000000
@@ -1,18 +0,0 @@
-#
-# Makefile for the linux smb-filesystem routines.
-#
-
-obj-$(CONFIG_SMB_FS) += smbfs.o
-
-smbfs-y := proc.o dir.o cache.o sock.o inode.o file.o ioctl.o getopt.o \
-		symlink.o smbiod.o request.o
-
-# If you want debugging output, you may add these flags to the EXTRA_CFLAGS
-# SMBFS_PARANOIA should normally be enabled.
-
-ccflags-y := -DSMBFS_PARANOIA
-#ccflags-y += -DSMBFS_DEBUG
-#ccflags-y += -DSMBFS_DEBUG_VERBOSE
-#ccflags-y += -DDEBUG_SMB_TIMESTAMP
-#ccflags-y += -Werror
-
drivers/staging/smbfs/TODO deleted +0 -8
deleted file mode 100644
index 24f4d29..0000000
@@ -1,8 +0,0 @@
-smbfs is on its way out of the kernel, it has been replaced
-by cifs several years ago.
-
-The smbfs code uses the big kernel lock which
-is getting deprecated.
-
-Users that find smbfs to work but not cifs should contact
-the CIFS developers on [email protected].
drivers/staging/smbfs/cache.c deleted +0 -208

208 lines not on this page · raw message

drivers/staging/smbfs/dir.c deleted +0 -699

699 lines not on this page · raw message

drivers/staging/smbfs/file.c deleted +0 -456

456 lines not on this page · raw message

drivers/staging/smbfs/getopt.c deleted +0 -64

64 lines not on this page · raw message

drivers/staging/smbfs/getopt.h deleted +0 -14
deleted file mode 100644
index 146219a..0000000
@@ -1,14 +0,0 @@
-#ifndef _LINUX_GETOPT_H
-#define _LINUX_GETOPT_H
-
-struct option {
-	const char *name;
-	unsigned long flag;
-	int val;
-};
-
-extern int smb_getopt(char *caller, char **options, struct option *opts,
-		      char **optopt, char **optarg, unsigned long *flag,
-		      unsigned long *value);
-
-#endif /* _LINUX_GETOPT_H */
drivers/staging/smbfs/inode.c deleted +0 -854

854 lines not on this page · raw message

drivers/staging/smbfs/ioctl.c deleted +0 -68

68 lines not on this page · raw message

drivers/staging/smbfs/proc.c deleted +0 -3502

540 lines not on this page · raw message

drivers/staging/smbfs/proto.h deleted +0 -89

89 lines not on this page · raw message

drivers/staging/smbfs/request.c deleted +0 -817

817 lines not on this page · raw message

drivers/staging/smbfs/request.h deleted +0 -70

70 lines not on this page · raw message

drivers/staging/smbfs/smb.h deleted +0 -118

118 lines not on this page · raw message

drivers/staging/smbfs/smb_debug.h deleted +0 -34
deleted file mode 100644
index fc4b1a5..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/smbfs/smb_fs.h deleted +0 -153

153 lines not on this page · raw message

drivers/staging/smbfs/smb_fs_i.h deleted +0 -37
deleted file mode 100644
index 8ccf4ec..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/smbfs/smb_fs_sb.h deleted +0 -100

100 lines not on this page · raw message

drivers/staging/smbfs/smb_mount.h deleted +0 -65

65 lines not on this page · raw message

drivers/staging/smbfs/smbfs.txt deleted +0 -8
deleted file mode 100644
index 194fb0d..0000000

Lines of this file are not shown. View the raw message.

drivers/staging/smbfs/smbiod.c deleted +0 -343

343 lines not on this page · raw message

drivers/staging/smbfs/smbno.h deleted +0 -363

363 lines not on this page · raw message

drivers/staging/smbfs/sock.c deleted +0 -385

385 lines not on this page · raw message

drivers/staging/smbfs/symlink.c deleted +0 -67

67 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.7.1

Part of the diff is not shown here. View the raw message.

#14 lore raw

PATCH 8/20 adfs: remove the big kernel lock

In reply to Arnd Bergmann

According to Russell King, adfs was written to not require the big kernel lock, and all inode updates are done under adfs_dir_lock.

All other metadata in adfs is read-only and does not require locking. The use of the BKL is the result of various pushdowns from the VFS operations.

Cc Russell King <[email protected]>
Cc Stuart Swales <[email protected]>
---
 fs/adfs/Kconfig |    1 -
 fs/adfs/dir.c   |    6 ------
 fs/adfs/inode.c |    6 ------
 fs/adfs/super.c |   13 +------------
 4 files changed, 1 insertions(+), 25 deletions(-)
fs/adfs/Kconfig +0 -1
index 1dd5f34..e55182a 100644
@@ -1,7 +1,6 @@
 config ADFS_FS
 	tristate "ADFS file system support (EXPERIMENTAL)"
 	depends on BLOCK && EXPERIMENTAL
-	depends on BKL # need to fix
 	help
 	  The Acorn Disc Filing System is the standard file system of the
 	  RiscOS operating system which runs on Acorn's ARM-based Risc PC
fs/adfs/dir.c +0 -6
index 3b4a764..3d83075 100644
@@ -9,7 +9,6 @@
  *
  *  Common directory handling for ADFS
  */
-#include <linux/smp_lock.h>
 #include "adfs.h"
 
 /*
@@ -27,8 +26,6 @@ adfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	struct adfs_dir dir;
 	int ret = 0;
 
-	lock_kernel();	
-
 	if (filp->f_pos >> 32)
 		goto out;
 
@@ -70,7 +67,6 @@ free_out:
 	ops->free(&dir);
 
 out:
-	unlock_kernel();
 	return ret;
 }
 
@@ -276,7 +272,6 @@ adfs_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 	struct object_info obj;
 	int error;
 
-	lock_kernel();
 	error = adfs_dir_lookup_byname(dir, &dentry->d_name, &obj);
 	if (error == 0) {
 		error = -EACCES;
@@ -288,7 +283,6 @@ adfs_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 		if (inode)
 			error = 0;
 	}
-	unlock_kernel();
 	d_add(dentry, inode);
 	return ERR_PTR(error);
 }
fs/adfs/inode.c +0 -6
index 65794b8..09fe401 100644
@@ -7,7 +7,6 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/writeback.h>
 #include "adfs.h"
@@ -316,8 +315,6 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
 	unsigned int ia_valid = attr->ia_valid;
 	int error;
 	
-	lock_kernel();
-
 	error = inode_change_ok(inode, attr);
 
 	/*
@@ -359,7 +356,6 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
 	if (ia_valid & (ATTR_SIZE | ATTR_MTIME | ATTR_MODE))
 		mark_inode_dirty(inode);
 out:
-	unlock_kernel();
 	return error;
 }
 
@@ -374,7 +370,6 @@ int adfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	struct object_info obj;
 	int ret;
 
-	lock_kernel();
 	obj.file_id	= inode->i_ino;
 	obj.name_len	= 0;
 	obj.parent_id	= ADFS_I(inode)->parent_id;
@@ -384,6 +379,5 @@ int adfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	obj.size	= inode->i_size;
 
 	ret = adfs_dir_update(sb, &obj, wbc->sync_mode == WB_SYNC_ALL);
-	unlock_kernel();
 	return ret;
 }
fs/adfs/super.c +1 -12
index 2d79540..06d7388 100644
@@ -14,7 +14,6 @@
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/statfs.h>
 #include "adfs.h"
 #include "dir_f.h"
@@ -120,15 +119,11 @@ static void adfs_put_super(struct super_block *sb)
 	int i;
 	struct adfs_sb_info *asb = ADFS_SB(sb);
 
-	lock_kernel();
-
 	for (i = 0; i < asb->s_map_size; i++)
 		brelse(asb->s_map[i].dm_bh);
 	kfree(asb->s_map);
 	kfree(asb);
 	sb->s_fs_info = NULL;
-
-	unlock_kernel();
 }
 
 static int adfs_show_options(struct seq_file *seq, struct vfsmount *mnt)
@@ -359,15 +354,11 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 	struct adfs_sb_info *asb;
 	struct inode *root;
 
-	lock_kernel();
-
 	sb->s_flags |= MS_NODIRATIME;
 
 	asb = kzalloc(sizeof(*asb), GFP_KERNEL);
-	if (!asb) {
-		unlock_kernel();
+	if (!asb)
 		return -ENOMEM;
-	}
 	sb->s_fs_info = asb;
 
 	/* set default options */
@@ -485,7 +476,6 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 		adfs_error(sb, "get root inode failed\n");
 		goto error;
 	}
-	unlock_kernel();
 	return 0;
 
 error_free_bh:
@@ -493,7 +483,6 @@ error_free_bh:
 error:
 	sb->s_fs_info = NULL;
 	kfree(asb);
-	unlock_kernel();
 	return -EINVAL;
 }
 
Signature · 1 line
1.7.1
Acked-by on v1 8/20
#15 lore raw

Re: [PATCH 08/20] adfs: remove the big kernel lock

Quoted 9 lines · Arnd Bergmann

According to Russell King, adfs was written to not require the big kernel lock, and all inode updates are done under adfs_dir_lock.

All other metadata in adfs is read-only and does not require locking. The use of the BKL is the result of various pushdowns from the VFS operations.

Quoted 173 lines
Cc Stuart Swales <[email protected]>
---
 fs/adfs/Kconfig |    1 -
 fs/adfs/dir.c   |    6 ------
 fs/adfs/inode.c |    6 ------
 fs/adfs/super.c |   13 +------------
 4 files changed, 1 insertions(+), 25 deletions(-)
fs/adfs/Kconfig +0 -1
index 1dd5f34..e55182a 100644
@@ -1,7 +1,6 @@
 config ADFS_FS
 	tristate "ADFS file system support (EXPERIMENTAL)"
 	depends on BLOCK && EXPERIMENTAL
-	depends on BKL # need to fix
 	help
 	  The Acorn Disc Filing System is the standard file system of the
 	  RiscOS operating system which runs on Acorn's ARM-based Risc PC
fs/adfs/dir.c +0 -6
index 3b4a764..3d83075 100644
@@ -9,7 +9,6 @@
  *
  *  Common directory handling for ADFS
  */
-#include <linux/smp_lock.h>
 #include "adfs.h"
 
 /*
@@ -27,8 +26,6 @@ adfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	struct adfs_dir dir;
 	int ret = 0;
 
-	lock_kernel();	
-
 	if (filp->f_pos >> 32)
 		goto out;
 
@@ -70,7 +67,6 @@ free_out:
 	ops->free(&dir);
 
 out:
-	unlock_kernel();
 	return ret;
 }
 
@@ -276,7 +272,6 @@ adfs_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 	struct object_info obj;
 	int error;
 
-	lock_kernel();
 	error = adfs_dir_lookup_byname(dir, &dentry->d_name, &obj);
 	if (error == 0) {
 		error = -EACCES;
@@ -288,7 +283,6 @@ adfs_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 		if (inode)
 			error = 0;
 	}
-	unlock_kernel();
 	d_add(dentry, inode);
 	return ERR_PTR(error);
 }
fs/adfs/inode.c +0 -6
index 65794b8..09fe401 100644
@@ -7,7 +7,6 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/writeback.h>
 #include "adfs.h"
@@ -316,8 +315,6 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
 	unsigned int ia_valid = attr->ia_valid;
 	int error;
 
-	lock_kernel();
-
 	error = inode_change_ok(inode, attr);
 
 	/*
@@ -359,7 +356,6 @@ adfs_notify_change(struct dentry *dentry, struct iattr *attr)
 	if (ia_valid & (ATTR_SIZE | ATTR_MTIME | ATTR_MODE))
 		mark_inode_dirty(inode);
 out:
-	unlock_kernel();
 	return error;
 }
 
@@ -374,7 +370,6 @@ int adfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	struct object_info obj;
 	int ret;
 
-	lock_kernel();
 	obj.file_id	= inode->i_ino;
 	obj.name_len	= 0;
 	obj.parent_id	= ADFS_I(inode)->parent_id;
@@ -384,6 +379,5 @@ int adfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 	obj.size	= inode->i_size;
 
 	ret = adfs_dir_update(sb, &obj, wbc->sync_mode == WB_SYNC_ALL);
-	unlock_kernel();
 	return ret;
 }
fs/adfs/super.c +1 -12
index 2d79540..06d7388 100644
@@ -14,7 +14,6 @@
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/statfs.h>
 #include "adfs.h"
 #include "dir_f.h"
@@ -120,15 +119,11 @@ static void adfs_put_super(struct super_block *sb)
 	int i;
 	struct adfs_sb_info *asb = ADFS_SB(sb);
 
-	lock_kernel();
-
 	for (i = 0; i < asb->s_map_size; i++)
 		brelse(asb->s_map[i].dm_bh);
 	kfree(asb->s_map);
 	kfree(asb);
 	sb->s_fs_info = NULL;
-
-	unlock_kernel();
 }
 
 static int adfs_show_options(struct seq_file *seq, struct vfsmount *mnt)
@@ -359,15 +354,11 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 	struct adfs_sb_info *asb;
 	struct inode *root;
 
-	lock_kernel();
-
 	sb->s_flags |= MS_NODIRATIME;
 
 	asb = kzalloc(sizeof(*asb), GFP_KERNEL);
-	if (!asb) {
-		unlock_kernel();
+	if (!asb)
 		return -ENOMEM;
-	}
 	sb->s_fs_info = asb;
 
 	/* set default options */
@@ -485,7 +476,6 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 		adfs_error(sb, "get root inode failed\n");
 		goto error;
 	}
-	unlock_kernel();
 	return 0;
 
 error_free_bh:
@@ -493,7 +483,6 @@ error_free_bh:
 error:
 	sb->s_fs_info = NULL;
 	kfree(asb);
-	unlock_kernel();
 	return -EINVAL;
 }
 
Signature · 1 line
1.7.1
Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#16 lore raw

PATCH 9/20 hpfs: rename big kernel lock to hpfs_lock

In reply to Arnd Bergmann

This is a preparation to make it easier to get rid of the BKL, by just having to change one place in the HPFS code. There is no global state in hpfs, so all locking can be done per superblock, and the fill_super/put_super functions can do without it entirely.

---
 fs/hpfs/dir.c     |   23 +++++++++++------------
 fs/hpfs/file.c    |    9 ++++-----
 fs/hpfs/hpfs_fn.h |   14 ++++++++++++++
 fs/hpfs/inode.c   |    9 ++++-----
 fs/hpfs/namei.c   |   49 ++++++++++++++++++++++++-------------------------
 fs/hpfs/super.c   |   20 +++++---------------
 6 files changed, 62 insertions(+), 62 deletions(-)
fs/hpfs/dir.c +11 -12
index d32f63a..b3d7c0d 100644
@@ -6,16 +6,15 @@
  *  directory VFS functions
  */
 
-#include <linux/smp_lock.h>
 #include <linux/slab.h>
 #include "hpfs_fn.h"
 
 static int hpfs_dir_release(struct inode *inode, struct file *filp)
 {
-	lock_kernel();
+	hpfs_lock(inode->i_sb);
 	hpfs_del_pos(inode, &filp->f_pos);
 	/*hpfs_write_if_changed(inode);*/
-	unlock_kernel();
+	hpfs_unlock(inode->i_sb);
 	return 0;
 }
 
@@ -30,7 +29,7 @@ static loff_t hpfs_dir_lseek(struct file *filp, loff_t off, int whence)
 	struct hpfs_inode_info *hpfs_inode = hpfs_i(i);
 	struct super_block *s = i->i_sb;
 
-	lock_kernel();
+	hpfs_lock(s);
 
 	/*printk("dir lseek\n");*/
 	if (new_off == 0 || new_off == 1 || new_off == 11 || new_off == 12 || new_off == 13) goto ok;
@@ -43,12 +42,12 @@ static loff_t hpfs_dir_lseek(struct file *filp, loff_t off, int whence)
 	}
 	mutex_unlock(&i->i_mutex);
 ok:
-	unlock_kernel();
+	hpfs_unlock(s);
 	return filp->f_pos = new_off;
 fail:
 	mutex_unlock(&i->i_mutex);
 	/*printk("illegal lseek: %016llx\n", new_off);*/
-	unlock_kernel();
+	hpfs_unlock(s);
 	return -ESPIPE;
 }
 
@@ -64,7 +63,7 @@ static int hpfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	int c1, c2 = 0;
 	int ret = 0;
 
-	lock_kernel();
+	hpfs_lock(inode->i_sb);
 
 	if (hpfs_sb(inode->i_sb)->sb_chk) {
 		if (hpfs_chk_sectors(inode->i_sb, inode->i_ino, 1, "dir_fnode")) {
@@ -167,7 +166,7 @@ static int hpfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 		hpfs_brelse4(&qbh);
 	}
 out:
-	unlock_kernel();
+	hpfs_unlock(inode->i_sb);
 	return ret;
 }
 
@@ -197,10 +196,10 @@ struct dentry *hpfs_lookup(struct inode *dir, struct dentry *dentry, struct name
 	struct inode *result = NULL;
 	struct hpfs_inode_info *hpfs_result;
 
-	lock_kernel();
+	hpfs_lock(dir->i_sb);
 	if ((err = hpfs_chk_name(name, &len))) {
 		if (err == -ENAMETOOLONG) {
-			unlock_kernel();
+			hpfs_unlock(dir->i_sb);
 			return ERR_PTR(-ENAMETOOLONG);
 		}
 		goto end_add;
@@ -298,7 +297,7 @@ struct dentry *hpfs_lookup(struct inode *dir, struct dentry *dentry, struct name
 
 	end:
 	end_add:
-	unlock_kernel();
+	hpfs_unlock(dir->i_sb);
 	d_add(dentry, result);
 	return NULL;
 
@@ -311,7 +310,7 @@ struct dentry *hpfs_lookup(struct inode *dir, struct dentry *dentry, struct name
 	
 	/*bail:*/
 
-	unlock_kernel();
+	hpfs_unlock(dir->i_sb);
 	return ERR_PTR(-ENOENT);
 }
 
fs/hpfs/file.c +4 -5
index c034088..2dbae20 100644
@@ -6,16 +6,15 @@
  *  file VFS functions
  */
 
-#include <linux/smp_lock.h>
 #include "hpfs_fn.h"
 
 #define BLOCKS(size) (((size) + 511) >> 9)
 
 static int hpfs_file_release(struct inode *inode, struct file *file)
 {
-	lock_kernel();
+	hpfs_lock(inode->i_sb);
 	hpfs_write_if_changed(inode);
-	unlock_kernel();
+	hpfs_unlock(inode->i_sb);
 	return 0;
 }
 
@@ -49,14 +48,14 @@ static secno hpfs_bmap(struct inode *inode, unsigned file_secno)
 static void hpfs_truncate(struct inode *i)
 {
 	if (IS_IMMUTABLE(i)) return /*-EPERM*/;
-	lock_kernel();
+	hpfs_lock(i->i_sb);
 	hpfs_i(i)->i_n_secs = 0;
 	i->i_blocks = 1 + ((i->i_size + 511) >> 9);
 	hpfs_i(i)->mmu_private = i->i_size;
 	hpfs_truncate_btree(i->i_sb, i->i_ino, 1, ((i->i_size + 511) >> 9));
 	hpfs_write_inode(i);
 	hpfs_i(i)->i_n_secs = 0;
-	unlock_kernel();
+	hpfs_unlock(i->i_sb);
 }
 
 static int hpfs_get_block(struct inode *inode, sector_t iblock, struct buffer_head *bh_result, int create)
fs/hpfs/hpfs_fn.h +14 -0
index 1c43dbe..6761cb3 100644
@@ -13,6 +13,7 @@
 #include <linux/pagemap.h>
 #include <linux/buffer_head.h>
 #include <linux/slab.h>
+#include <linux/smp_lock.h>
 
 #include "hpfs.h"
 
@@ -342,3 +343,16 @@ static inline time32_t gmt_to_local(struct super_block *s, time_t t)
 	extern struct timezone sys_tz;
 	return t - sys_tz.tz_minuteswest * 60 - hpfs_sb(s)->sb_timeshift;
 }
+
+/*
+ * Locking
+ */
+static inline void hpfs_lock(struct super_block *s)
+{
+	lock_kernel();
+}
+
+static inline void hpfs_unlock(struct super_block *s)
+{
+	unlock_kernel();
+}
fs/hpfs/inode.c +4 -5
index 1ae35ba..87f1f78 100644
@@ -6,7 +6,6 @@
  *  inode VFS functions
  */
 
-#include <linux/smp_lock.h>
 #include <linux/slab.h>
 #include "hpfs_fn.h"
 
@@ -267,7 +266,7 @@ int hpfs_setattr(struct dentry *dentry, struct iattr *attr)
 	struct inode *inode = dentry->d_inode;
 	int error = -EINVAL;
 
-	lock_kernel();
+	hpfs_lock(inode->i_sb);
 	if (inode->i_ino == hpfs_sb(inode->i_sb)->sb_root)
 		goto out_unlock;
 	if ((attr->ia_valid & ATTR_SIZE) && attr->ia_size > inode->i_size)
@@ -290,7 +289,7 @@ int hpfs_setattr(struct dentry *dentry, struct iattr *attr)
 	hpfs_write_inode(inode);
 
  out_unlock:
-	unlock_kernel();
+	hpfs_unlock(inode->i_sb);
 	return error;
 }
 
@@ -307,8 +306,8 @@ void hpfs_evict_inode(struct inode *inode)
 	truncate_inode_pages(&inode->i_data, 0);
 	end_writeback(inode);
 	if (!inode->i_nlink) {
-		lock_kernel();
+		hpfs_lock(inode->i_sb);
 		hpfs_remove_fnode(inode->i_sb, inode->i_ino);
-		unlock_kernel();
+		hpfs_unlock(inode->i_sb);
 	}
 }
fs/hpfs/namei.c +24 -25

194 lines not on this page · raw message

fs/hpfs/super.c +5 -15
index b30426b..501ea86 100644
@@ -13,7 +13,6 @@
 #include <linux/statfs.h>
 #include <linux/magic.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/bitmap.h>
 #include <linux/slab.h>
 
@@ -103,15 +102,11 @@ static void hpfs_put_super(struct super_block *s)
 {
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
 
-	lock_kernel();
-
 	kfree(sbi->sb_cp_table);
 	kfree(sbi->sb_bmp_dir);
 	unmark_dirty(s);
 	s->s_fs_info = NULL;
 	kfree(sbi);
-
-	unlock_kernel();
 }
 
 unsigned hpfs_count_one_bitmap(struct super_block *s, secno secno)
@@ -143,7 +138,7 @@ static int hpfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct super_block *s = dentry->d_sb;
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
 	u64 id = huge_encode_dev(s->s_bdev->bd_dev);
-	lock_kernel();
+	hpfs_lock(s);
 
 	/*if (sbi->sb_n_free == -1) {*/
 		sbi->sb_n_free = count_bitmaps(s);
@@ -160,7 +155,7 @@ static int hpfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_fsid.val[1] = (u32)(id >> 32);
 	buf->f_namelen = 254;
 
-	unlock_kernel();
+	hpfs_unlock(s);
 
 	return 0;
 }
@@ -406,7 +401,7 @@ static int hpfs_remount_fs(struct super_block *s, int *flags, char *data)
 	
 	*flags |= MS_NOATIME;
 	
-	lock_kernel();
+	hpfs_lock(s);
 	lock_super(s);
 	uid = sbi->sb_uid; gid = sbi->sb_gid;
 	umask = 0777 & ~sbi->sb_mode;
@@ -441,12 +436,12 @@ static int hpfs_remount_fs(struct super_block *s, int *flags, char *data)
 	replace_mount_options(s, new_opts);
 
 	unlock_super(s);
-	unlock_kernel();
+	hpfs_unlock(s);
 	return 0;
 
 out_err:
 	unlock_super(s);
-	unlock_kernel();
+	hpfs_unlock(s);
 	kfree(new_opts);
 	return -EINVAL;
 }
@@ -484,13 +479,10 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 
 	int o;
 
-	lock_kernel();
-
 	save_mount_options(s, options);
 
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi) {
-		unlock_kernel();
 		return -ENOMEM;
 	}
 	s->s_fs_info = sbi;
@@ -677,7 +669,6 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 			root->i_blocks = 5;
 		hpfs_brelse4(&qbh);
 	}
-	unlock_kernel();
 	return 0;
 
 bail4:	brelse(bh2);
@@ -689,7 +680,6 @@ bail0:
 	kfree(sbi->sb_cp_table);
 	s->s_fs_info = NULL;
 	kfree(sbi);
-	unlock_kernel();
 	return -EINVAL;
 }
 
Signature · 1 line
1.7.1
#17 lore raw

PATCH 10/20 hpfs: replace BKL with a global mutex

In reply to Arnd Bergmann

This removes the BKL in hpfs in a rather awful way, by replacing it with a mutex that basically serializes the entire file system.

The only reason I hope to get away with this is because there don't seem to be any real users left, and the file system is going out through the staging door anyways.

Since the mutex code is not even tested and I don't expect anyone to do that, I'm marking it as broken on SMP and PREEMPT kernels, so it won't actually get used.

---
 fs/hpfs/Kconfig   |    2 +-
 fs/hpfs/hpfs_fn.h |   30 ++++++++++++++++++++++++++----
 fs/hpfs/super.c   |    1 +
 3 files changed, 28 insertions(+), 5 deletions(-)
fs/hpfs/Kconfig +1 -1
index 63b6f56..73476c1 100644
@@ -1,7 +1,7 @@
 config HPFS_FS
 	tristate "OS/2 HPFS file system support"
 	depends on BLOCK
-	depends on BKL # nontrivial to fix
+	depends on BROKEN || !(SMP || PREEMPT)
 	help
 	  OS/2 is IBM's operating system for PC's, the same as Warp, and HPFS
 	  is the file system used for organizing files on OS/2 hard disk
fs/hpfs/hpfs_fn.h +26 -4
index 6761cb3..ffdb16e 100644
@@ -13,7 +13,6 @@
 #include <linux/pagemap.h>
 #include <linux/buffer_head.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 
 #include "hpfs.h"
 
@@ -90,6 +89,7 @@ struct hpfs_sb_info {
 	unsigned sb_max_fwd_alloc;	/* max forwad allocation */
 	struct mutex hpfs_creation_de;	/* when creating dirents, nobody else
 					   can alloc blocks */
+	struct mutex global_mutex;	/* replaces BKL, serializes everything on SMP */
 	/*unsigned sb_mounting : 1;*/
 	int sb_timeshift;
 };
@@ -345,14 +345,36 @@ static inline time32_t gmt_to_local(struct super_block *s, time_t t)
 }
 
 /*
- * Locking
+ * Locking:
+ *
+ * hpfs_lock() is a leftover from the big kernel lock.
+ * This mutex is used as the outermost lock in hpfs,
+ * all other mutexes (inode->i_mutex, hpfs_inode_info->i_mutex,
+ * hpfs_inode_info->i_parent_mutex, hpfs_creation_de and
+ * lock_super) can get taken while hpfs_lock is held already.
+ *
+ * Code inspection has shown that hpfs_lock never gets taken
+ * recursively, and that it never nests inside one of the
+ * mutexes above, so there should be no deadlocks.
+ *
+ * However, it is frequently held while wait_for_buffer
+ * sleeps, which probably causes a huge slowdown, since
+ * that serializes all operations on a file system.
+ *
+ * Moreover, the locking code is entirely untested, use at
+ * your own risk.
  */
 static inline void hpfs_lock(struct super_block *s)
 {
-	lock_kernel();
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
+#warning HPFS is likely broken on SMP configurations
+	mutex_lock(&hpfs_sb(s)->global_mutex);
+#endif
 }
 
 static inline void hpfs_unlock(struct super_block *s)
 {
-	unlock_kernel();
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
+	mutex_unlock(&hpfs_sb(s)->global_mutex);
+#endif
 }
fs/hpfs/super.c +1 -0
index 501ea86..694a683 100644
@@ -491,6 +491,7 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 	sbi->sb_cp_table = NULL;
 
 	mutex_init(&sbi->hpfs_creation_de);
+	mutex_init(&sbi->global_mutex);
 
 	uid = current_uid();
 	gid = current_gid();
Signature · 1 line
1.7.1
#18 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

Quoted 3 lines · Arnd Bergmann

This removes the BKL in hpfs in a rather awful
way, by replacing it with a mutex that basically
serializes the entire file system.

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

-Andi

[1] showing my age

Signature · 2 lines
Andi Kleen
Intel Open Source Technology Center
#19 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

In reply to Arnd Bergmann
Quoted 3 lines · Arnd Bergmann

This removes the BKL in hpfs in a rather awful
way, by replacing it with a mutex that basically
serializes the entire file system.

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

-Andi

[1] showing my age

Signature · 2 lines
Andi Kleen
Intel Open Source Technology Center
#20 lore raw

PATCH v2 1/1 hpfs: remove the BKL

This removes the BKL in hpfs in a rather awful way, by making the code only work on uniprocessor systems without kernel preemption, as suggested by Andi Kleen.

The HPFS code probably has close to zero remaining users on current kernels, all archeological uses of the file system can probably be done with the significant restrictions.

The hpfs_lock/hpfs_unlock functions are left in the code as stubs, in case someone feels motivated to make it work on SMP again.

fs/hpfs/Kconfig +1 -1
index 63b6f56..0c39dc3 100644
@@ -1,7 +1,7 @@
 config HPFS_FS
 	tristate "OS/2 HPFS file system support"
 	depends on BLOCK
-	depends on BKL # nontrivial to fix
+	depends on BROKEN || !PREEMPT
 	help
 	  OS/2 is IBM's operating system for PC's, the same as Warp, and HPFS
 	  is the file system used for organizing files on OS/2 hard disk
fs/hpfs/hpfs_fn.h +12 -4
index 6761cb3..c15adbc 100644
@@ -13,7 +13,6 @@
 #include <linux/pagemap.h>
 #include <linux/buffer_head.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 
 #include "hpfs.h"
 
@@ -345,14 +344,23 @@ static inline time32_t gmt_to_local(struct super_block *s, time_t t)
 }
 
 /*
- * Locking
+ * Locking:
+ *
+ * hpfs_lock() is a leftover from the big kernel lock.
+ * Right now, these functions are empty and only left
+ * for documentation purposes. The file system no longer
+ * works on SMP systems, so the lock is not needed
+ * any more.
+ *
+ * If someone is interested in making it work again, this
+ * would be the place to start by adding a per-superblock
+ * mutex and fixing all the bugs and performance issues
+ * caused by that.
  */
 static inline void hpfs_lock(struct super_block *s)
 {
-	lock_kernel();
 }
 
 static inline void hpfs_unlock(struct super_block *s)
 {
-	unlock_kernel();
 }
fs/hpfs/super.c +5 -0
index 501ea86..c89b408 100644
@@ -479,6 +479,11 @@ static int hpfs_fill_super(struct super_block *s, void *options, int silent)
 
 	int o;
 
+	if (num_possible_cpus() > 1) {
+		printk(KERN_ERR "HPFS is not SMP safe\n");
+		return -EINVAL;
+	}
+
 	save_mount_options(s, options);
 
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
#21 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

In reply to Andi Kleen
Quoted 5 lines · Andi Kleen

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

I don't see much value of one evil over the other, but why not. The code is going away anyway unless someone cleans it up further.

Arnd

#22 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

Quoted 9 lines · Arnd Bergmann
Quoted 5 lines · Andi Kleen

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

I don't see much value of one evil over the other, but why not. The code is going away anyway unless someone cleans it up further.

I think it's fine because systems running OS/2 likely only have one CPU. This way these guys (if they exist) won't see any regression.

-Andi

Signature · 2 lines
Andi Kleen
Intel Open Source Technology Center
#23 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

Quoted 5 lines · Andi Kleen
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · Andi Kleen

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

I don't see much value of one evil over the other, but why not. The code is going away anyway unless someone cleans it up further.

I think it's fine because systems running OS/2 likely only have one CPU. This way these guys (if they exist) won't see any regression.

It would be really neat if there was a project to destage a lot of these old filesystems and implement read-only support with fuse implementations (not that I'm volunteering :P).

I almost think write access to a lot of these old filesystems is pretty dangerous to support in the kernel due to lack of testing and bitrot.

#24 lore raw

Re: [PATCH 10/20] hpfs: replace BKL with a global mutex

Quoted 12 lines · Nick Piggin
Quoted 11 lines · Andi Kleen
Quoted 9 lines · Arnd Bergmann
Quoted 5 lines · Andi Kleen

I don't think that's a very good idea, there's danger
of holding it over IO and that would be really bad (as in like MINIX[1])

It would be better to do it the i810 way and check for the number of CPUs at module init time and refuse to run if it's > 1

I don't see much value of one evil over the other, but why not. The code is going away anyway unless someone cleans it up further.

Quoted 6 lines
Quoted 2 lines

I think it's fine because systems running OS/2 likely only have one CPU. This way these guys (if they exist) won't see any regression.

It would be really neat if there was a project to destage a lot of these old filesystems and implement read-only support with fuse implementations (not that I'm volunteering :P).

I did that using UML + fuse. It's pretty neat, UML boots fast when it doesn't need a big userspace, so the mount is done in a few hundred milliseconds, and performance isn't horrible. And you get support for *all* the filesystems in a single 3MB executable.

The big drawback is that UML is not portable to lots of architectures (x86 and ppc are the only ones it works on, I think).

Thanks,
Miklos

#25 lore raw

PATCH 11/20 hpfs: move to drivers/staging

In reply to Arnd Bergmann

hpfs has not seen a single patch from its maintainer since the start of the git history, and the user base seems to be completely gone after the end of OS/2.

I have tried to keep the file system alive as much as possible through the BKL removal, but this its time seems to have come. If we can find an actual user who is willing to test patches, or even a maintainer that can work on the code, it can easily be moved back into the main tree.

If that does not happen, it will get removed from the kernel tree after a few releases in staging/.

---
 MAINTAINERS                            |    2 +-
 drivers/staging/Kconfig                |    2 ++
 drivers/staging/Makefile               |    1 +
 {fs => drivers/staging}/hpfs/Kconfig   |    3 +++
 {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     |    0
 {fs => drivers/staging}/hpfs/dnode.c   |    0
 {fs => drivers/staging}/hpfs/ea.c      |    0
 {fs => drivers/staging}/hpfs/file.c    |    0
 {fs => drivers/staging}/hpfs/hpfs.h    |    0
 {fs => drivers/staging}/hpfs/hpfs_fn.h |    0
 {fs => drivers/staging}/hpfs/inode.c   |    0
 {fs => drivers/staging}/hpfs/map.c     |    0
 {fs => drivers/staging}/hpfs/name.c    |    0
 {fs => drivers/staging}/hpfs/namei.c   |    0
 {fs => drivers/staging}/hpfs/super.c   |    0
 fs/Kconfig                             |    1 -
 fs/Makefile                            |    1 -
 23 files changed, 12 insertions(+), 3 deletions(-)
 rename {fs => drivers/staging}/hpfs/Kconfig (85%)
 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 (100%)
 rename {fs => drivers/staging}/hpfs/dnode.c (100%)
 rename {fs => drivers/staging}/hpfs/ea.c (100%)
 rename {fs => drivers/staging}/hpfs/file.c (100%)
 rename {fs => drivers/staging}/hpfs/hpfs.h (100%)
 rename {fs => drivers/staging}/hpfs/hpfs_fn.h (100%)
 rename {fs => drivers/staging}/hpfs/inode.c (100%)
 rename {fs => drivers/staging}/hpfs/map.c (100%)
 rename {fs => drivers/staging}/hpfs/name.c (100%)
 rename {fs => drivers/staging}/hpfs/namei.c (100%)
 rename {fs => drivers/staging}/hpfs/super.c (100%)
MAINTAINERS +1 -1
index 246ee22..fe5ca5f 100644
@@ -3009,7 +3009,7 @@ HPFS FILESYSTEM
 M:	Mikulas Patocka <[email protected]>
 W:	http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi
 S:	Maintained
-F:	fs/hpfs/
+F:	drivers/staging/hpfs/
 
 HSO 3G MODEM DRIVER
 M:	Jan Dumon <[email protected]>
drivers/staging/Kconfig +2 -0
index 6ac0418..9fc5aa6 100644
@@ -175,5 +175,7 @@ source "drivers/staging/cptm1217/Kconfig"
 
 source "drivers/staging/ste_rmi4/Kconfig"
 
+source "drivers/staging/hpfs/Kconfig"
+
 endif # !STAGING_EXCLUDE_BUILD
 endif # STAGING
drivers/staging/Makefile +1 -0
index a834d2e..878f381 100644
@@ -68,3 +68,4 @@ obj-$(CONFIG_SND_INTEL_SST)		+= intel_sst/
 obj-$(CONFIG_SPEAKUP)	+= speakup/
 obj-$(CONFIG_TOUCHSCREEN_CLEARPAD_TM1217)	+= cptm1217/
 obj-$(CONFIG_TOUCHSCREEN_SYNAPTICS_I2C_RMI4)	+= ste_rmi4/
+obj-$(CONFIG_HPFS_FS)		+= hpfs/
drivers/staging/hpfs/Kconfig renamed from fs/hpfs/Kconfig +3 -0
similarity index 85%
rename from fs/hpfs/Kconfig
rename to drivers/staging/hpfs/Kconfig
index 73476c1..f979319 100644
@@ -11,5 +11,8 @@ config HPFS_FS
 	  option in order to be able to read them. Read
 	  <file:Documentation/filesystems/hpfs.txt>.
 
+	  The HPFS code is about to get removed from the kernel and is no
+	  longer supported on SMP or PREEMPT kernels.
+
 	  To compile this file system support as a module, choose M here: the
 	  module will be called hpfs.  If unsure, say N.
drivers/staging/hpfs/Makefile renamed from fs/hpfs/Makefile +0 -0
similarity index 100%
rename from fs/hpfs/Makefile
rename to drivers/staging/hpfs/Makefile
drivers/staging/hpfs/TODO new file +5 -0
new file mode 100644
index 0000000..576a01c
@@ -0,0 +1,5 @@
+HPFS seems to have no remaining users, and it is getting
+increasingly hard to maintain. It will be removed in one
+of the next kernel releases unless someone puts effort
+into fixing the locking. The new hpfs_lock that replaces
+the BKL has not been tested.
drivers/staging/hpfs/alloc.c renamed from fs/hpfs/alloc.c +0 -0
similarity index 100%
rename from fs/hpfs/alloc.c
rename to drivers/staging/hpfs/alloc.c
drivers/staging/hpfs/anode.c renamed from fs/hpfs/anode.c +0 -0
similarity index 100%
rename from fs/hpfs/anode.c
rename to drivers/staging/hpfs/anode.c
drivers/staging/hpfs/buffer.c renamed from fs/hpfs/buffer.c +0 -0
similarity index 100%
rename from fs/hpfs/buffer.c
rename to drivers/staging/hpfs/buffer.c
drivers/staging/hpfs/dentry.c renamed from fs/hpfs/dentry.c +0 -0
similarity index 100%
rename from fs/hpfs/dentry.c
rename to drivers/staging/hpfs/dentry.c
drivers/staging/hpfs/dir.c renamed from fs/hpfs/dir.c +0 -0
similarity index 100%
rename from fs/hpfs/dir.c
rename to drivers/staging/hpfs/dir.c
drivers/staging/hpfs/dnode.c renamed from fs/hpfs/dnode.c +0 -0
similarity index 100%
rename from fs/hpfs/dnode.c
rename to drivers/staging/hpfs/dnode.c
drivers/staging/hpfs/ea.c renamed from fs/hpfs/ea.c +0 -0
similarity index 100%
rename from fs/hpfs/ea.c
rename to drivers/staging/hpfs/ea.c
drivers/staging/hpfs/file.c renamed from fs/hpfs/file.c +0 -0
similarity index 100%
rename from fs/hpfs/file.c
rename to drivers/staging/hpfs/file.c
drivers/staging/hpfs/hpfs.h renamed from fs/hpfs/hpfs.h +0 -0
similarity index 100%
rename from fs/hpfs/hpfs.h
rename to drivers/staging/hpfs/hpfs.h
drivers/staging/hpfs/hpfs_fn.h renamed from fs/hpfs/hpfs_fn.h +0 -0
similarity index 100%
rename from fs/hpfs/hpfs_fn.h
rename to drivers/staging/hpfs/hpfs_fn.h
drivers/staging/hpfs/inode.c renamed from fs/hpfs/inode.c +0 -0
similarity index 100%
rename from fs/hpfs/inode.c
rename to drivers/staging/hpfs/inode.c
drivers/staging/hpfs/map.c renamed from fs/hpfs/map.c +0 -0
similarity index 100%
rename from fs/hpfs/map.c
rename to drivers/staging/hpfs/map.c
drivers/staging/hpfs/name.c renamed from fs/hpfs/name.c +0 -0
similarity index 100%
rename from fs/hpfs/name.c
rename to drivers/staging/hpfs/name.c
drivers/staging/hpfs/namei.c renamed from fs/hpfs/namei.c +0 -0
similarity index 100%
rename from fs/hpfs/namei.c
rename to drivers/staging/hpfs/namei.c
drivers/staging/hpfs/super.c renamed from fs/hpfs/super.c +0 -0
similarity index 100%
rename from fs/hpfs/super.c
rename to drivers/staging/hpfs/super.c
fs/Kconfig +0 -1
index 3db9caa..5a54574 100644
@@ -184,7 +184,6 @@ source "fs/squashfs/Kconfig"
 source "fs/freevxfs/Kconfig"
 source "fs/minix/Kconfig"
 source "fs/omfs/Kconfig"
-source "fs/hpfs/Kconfig"
 source "fs/qnx4/Kconfig"
 source "fs/romfs/Kconfig"
 source "fs/sysv/Kconfig"
fs/Makefile +0 -1
index a7f7cef..c920ca9 100644
@@ -90,7 +90,6 @@ obj-$(CONFIG_NLS)		+= nls/
 obj-$(CONFIG_SYSV_FS)		+= sysv/
 obj-$(CONFIG_CIFS)		+= cifs/
 obj-$(CONFIG_NCP_FS)		+= ncpfs/
-obj-$(CONFIG_HPFS_FS)		+= hpfs/
 obj-$(CONFIG_NTFS_FS)		+= ntfs/
 obj-$(CONFIG_UFS_FS)		+= ufs/
 obj-$(CONFIG_EFS_FS)		+= efs/
Signature · 1 line
1.7.1
maintainer
#26 lore raw

Re: [PATCH 11/20] hpfs: move to drivers/staging

Hey, I will solve it sometimes, but I am in a mental hospital now.

Mikulas

Quoted 213 lines · Arnd Bergmann

hpfs has not seen a single patch from its maintainer since the start of the git history, and the user base seems to be completely gone after the end of OS/2.

I have tried to keep the file system alive as much as possible through the BKL removal, but this its time seems to have come. If we can find an actual user who is willing to test patches, or even a maintainer that can work on the code, it can easily be moved back into the main tree.

If that does not happen, it will get removed from the kernel tree after a few releases in staging/.

---
 MAINTAINERS                            |    2 +-
 drivers/staging/Kconfig                |    2 ++
 drivers/staging/Makefile               |    1 +
 {fs => drivers/staging}/hpfs/Kconfig   |    3 +++
 {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     |    0
 {fs => drivers/staging}/hpfs/dnode.c   |    0
 {fs => drivers/staging}/hpfs/ea.c      |    0
 {fs => drivers/staging}/hpfs/file.c    |    0
 {fs => drivers/staging}/hpfs/hpfs.h    |    0
 {fs => drivers/staging}/hpfs/hpfs_fn.h |    0
 {fs => drivers/staging}/hpfs/inode.c   |    0
 {fs => drivers/staging}/hpfs/map.c     |    0
 {fs => drivers/staging}/hpfs/name.c    |    0
 {fs => drivers/staging}/hpfs/namei.c   |    0
 {fs => drivers/staging}/hpfs/super.c   |    0
 fs/Kconfig                             |    1 -
 fs/Makefile                            |    1 -
 23 files changed, 12 insertions(+), 3 deletions(-)
 rename {fs => drivers/staging}/hpfs/Kconfig (85%)
 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 (100%)
 rename {fs => drivers/staging}/hpfs/dnode.c (100%)
 rename {fs => drivers/staging}/hpfs/ea.c (100%)
 rename {fs => drivers/staging}/hpfs/file.c (100%)
 rename {fs => drivers/staging}/hpfs/hpfs.h (100%)
 rename {fs => drivers/staging}/hpfs/hpfs_fn.h (100%)
 rename {fs => drivers/staging}/hpfs/inode.c (100%)
 rename {fs => drivers/staging}/hpfs/map.c (100%)
 rename {fs => drivers/staging}/hpfs/name.c (100%)
 rename {fs => drivers/staging}/hpfs/namei.c (100%)
 rename {fs => drivers/staging}/hpfs/super.c (100%)
MAINTAINERS +1 -1
index 246ee22..fe5ca5f 100644
@@ -3009,7 +3009,7 @@ HPFS FILESYSTEM
 M:	Mikulas Patocka <[email protected]>
 W:	http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi
 S:	Maintained
-F:	fs/hpfs/
+F:	drivers/staging/hpfs/
 
 HSO 3G MODEM DRIVER
 M:	Jan Dumon <[email protected]>
drivers/staging/Kconfig +2 -0
index 6ac0418..9fc5aa6 100644
@@ -175,5 +175,7 @@ source "drivers/staging/cptm1217/Kconfig"
 
 source "drivers/staging/ste_rmi4/Kconfig"
 
+source "drivers/staging/hpfs/Kconfig"
+
 endif # !STAGING_EXCLUDE_BUILD
 endif # STAGING
drivers/staging/Makefile +1 -0
index a834d2e..878f381 100644
@@ -68,3 +68,4 @@ obj-$(CONFIG_SND_INTEL_SST)		+= intel_sst/
 obj-$(CONFIG_SPEAKUP)	+= speakup/
 obj-$(CONFIG_TOUCHSCREEN_CLEARPAD_TM1217)	+= cptm1217/
 obj-$(CONFIG_TOUCHSCREEN_SYNAPTICS_I2C_RMI4)	+= ste_rmi4/
+obj-$(CONFIG_HPFS_FS)		+= hpfs/
drivers/staging/hpfs/Kconfig renamed from fs/hpfs/Kconfig +3 -0
similarity index 85%
rename from fs/hpfs/Kconfig
rename to drivers/staging/hpfs/Kconfig
index 73476c1..f979319 100644
@@ -11,5 +11,8 @@ config HPFS_FS
 	  option in order to be able to read them. Read
 	  <file:Documentation/filesystems/hpfs.txt>.
 
+	  The HPFS code is about to get removed from the kernel and is no
+	  longer supported on SMP or PREEMPT kernels.
+
 	  To compile this file system support as a module, choose M here: the
 	  module will be called hpfs.  If unsure, say N.
drivers/staging/hpfs/Makefile renamed from fs/hpfs/Makefile +0 -0
similarity index 100%
rename from fs/hpfs/Makefile
rename to drivers/staging/hpfs/Makefile
drivers/staging/hpfs/TODO new file +5 -0
new file mode 100644
index 0000000..576a01c
@@ -0,0 +1,5 @@
+HPFS seems to have no remaining users, and it is getting
+increasingly hard to maintain. It will be removed in one
+of the next kernel releases unless someone puts effort
+into fixing the locking. The new hpfs_lock that replaces
+the BKL has not been tested.
drivers/staging/hpfs/alloc.c renamed from fs/hpfs/alloc.c +0 -0
similarity index 100%
rename from fs/hpfs/alloc.c
rename to drivers/staging/hpfs/alloc.c
drivers/staging/hpfs/anode.c renamed from fs/hpfs/anode.c +0 -0
similarity index 100%
rename from fs/hpfs/anode.c
rename to drivers/staging/hpfs/anode.c
drivers/staging/hpfs/buffer.c renamed from fs/hpfs/buffer.c +0 -0
similarity index 100%
rename from fs/hpfs/buffer.c
rename to drivers/staging/hpfs/buffer.c
drivers/staging/hpfs/dentry.c renamed from fs/hpfs/dentry.c +0 -0
similarity index 100%
rename from fs/hpfs/dentry.c
rename to drivers/staging/hpfs/dentry.c
drivers/staging/hpfs/dir.c renamed from fs/hpfs/dir.c +0 -0
similarity index 100%
rename from fs/hpfs/dir.c
rename to drivers/staging/hpfs/dir.c
drivers/staging/hpfs/dnode.c renamed from fs/hpfs/dnode.c +0 -0
similarity index 100%
rename from fs/hpfs/dnode.c
rename to drivers/staging/hpfs/dnode.c
drivers/staging/hpfs/ea.c renamed from fs/hpfs/ea.c +0 -0
similarity index 100%
rename from fs/hpfs/ea.c
rename to drivers/staging/hpfs/ea.c
drivers/staging/hpfs/file.c renamed from fs/hpfs/file.c +0 -0
similarity index 100%
rename from fs/hpfs/file.c
rename to drivers/staging/hpfs/file.c
drivers/staging/hpfs/hpfs.h renamed from fs/hpfs/hpfs.h +0 -0
similarity index 100%
rename from fs/hpfs/hpfs.h
rename to drivers/staging/hpfs/hpfs.h
drivers/staging/hpfs/hpfs_fn.h renamed from fs/hpfs/hpfs_fn.h +0 -0
similarity index 100%
rename from fs/hpfs/hpfs_fn.h
rename to drivers/staging/hpfs/hpfs_fn.h
drivers/staging/hpfs/inode.c renamed from fs/hpfs/inode.c +0 -0
similarity index 100%
rename from fs/hpfs/inode.c
rename to drivers/staging/hpfs/inode.c
drivers/staging/hpfs/map.c renamed from fs/hpfs/map.c +0 -0
similarity index 100%
rename from fs/hpfs/map.c
rename to drivers/staging/hpfs/map.c
drivers/staging/hpfs/name.c renamed from fs/hpfs/name.c +0 -0
similarity index 100%
rename from fs/hpfs/name.c
rename to drivers/staging/hpfs/name.c
drivers/staging/hpfs/namei.c renamed from fs/hpfs/namei.c +0 -0
similarity index 100%
rename from fs/hpfs/namei.c
rename to drivers/staging/hpfs/namei.c
drivers/staging/hpfs/super.c renamed from fs/hpfs/super.c +0 -0
similarity index 100%
rename from fs/hpfs/super.c
rename to drivers/staging/hpfs/super.c
fs/Kconfig +0 -1
index 3db9caa..5a54574 100644
@@ -184,7 +184,6 @@ source "fs/squashfs/Kconfig"
 source "fs/freevxfs/Kconfig"
 source "fs/minix/Kconfig"
 source "fs/omfs/Kconfig"
-source "fs/hpfs/Kconfig"
 source "fs/qnx4/Kconfig"
 source "fs/romfs/Kconfig"
 source "fs/sysv/Kconfig"
fs/Makefile +0 -1
index a7f7cef..c920ca9 100644
@@ -90,7 +90,6 @@ obj-$(CONFIG_NLS)		+= nls/
 obj-$(CONFIG_SYSV_FS)		+= sysv/
 obj-$(CONFIG_CIFS)		+= cifs/
 obj-$(CONFIG_NCP_FS)		+= ncpfs/
-obj-$(CONFIG_HPFS_FS)		+= hpfs/
 obj-$(CONFIG_NTFS_FS)		+= ntfs/
 obj-$(CONFIG_UFS_FS)		+= ufs/
 obj-$(CONFIG_EFS_FS)		+= efs/
Signature · 1 line
1.7.1
#27 lore raw

Re: [PATCH 11/20] hpfs: move to drivers/staging

Quoted 2 lines · On Monday 07 February 2011 17:17:37 Mikulas Patocka wrote:

Hey, I will solve it sometimes, but I am in a mental hospital now.

Hi Mikulas,

Good to hear that you are still interested in the code. I'll make sure we don't just remove it then, but I suppose we can still mark it as non-SMP in the meantime.

I hope you get well soon,

Arnd

#28 lore raw

PATCH 12/20 x25: remove the BKL

In reply to Arnd Bergmann

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.

---
 net/x25/Kconfig   |    1 -
 net/x25/af_x25.c  |   61 ++++++++++++++++------------------------------------
 net/x25/x25_out.c |    7 ++++-
 3 files changed, 24 insertions(+), 45 deletions(-)
net/x25/Kconfig +0 -1
index 2196e55..e6759c9 100644
@@ -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
net/x25/af_x25.c +19 -42

162 lines not on this page · raw message

net/x25/x25_out.c +5 -2
index d00649f..f1a6ff1 100644
@@ -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;
Signature · 1 line
1.7.1
#29 lore raw

Re: [PATCH 12/20] x25: remove the BKL

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...

Quoted 231 lines · Arnd Bergmann

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.

---
 net/x25/Kconfig   |    1 -
 net/x25/af_x25.c  |   61 ++++++++++++++++------------------------------------ net/x25/x25_out.c |    7 ++++-

 3 files changed, 24 insertions(+), 45 deletions(-)
net/x25/Kconfig +0 -0
index 2196e55..e6759c9 100644
@@ -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

net/x25/af_x25.c +3 -12
index ad96ee9..8f5d1bb 100644
@@ -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:

net/x25/af_x25.c +0 -0
@@ -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;
 }

net/x25/af_x25.c +4 -19
@@ -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);

net/x25/af_x25.c +3 -3
@@ -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;
 }

net/x25/af_x25.c +4 -4
@@ -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;
                }
net/x25/x25_out.c +5 -3
index d00649f..f1a6ff1 100644
@@ -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

#30 lore raw

Re: [PATCH 12/20] x25: remove the BKL

Quoted 5 lines · Andrew Hendry

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:

Quoted 10 lines
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)

Quoted 13 lines
        /* 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.

Quoted 13 lines

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);
Quoted 1 line

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:

Quoted 11 lines

net/x25/af_x25.c static int x25_sendmsg(struct kiocb *iocb, struct socket *sock, · line 1147

Quoted 11 lines
net/x25/af_x25.c +3 -2
@@ -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

#31 lore raw

PATCH v2 1/1 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.

Compile-tested only.

---
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(-)
net/x25/Kconfig +0 -1
index 2196e55..e6759c9 100644
@@ -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
net/x25/af_x25.c +18 -40

159 lines not on this page · raw message

net/x25/x25_out.c +5 -2
index d00649f..f1a6ff1 100644
@@ -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;
#32 lore raw

Re: [PATCH v2] x25: remove the BKL

Quoted 8 lines · Arnd Bergmann
net/x25/x25_out.c +0 -0
index d00649f..f1a6ff1 100644
@@ -68,8 +68,11 @@ int x25_output(struct sock *sk, struct sk_buff *skb)
 		frontlen = skb_headroom(skb);
 
 		while (skb->len > 0) {

net/x25/x25_out.c int x25_output(struct sock *sk, struct sk_buff *skb) · line 78

Quoted 10 lines
net/x25/x25_out.c +5 -2
-			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() ?

#33 lore raw

Re: [PATCH v2] x25: remove the BKL

Quoted 22 lines · Eric Dumazet
Quoted 8 lines · Arnd Bergmann
net/x25/x25_out.c +0 -0
index d00649f..f1a6ff1 100644
@@ -68,8 +68,11 @@ int x25_output(struct sock *sk, struct sk_buff *skb)
              frontlen = skb_headroom(skb);
 
              while (skb->len > 0) {

net/x25/x25_out.c int x25_output(struct sock *sk, struct sk_buff *skb) · line 78

Quoted 10 lines
net/x25/x25_out.c +5 -2
-                     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 :

Quoted 3 lines

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

#34 lore raw

PATCH 13/20 appletalk: move to staging

In reply to Arnd Bergmann

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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 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%)
MAINTAINERS +1 -2
index fe5ca5f..5021051 100644
@@ -554,8 +554,7 @@ F:	drivers/hwmon/applesmc.c
 APPLETALK NETWORK LAYER
 M:	Arnaldo Carvalho de Melo <[email protected]>
 S:	Maintained
-F:	drivers/net/appletalk/
-F:	net/appletalk/
+F:	drivers/staging/appletalk/
 
 ARC FRAMEBUFFER DRIVER
 M:	Jaya Kumar <[email protected]>
drivers/net/Makefile +0 -1
index b90738d..11a9c05 100644
@@ -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/
drivers/net/appletalk/Makefile deleted +0 -7
deleted file mode 100644
index 6cfc705..0000000
@@ -1,7 +0,0 @@
-#
-# Makefile for drivers/net/appletalk
-#
-
-obj-$(CONFIG_IPDDP) += ipddp.o
-obj-$(CONFIG_COPS) += cops.o
-obj-$(CONFIG_LTPC) += ltpc.o
drivers/staging/Kconfig +2 -0
index 9fc5aa6..04514b5 100644
@@ -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"
drivers/staging/Makefile +1 -0
index 878f381..0ec94e8 100644
@@ -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/
drivers/staging/appletalk/Kconfig renamed from drivers/net/appletalk/Kconfig +0 -0
similarity index 100%
rename from drivers/net/appletalk/Kconfig
rename to drivers/staging/appletalk/Kconfig
drivers/staging/appletalk/Makefile renamed from net/appletalk/Makefile +5 -2
similarity index 56%
rename from net/appletalk/Makefile
rename to drivers/staging/appletalk/Makefile
index 5cda56e..2a5129a 100644
@@ -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
drivers/staging/appletalk/aarp.c renamed from net/appletalk/aarp.c +1 -1
similarity index 99%
rename from net/appletalk/aarp.c
rename to drivers/staging/appletalk/aarp.c
index 50dce79..7163a1d 100644
@@ -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>
drivers/staging/appletalk/atalk.h renamed from include/linux/atalk.h +0 -0
similarity index 100%
rename from include/linux/atalk.h
rename to drivers/staging/appletalk/atalk.h
drivers/staging/appletalk/atalk_proc.c renamed from net/appletalk/atalk_proc.c +1 -1
similarity index 99%
rename from net/appletalk/atalk_proc.c
rename to drivers/staging/appletalk/atalk_proc.c
index 6ef0e76..d012ba2 100644
@@ -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)
drivers/staging/appletalk/cops.c renamed from drivers/net/appletalk/cops.c +1 -1
similarity index 99%
rename from drivers/net/appletalk/cops.c
rename to drivers/staging/appletalk/cops.c
index 748c9f5..661d42e 100644
@@ -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. */
drivers/staging/appletalk/cops.h renamed from drivers/net/appletalk/cops.h +0 -0
similarity index 100%
rename from drivers/net/appletalk/cops.h
rename to drivers/staging/appletalk/cops.h
drivers/staging/appletalk/cops_ffdrv.h renamed from drivers/net/appletalk/cops_ffdrv.h +0 -0
similarity index 100%
rename from drivers/net/appletalk/cops_ffdrv.h
rename to drivers/staging/appletalk/cops_ffdrv.h
drivers/staging/appletalk/cops_ltdrv.h renamed from drivers/net/appletalk/cops_ltdrv.h +0 -0
similarity index 100%
rename from drivers/net/appletalk/cops_ltdrv.h
rename to drivers/staging/appletalk/cops_ltdrv.h
drivers/staging/appletalk/ddp.c renamed from net/appletalk/ddp.c +2 -2
similarity index 99%
rename from net/appletalk/ddp.c
rename to drivers/staging/appletalk/ddp.c
index c410b93..940dd19 100644
@@ -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;
drivers/staging/appletalk/dev.c renamed from net/appletalk/dev.c +0 -0
similarity index 100%
rename from net/appletalk/dev.c
rename to drivers/staging/appletalk/dev.c
drivers/staging/appletalk/ipddp.c renamed from drivers/net/appletalk/ipddp.c +1 -1
similarity index 99%
rename from drivers/net/appletalk/ipddp.c
rename to drivers/staging/appletalk/ipddp.c
index 10d0dba..58b4e60 100644
@@ -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 <[email protected]>\n";
drivers/staging/appletalk/ipddp.h renamed from drivers/net/appletalk/ipddp.h +0 -0
similarity index 100%
rename from drivers/net/appletalk/ipddp.h
rename to drivers/staging/appletalk/ipddp.h
drivers/staging/appletalk/ltpc.c renamed from drivers/net/appletalk/ltpc.c +1 -1
similarity index 99%
rename from drivers/net/appletalk/ltpc.c
rename to drivers/staging/appletalk/ltpc.c
index e69eead..60caf89 100644
@@ -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);
drivers/staging/appletalk/ltpc.h renamed from drivers/net/appletalk/ltpc.h +0 -0
similarity index 100%
rename from drivers/net/appletalk/ltpc.h
rename to drivers/staging/appletalk/ltpc.h
drivers/staging/appletalk/sysctl_net_atalk.c renamed from net/appletalk/sysctl_net_atalk.c +1 -1
similarity index 98%
rename from net/appletalk/sysctl_net_atalk.c
rename to drivers/staging/appletalk/sysctl_net_atalk.c
index 04e9c0d..4c896b6 100644
@@ -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[] = {
 	{
fs/compat_ioctl.c +0 -1
index 61abb63..86a2d7d 100644
@@ -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>
include/linux/Kbuild +0 -1
index 2296d8b..362041b 100644
@@ -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
net/Kconfig +0 -1
index 7284062..082c8bc 100644
@@ -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"
net/Makefile +0 -1
index a3330eb..16d9947 100644
@@ -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/
net/socket.c +0 -1
index ac2219f..26f7bcf 100644
@@ -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,
Signature · 1 line
1.7.1
#35 lore raw

PATCH 14/20 staging/appletalk: remove the BKL

In reply to Arnd Bergmann

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.

Cc Arnaldo Carvalho de Melo <[email protected]>
---
 drivers/staging/appletalk/Kconfig |    1 -
 drivers/staging/appletalk/ddp.c   |   40 ++++++++++++++----------------------
 2 files changed, 16 insertions(+), 25 deletions(-)
drivers/staging/appletalk/Kconfig +0 -1
index 0b376a9..f5a8916 100644
@@ -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
drivers/staging/appletalk/ddp.c +16 -24
index 940dd19..0e2240c 100644
@@ -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,
Signature · 1 line
1.7.1
maintainer
#36 lore raw

Re: [PATCH 14/20] staging/appletalk: remove the BKL

From: Arnd Bergmann <[email protected]>
Date: Tue, 25 Jan 2011 23:17:28 +0100

Quoted 9 lines

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.

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.

#37 lore raw

Re: [PATCH 14/20] staging/appletalk: remove the BKL

Quoted 8 lines · David Miller

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

#38 lore raw

PATCH 15/20 ufs: remove the BKL

In reply to Arnd Bergmann

This introduces a new per-superblock mutex in UFS to replace the big kernel lock. I have been careful to avoid nested calls to lock_ufs and to get the lock order right with respect to other mutexes, in particular lock_super.

I did not make any attempt to prove that the big kernel lock is not needed in a particular place in the code, which is very possible.

The code is still only compile-tested, but it should at least be harmless on non-SMP systems, since the new mutex is not taken on those.

Cc Nick Bowler <[email protected]>
Cc Evgeniy Dushistov <[email protected]>
---
 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 +++-
 6 files changed, 77 insertions(+), 103 deletions(-)
fs/ufs/Kconfig +0 -1
index 30c8f22..e4f10a4 100644
@@ -1,7 +1,6 @@
 config UFS_FS
 	tristate "UFS file system support (read only)"
 	depends on BLOCK
-	depends on BKL # probably fixable
 	help
 	  BSD and derivate versions of Unix (such as SunOS, FreeBSD, NetBSD,
 	  OpenBSD and NeXTstep) use a file system called UFS. Some System V
fs/ufs/inode.c +21 -57

163 lines not on this page · raw message

fs/ufs/namei.c +17 -18
index 12f39b9..205030a 100644
@@ -29,7 +29,6 @@
 
 #include <linux/time.h>
 #include <linux/fs.h>
-#include <linux/smp_lock.h>
 
 #include "ufs_fs.h"
 #include "ufs.h"
@@ -55,16 +54,16 @@ static struct dentry *ufs_lookup(struct inode * dir, struct dentry *dentry, stru
 	if (dentry->d_name.len > UFS_MAXNAMLEN)
 		return ERR_PTR(-ENAMETOOLONG);
 
-	lock_kernel();
+	lock_ufs(dir->i_sb);
 	ino = ufs_inode_by_name(dir, &dentry->d_name);
 	if (ino) {
 		inode = ufs_iget(dir->i_sb, ino);
 		if (IS_ERR(inode)) {
-			unlock_kernel();
+			unlock_ufs(dir->i_sb);
 			return ERR_CAST(inode);
 		}
 	}
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 	d_add(dentry, inode);
 	return NULL;
 }
@@ -93,9 +92,9 @@ static int ufs_create (struct inode * dir, struct dentry * dentry, int mode,
 		inode->i_fop = &ufs_file_operations;
 		inode->i_mapping->a_ops = &ufs_aops;
 		mark_inode_dirty(inode);
-		lock_kernel();
+		lock_ufs(dir->i_sb);
 		err = ufs_add_nondir(dentry, inode);
-		unlock_kernel();
+		unlock_ufs(dir->i_sb);
 	}
 	UFSD("END: err=%d\n", err);
 	return err;
@@ -115,9 +114,9 @@ static int ufs_mknod (struct inode * dir, struct dentry *dentry, int mode, dev_t
 		init_special_inode(inode, mode, rdev);
 		ufs_set_inode_dev(inode->i_sb, UFS_I(inode), rdev);
 		mark_inode_dirty(inode);
-		lock_kernel();
+		lock_ufs(dir->i_sb);
 		err = ufs_add_nondir(dentry, inode);
-		unlock_kernel();
+		unlock_ufs(dir->i_sb);
 	}
 	return err;
 }
@@ -133,7 +132,7 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,
 	if (l > sb->s_blocksize)
 		goto out_notlocked;
 
-	lock_kernel();
+	lock_ufs(dir->i_sb);
 	inode = ufs_new_inode(dir, S_IFLNK | S_IRWXUGO);
 	err = PTR_ERR(inode);
 	if (IS_ERR(inode))
@@ -156,7 +155,7 @@ static int ufs_symlink (struct inode * dir, struct dentry * dentry,
 
 	err = ufs_add_nondir(dentry, inode);
 out:
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 out_notlocked:
 	return err;
 
@@ -172,9 +171,9 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
 	struct inode *inode = old_dentry->d_inode;
 	int error;
 
-	lock_kernel();
+	lock_ufs(dir->i_sb);
 	if (inode->i_nlink >= UFS_LINK_MAX) {
-		unlock_kernel();
+		unlock_ufs(dir->i_sb);
 		return -EMLINK;
 	}
 
@@ -183,7 +182,7 @@ static int ufs_link (struct dentry * old_dentry, struct inode * dir,
 	ihold(inode);
 
 	error = ufs_add_nondir(dentry, inode);
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 	return error;
 }
 
@@ -195,7 +194,7 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
 	if (dir->i_nlink >= UFS_LINK_MAX)
 		goto out;
 
-	lock_kernel();
+	lock_ufs(dir->i_sb);
 	inode_inc_link_count(dir);
 
 	inode = ufs_new_inode(dir, S_IFDIR|mode);
@@ -216,7 +215,7 @@ static int ufs_mkdir(struct inode * dir, struct dentry * dentry, int mode)
 	err = ufs_add_link(dentry, inode);
 	if (err)
 		goto out_fail;
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 
 	d_instantiate(dentry, inode);
 out:
@@ -228,7 +227,7 @@ out_fail:
 	iput (inode);
 out_dir:
 	inode_dec_link_count(dir);
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 	goto out;
 }
 
@@ -259,7 +258,7 @@ static int ufs_rmdir (struct inode * dir, struct dentry *dentry)
 	struct inode * inode = dentry->d_inode;
 	int err= -ENOTEMPTY;
 
-	lock_kernel();
+	lock_ufs(dir->i_sb);
 	if (ufs_empty_dir (inode)) {
 		err = ufs_unlink(dir, dentry);
 		if (!err) {
@@ -268,7 +267,7 @@ static int ufs_rmdir (struct inode * dir, struct dentry *dentry)
 			inode_dec_link_count(dir);
 		}
 	}
-	unlock_kernel();
+	unlock_ufs(dir->i_sb);
 	return err;
 }
 
fs/ufs/super.c +32 -23

180 lines not on this page · raw message

fs/ufs/truncate.c +2 -3
index a58f915..e56a4f5 100644
@@ -40,7 +40,6 @@
 #include <linux/time.h>
 #include <linux/stat.h>
 #include <linux/string.h>
-#include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/blkdev.h>
 #include <linux/sched.h>
@@ -467,7 +466,6 @@ int ufs_truncate(struct inode *inode, loff_t old_i_size)
 
 	block_truncate_page(inode->i_mapping, inode->i_size, ufs_getfrag_block);
 
-	lock_kernel();
 	while (1) {
 		retry = ufs_trunc_direct(inode);
 		retry |= ufs_trunc_indirect(inode, UFS_IND_BLOCK,
@@ -487,7 +485,6 @@ int ufs_truncate(struct inode *inode, loff_t old_i_size)
 
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
 	ufsi->i_lastfrag = DIRECT_FRAGMENT;
-	unlock_kernel();
 	mark_inode_dirty(inode);
 out:
 	UFSD("EXIT: err %d\n", err);
@@ -510,7 +507,9 @@ int ufs_setattr(struct dentry *dentry, struct iattr *attr)
 		/* XXX(truncate): truncate_setsize should be called last */
 		truncate_setsize(inode, attr->ia_size);
 
+		lock_ufs(inode->i_sb);
 		error = ufs_truncate(inode, old_i_size);
+		unlock_ufs(inode->i_sb);
 		if (error)
 			return error;
 	}
fs/ufs/ufs.h +5 -1
index c08782e..5be2755 100644
@@ -18,6 +18,8 @@ struct ufs_sb_info {
 	unsigned s_cgno[UFS_MAX_GROUP_LOADED];
 	unsigned short s_cg_loaded;
 	unsigned s_mount_opt;
+	struct mutex mutex;
+	struct task_struct *mutex_owner;
 };
 
 struct ufs_inode_info {
@@ -109,7 +111,6 @@ extern struct inode *ufs_iget(struct super_block *, unsigned long);
 extern int ufs_write_inode (struct inode *, struct writeback_control *);
 extern int ufs_sync_inode (struct inode *);
 extern void ufs_evict_inode (struct inode *);
-extern struct buffer_head * ufs_bread (struct inode *, unsigned, int, int *);
 extern int ufs_getfrag_block (struct inode *inode, sector_t fragment, struct buffer_head *bh_result, int create);
 
 /* namei.c */
@@ -154,4 +155,7 @@ static inline u32 ufs_dtogd(struct ufs_sb_private_info * uspi, u64 b)
 	return do_div(b, uspi->s_fpg);
 }
 
+extern void lock_ufs(struct super_block *sb);
+extern void unlock_ufs(struct super_block *sb);
+
 #endif /* _UFS_UFS_H */
Signature · 1 line
1.7.1
#39 lore raw

Re: [PATCH 15/20] ufs: remove the BKL

Quoted 8 lines · Arnd Bergmann

This introduces a new per-superblock mutex in UFS to replace the big kernel lock. I have been careful to avoid nested calls to lock_ufs and to get the lock order right with respect to other mutexes, in particular lock_super.

The code is still only compile-tested, but it should at least be harmless on non-SMP systems, since the new mutex is not taken on those.

[...]

Quoted 9 lines
fs/ufs/super.c +9 -0
+void lock_ufs(struct super_block *sb)
+{
+#if defined(CONFIG_SMP) || defined (CONFIG_PREEMPT)
+	struct ufs_sb_info *sbi = UFS_SB(sb);
+
+	mutex_lock(&sbi->mutex);
+	sbi->mutex_owner = current;
+#endif
+}

fs/ufs/super.c line 116

Quoted 10 lines
fs/ufs/super.c +10 -0
+
+void unlock_ufs(struct super_block *sb)
+{
+#if defined(CONFIG_SMP) || defined (CONFIG_PREEMPT)
+	struct ufs_sb_info *sbi = UFS_SB(sb);
+
+	sbi->mutex_owner = NULL;
+	mutex_unlock(&sbi->mutex);
+#endif
+}

The sbi->mutex lock used by these functions is never initialized, which causes an insta-crash on mount. With a mutex_init call added to ufs_fill_super, there are no obvious problems with my read-only mounts after some (very light) testing.

Signature · 1 line
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
#40 lore raw

Re: [PATCH 15/20] ufs: remove the BKL

Quoted 4 lines · Nick Bowler

The sbi->mutex lock used by these functions is never initialized, which causes an insta-crash on mount. With a mutex_init call added to ufs_fill_super, there are no obvious problems with my read-only mounts after some (very light) testing.

Thanks for the testing!

I fixed it up now and pushed the new version to the "final" branch of my
git://git.kernel.org/pub/scm/linux/kernel/git/arnd/bkl.git tree.

Arnd

#41 lore raw

Re: [PATCH 15/20] ufs: remove the BKL

In reply to Arnd Bergmann

Really great work in removing BKL, Arnd. It's awesome work and I'm sure a lot of it was pretty thankless along the way.

Quoted 4 lines · Arnd Bergmann

This introduces a new per-superblock mutex in UFS to replace the big kernel lock. I have been careful to avoid nested calls to lock_ufs and to get the lock order right with respect to other mutexes, in particular lock_super.

When I looked at removing bkl from minix a long time ago, I was a bit worried about reclaim and fs/io recursion in some of the filesystems with bkl.

Quoted 10 lines
fs/ufs/inode.c +2 -1
@@ -436,7 +439,8 @@ int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head
        ret = 0;
        bh = NULL;
 
-       lock_kernel();
+       if (needs_lock)
+               lock_ufs(sb);
 
        UFSD("ENTER, ino %lu, fragment %llu\n", inode->i_ino, (unsigned long long)fragment);

       if (fragment >

[...]

Quoted 10 lines
fs/ufs/namei.c +1 -1
@@ -55,16 +54,16 @@ static struct dentry *ufs_lookup(struct inode * dir, struct dentry *dentry, stru
        if (dentry->d_name.len > UFS_MAXNAMLEN)
                return ERR_PTR(-ENAMETOOLONG);
 
-       lock_kernel();
+       lock_ufs(dir->i_sb);
        ino = ufs_inode_by_name(dir, &dentry->d_name);
        if (ino) {
                inode = ufs_iget(dir->i_sb, ino);
                if (IS_ERR(inode)) {

fs/ufs/namei.c static struct dentry *ufs_lookup(struct inode * dir, struct dentry *dentry, stru · line 69

Quoted 10 lines
fs/ufs/namei.c +2 -2
-                       unlock_kernel();
+                       unlock_ufs(dir->i_sb);
                        return ERR_CAST(inode);
                }
        }
-       unlock_kernel();
+       unlock_ufs(dir->i_sb);
        d_add(dentry, inode);
        return NULL;
  }

versus

1405static struct inode *ufs_alloc_inode(struct super_block *sb) 1406{
1407 struct ufs_inode_info *ei;
1408 ei = (struct ufs_inode_info
*)kmem_cache_alloc(ufs_inode_cachep, GFP_KERNEL);
1409 if (!ei)
1410 return NULL;
1411 ei->vfs_inode.i_version = 1;
1412 return &ei->vfs_inode;
1413}

So, get_block can be called for .writepage in page reclaim, which takes the lock. ufs_lookup takes the lock and winds up calling ufs_alloc_inode. And ufs_alloc_inode does GFP_KERNEL, which can enter reclaim with __GFP_FS set.

I didn't look through all your filesystem conversions, but it is something tricky to watch out for I think.

Changing everything to GFP_NOFS may be an option, for such crufty old filesystems...

#42 lore raw

Re: [PATCH 15/20] ufs: remove the BKL

Quoted 9 lines · Nick Piggin
Quoted 4 lines · Arnd Bergmann

This introduces a new per-superblock mutex in UFS to replace the big kernel lock. I have been careful to avoid nested calls to lock_ufs and to get the lock order right with respect to other mutexes, in particular lock_super.

When I looked at removing bkl from minix a long time ago, I was a bit worried about reclaim and fs/io recursion in some of the filesystems with bkl.

Thanks for looking at this, you certainly understand these interactions much better than me!

Quoted 8 lines
Quoted 8 lines
(quoted diff) +2 -1
@@ -436,7 +439,8 @@ int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head
       ret = 0;
       bh = NULL;
 
-       lock_kernel();
+       if (needs_lock)
+               lock_ufs(sb);
 
Quoted 9 lines

Review int ufs_getfrag_block(struct inode *inode, sector_t fragment, struct buffer_head · line 446

Quoted 2 lines
(quoted diff) +0 -0
       UFSD("ENTER, ino %lu, fragment %llu\n", inode->i_ino, (unsigned long long)fragment);
       if (fragment >

So, get_block can be called for .writepage in page reclaim, which takes the lock. ufs_lookup takes the lock and winds up calling ufs_alloc_inode. And ufs_alloc_inode does GFP_KERNEL, which can enter reclaim with __GFP_FS set.

This is one path that I did not consider. My initial thought when you mentioned it was that this is still fine because ufs_getfrag_block can be called with the lock held by the current task, but you are of course right that this won't work when we are called by the writeback task.

Quoted 2 lines

I didn't look through all your filesystem conversions, but it is something tricky to watch out for I think.

The only other one where I added locks is isofs, and I'm pretty sure that's fine because it's read-only and does not do writeback. HPFS with Andi's suggested solution of making it UP-only is of course also fine.

Quoted 2 lines

Changing everything to GFP_NOFS may be an option, for such crufty old filesystems...

Yes. All performance optimizations in UFS are pointless anyway as long as we do most of the disk I/O under a global mutex. The patch below converts all allocations that are done under a lock in UFS to GFP_NOFS. If nobody comes up with a better solution, I'll fold that into my other patch.

Arnd
---

 fs/ufs/super.c |    8 ++++----
 fs/ufs/util.c  |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
fs/ufs/super.c +4 -4
index e4e06ef..7693d62 100644
@@ -539,7 +539,7 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
 	 */
 	size = uspi->s_cssize;
 	blks = (size + uspi->s_fsize - 1) >> uspi->s_fshift;
-	base = space = kmalloc(size, GFP_KERNEL);
+	base = space = kmalloc(size, GFP_NOFS);
 	if (!base)
 		goto failed; 
 	sbi->s_csp = (struct ufs_csum *)space;
@@ -564,7 +564,7 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
 	 * Read cylinder group (we read only first fragment from block
 	 * at this time) and prepare internal data structures for cg caching.
 	 */
-	if (!(sbi->s_ucg = kmalloc (sizeof(struct buffer_head *) * uspi->s_ncg, GFP_KERNEL)))
+	if (!(sbi->s_ucg = kmalloc (sizeof(struct buffer_head *) * uspi->s_ncg, GFP_NOFS)))
 		goto failed;
 	for (i = 0; i < uspi->s_ncg; i++) 
 		sbi->s_ucg[i] = NULL;
@@ -582,7 +582,7 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
 		ufs_print_cylinder_stuff(sb, (struct ufs_cylinder_group *) sbi->s_ucg[i]->b_data);
 	}
 	for (i = 0; i < UFS_MAX_GROUP_LOADED; i++) {
-		if (!(sbi->s_ucpi[i] = kmalloc (sizeof(struct ufs_cg_private_info), GFP_KERNEL)))
+		if (!(sbi->s_ucpi[i] = kmalloc (sizeof(struct ufs_cg_private_info), GFP_NOFS)))
 			goto failed;
 		sbi->s_cgno[i] = UFS_CGNO_EMPTY;
 	}
@@ -1415,7 +1415,7 @@ static struct kmem_cache * ufs_inode_cachep;
 static struct inode *ufs_alloc_inode(struct super_block *sb)
 {
 	struct ufs_inode_info *ei;
-	ei = (struct ufs_inode_info *)kmem_cache_alloc(ufs_inode_cachep, GFP_KERNEL);
+	ei = (struct ufs_inode_info *)kmem_cache_alloc(ufs_inode_cachep, GFP_NOFS);
 	if (!ei)
 		return NULL;
 	ei->vfs_inode.i_version = 1;
fs/ufs/util.c +1 -1
index d2c36d5..95425b5 100644
@@ -27,7 +27,7 @@ struct ufs_buffer_head * _ubh_bread_ (struct ufs_sb_private_info * uspi,
 	if (count > UFS_MAXFRAG)
 		return NULL;
 	ubh = (struct ufs_buffer_head *)
-		kmalloc (sizeof (struct ufs_buffer_head), GFP_KERNEL);
+		kmalloc (sizeof (struct ufs_buffer_head), GFP_NOFS);
 	if (!ubh)
 		return NULL;
 	ubh->fragment = fragment;
#43 lore raw

PATCH 16/20 ipx: remove the BKL

In reply to Arnd Bergmann

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.

Cc Arnaldo Carvalho de Melo <[email protected]>
---
 net/ipx/Kconfig  |    1 -
 net/ipx/af_ipx.c |   52 ++++++++++++++++++++--------------------------------
 2 files changed, 20 insertions(+), 33 deletions(-)
net/ipx/Kconfig +0 -1
index 02549cb..e9ad006 100644
@@ -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
net/ipx/af_ipx.c +20 -32

169 lines not on this page · raw message

Signature · 1 line
1.7.1
#44 lore raw

PATCH 17/20 tracing: don't trace the BKL

In reply to Arnd Bergmann

No reason to trace it when the last user is gone.

Cc Frederic Weisbecker <[email protected]>
---
 include/trace/events/bkl.h |   61 --------------------------------------------
 lib/kernel_lock.c          |    7 -----
 2 files changed, 0 insertions(+), 68 deletions(-)
 delete mode 100644 include/trace/events/bkl.h
include/trace/events/bkl.h deleted +0 -61
deleted file mode 100644
index 1af72dc..0000000
@@ -1,61 +0,0 @@
-#undef TRACE_SYSTEM
-#define TRACE_SYSTEM bkl
-
-#if !defined(_TRACE_BKL_H) || defined(TRACE_HEADER_MULTI_READ)
-#define _TRACE_BKL_H
-
-#include <linux/tracepoint.h>
-
-TRACE_EVENT(lock_kernel,
-
-	TP_PROTO(const char *func, const char *file, int line),
-
-	TP_ARGS(func, file, line),
-
-	TP_STRUCT__entry(
-		__field(	int,		depth			)
-		__field_ext(	const char *,	func, FILTER_PTR_STRING	)
-		__field_ext(	const char *,	file, FILTER_PTR_STRING	)
-		__field(	int,		line			)
-	),
-
-	TP_fast_assign(
-		/* We want to record the lock_depth after lock is acquired */
-		__entry->depth = current->lock_depth + 1;
-		__entry->func = func;
-		__entry->file = file;
-		__entry->line = line;
-	),
-
-	TP_printk("depth=%d file:line=%s:%d func=%s()", __entry->depth,
-		  __entry->file, __entry->line, __entry->func)
-);
-
-TRACE_EVENT(unlock_kernel,
-
-	TP_PROTO(const char *func, const char *file, int line),
-
-	TP_ARGS(func, file, line),
-
-	TP_STRUCT__entry(
-		__field(int,		depth		)
-		__field(const char *,	func		)
-		__field(const char *,	file		)
-		__field(int,		line		)
-	),
-
-	TP_fast_assign(
-		__entry->depth = current->lock_depth;
-		__entry->func = func;
-		__entry->file = file;
-		__entry->line = line;
-	),
-
-	TP_printk("depth=%d file:line=%s:%d func=%s()", __entry->depth,
-		  __entry->file, __entry->line, __entry->func)
-);
-
-#endif /* _TRACE_BKL_H */
-
-/* This part must be outside protection */
-#include <trace/define_trace.h>
lib/kernel_lock.c +0 -7
index b135d04..d80e122 100644
@@ -10,9 +10,6 @@
 #include <linux/semaphore.h>
 #include <linux/smp_lock.h>
 
-#define CREATE_TRACE_POINTS
-#include <trace/events/bkl.h>
-
 /*
  * The 'big kernel lock'
  *
@@ -120,8 +117,6 @@ void __lockfunc _lock_kernel(const char *func, const char *file, int line)
 {
 	int depth = current->lock_depth + 1;
 
-	trace_lock_kernel(func, file, line);
-
 	if (likely(!depth)) {
 		might_sleep();
 		__lock_kernel();
@@ -134,8 +129,6 @@ void __lockfunc _unlock_kernel(const char *func, const char *file, int line)
 	BUG_ON(current->lock_depth < 0);
 	if (likely(--current->lock_depth < 0))
 		__unlock_kernel();
-
-	trace_unlock_kernel(func, file, line);
 }
 
 EXPORT_SYMBOL(_lock_kernel);
Signature · 1 line
1.7.1
Acked-by on v1 17/20
#45 lore raw

Re: [PATCH 17/20] tracing: don't trace the BKL

Quoted 3 lines · Arnd Bergmann

No reason to trace it when the last user is gone.

You could also remove file and line arguments from _(un)lock_kernel but as the final removal follows, we just don't care.

Tell us if you want that in the tracing tree. But I guess this should be included in the same set than the rest or?

Thanks!

#46 lore raw

PATCH 18/20 rtmutex-tester: remove BKL tests

In reply to Arnd Bergmann

The BKL is going away, no need to test it any more. I left the definitions of the test case numbers in, so that the other tests do not get renumbered.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Andrew Morton <[email protected]>
---
 kernel/rtmutex-tester.c |   39 ++++-----------------------------------
 1 files changed, 4 insertions(+), 35 deletions(-)
kernel/rtmutex-tester.c +4 -35
index 66cb89b..d5b5435 100644
@@ -9,7 +9,6 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/spinlock.h>
 #include <linux/sysdev.h>
 #include <linux/timer.h>
@@ -27,7 +26,6 @@ struct test_thread_data {
 	int			opcode;
 	int			opdata;
 	int			mutexes[MAX_RT_TEST_MUTEXES];
-	int			bkl;
 	int			event;
 	struct sys_device	sysdev;
 };
@@ -46,8 +44,8 @@ enum test_opcodes {
 	RTTEST_LOCKINTNOWAIT,	/* 6 Lock interruptible no wait in wakeup, data = lockindex */
 	RTTEST_LOCKCONT,	/* 7 Continue locking after the wakeup delay */
 	RTTEST_UNLOCK,		/* 8 Unlock, data = lockindex */
-	RTTEST_LOCKBKL,		/* 9 Lock BKL */
-	RTTEST_UNLOCKBKL,	/* 10 Unlock BKL */
+	RTTEST_LOCKBKL,		/* 9 Was: Lock BKL */
+	RTTEST_UNLOCKBKL,	/* 10 Was: Unlock BKL */
 	RTTEST_SIGNAL,		/* 11 Signal other test thread, data = thread id */
 	RTTEST_RESETEVENT = 98,	/* 98 Reset event counter */
 	RTTEST_RESET = 99,	/* 99 Reset all pending operations */
@@ -74,13 +72,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 				td->mutexes[i] = 0;
 			}
 		}
-
-		if (!lockwakeup && td->bkl == 4) {
-#ifdef CONFIG_LOCK_KERNEL
-			unlock_kernel();
-#endif
-			td->bkl = 0;
-		}
 		return 0;
 
 	case RTTEST_RESETEVENT:
@@ -131,25 +122,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 		td->mutexes[id] = 0;
 		return 0;
 
-	case RTTEST_LOCKBKL:
-		if (td->bkl)
-			return 0;
-		td->bkl = 1;
-#ifdef CONFIG_LOCK_KERNEL
-		lock_kernel();
-#endif
-		td->bkl = 4;
-		return 0;
-
-	case RTTEST_UNLOCKBKL:
-		if (td->bkl != 4)
-			break;
-#ifdef CONFIG_LOCK_KERNEL
-		unlock_kernel();
-#endif
-		td->bkl = 0;
-		return 0;
-
 	default:
 		break;
 	}
@@ -196,7 +168,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		break;
 
-	case RTTEST_LOCKBKL:
 	default:
 		break;
 	}
@@ -229,8 +200,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		return;
 
-	case RTTEST_LOCKBKL:
-		return;
 	default:
 		return;
 	}
@@ -380,11 +349,11 @@ static ssize_t sysfs_test_status(struct sys_device *dev, struct sysdev_attribute
 	spin_lock(&rttest_lock);
 
 	curr += sprintf(curr,
-		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, K: %d, M:",
+		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, M:",
 		td->opcode, td->event, tsk->state,
 			(MAX_RT_PRIO - 1) - tsk->prio,
 			(MAX_RT_PRIO - 1) - tsk->normal_prio,
-		tsk->pi_blocked_on, td->bkl);
+		tsk->pi_blocked_on);
 
 	for (i = MAX_RT_TEST_MUTEXES - 1; i >=0 ; i--)
 		curr += sprintf(curr, "%d", td->mutexes[i]);
Signature · 1 line
1.7.1
#47 lore raw

[tip:core/locking] rtmutex-tester: Remove BKL tests

Commit-ID:  ccaa8d657117bb1876d471bd91579d774106778d
Gitweb:     http://git.kernel.org/tip/ccaa8d657117bb1876d471bd91579d774106778d
Author:     Arnd Bergmann <[email protected]>
AuthorDate: Tue, 25 Jan 2011 23:17:32 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 26 Jan 2011 15:58:12 +0100

rtmutex-tester: Remove BKL tests

The BKL is going away, no need to test it any more. I left the definitions of the test case numbers in, so that the other tests do not get renumbered.

Cc Arjan van de Ven <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>

LKML-Reference: <[email protected]>

---
 kernel/rtmutex-tester.c |   39 ++++-----------------------------------
 1 files changed, 4 insertions(+), 35 deletions(-)
kernel/rtmutex-tester.c +4 -35
index 66cb89b..d5b5435 100644
@@ -9,7 +9,6 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/spinlock.h>
 #include <linux/sysdev.h>
 #include <linux/timer.h>
@@ -27,7 +26,6 @@ struct test_thread_data {
 	int			opcode;
 	int			opdata;
 	int			mutexes[MAX_RT_TEST_MUTEXES];
-	int			bkl;
 	int			event;
 	struct sys_device	sysdev;
 };
@@ -46,8 +44,8 @@ enum test_opcodes {
 	RTTEST_LOCKINTNOWAIT,	/* 6 Lock interruptible no wait in wakeup, data = lockindex */
 	RTTEST_LOCKCONT,	/* 7 Continue locking after the wakeup delay */
 	RTTEST_UNLOCK,		/* 8 Unlock, data = lockindex */
-	RTTEST_LOCKBKL,		/* 9 Lock BKL */
-	RTTEST_UNLOCKBKL,	/* 10 Unlock BKL */
+	RTTEST_LOCKBKL,		/* 9 Was: Lock BKL */
+	RTTEST_UNLOCKBKL,	/* 10 Was: Unlock BKL */
 	RTTEST_SIGNAL,		/* 11 Signal other test thread, data = thread id */
 	RTTEST_RESETEVENT = 98,	/* 98 Reset event counter */
 	RTTEST_RESET = 99,	/* 99 Reset all pending operations */
@@ -74,13 +72,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 				td->mutexes[i] = 0;
 			}
 		}
-
-		if (!lockwakeup && td->bkl == 4) {
-#ifdef CONFIG_LOCK_KERNEL
-			unlock_kernel();
-#endif
-			td->bkl = 0;
-		}
 		return 0;
 
 	case RTTEST_RESETEVENT:
@@ -131,25 +122,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 		td->mutexes[id] = 0;
 		return 0;
 
-	case RTTEST_LOCKBKL:
-		if (td->bkl)
-			return 0;
-		td->bkl = 1;
-#ifdef CONFIG_LOCK_KERNEL
-		lock_kernel();
-#endif
-		td->bkl = 4;
-		return 0;
-
-	case RTTEST_UNLOCKBKL:
-		if (td->bkl != 4)
-			break;
-#ifdef CONFIG_LOCK_KERNEL
-		unlock_kernel();
-#endif
-		td->bkl = 0;
-		return 0;
-
 	default:
 		break;
 	}
@@ -196,7 +168,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		break;
 
-	case RTTEST_LOCKBKL:
 	default:
 		break;
 	}
@@ -229,8 +200,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		return;
 
-	case RTTEST_LOCKBKL:
-		return;
 	default:
 		return;
 	}
@@ -380,11 +349,11 @@ static ssize_t sysfs_test_status(struct sys_device *dev, struct sysdev_attribute
 	spin_lock(&rttest_lock);
 
 	curr += sprintf(curr,
-		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, K: %d, M:",
+		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, M:",
 		td->opcode, td->event, tsk->state,
 			(MAX_RT_PRIO - 1) - tsk->prio,
 			(MAX_RT_PRIO - 1) - tsk->normal_prio,
-		tsk->pi_blocked_on, td->bkl);
+		tsk->pi_blocked_on);
 
 	for (i = MAX_RT_TEST_MUTEXES - 1; i >=0 ; i--)
 		curr += sprintf(curr, "%d", td->mutexes[i]);
#48 lore raw

[tip:irq/core] rtmutex: tester: Remove BKL tests

In reply to Arnd Bergmann
Commit-ID:  fbabf8ca140fff2ddf60c23397d5892201e08095
Gitweb:     http://git.kernel.org/tip/fbabf8ca140fff2ddf60c23397d5892201e08095
Author:     Arnd Bergmann <[email protected]>
AuthorDate: Tue, 25 Jan 2011 23:17:32 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 22 Feb 2011 21:45:55 +0100

rtmutex: tester: Remove BKL tests

The BKL is going away, no need to test it any more. I left the definitions of the test case numbers in, so that the other tests do not get renumbered.

[ tglx: renamed the place holder enum members ]

Cc Arjan van de Ven <[email protected]>
Cc Andrew Morton <[email protected]>

LKML-Reference: <[email protected]>

---
 kernel/rtmutex-tester.c |   39 ++++-----------------------------------
 1 files changed, 4 insertions(+), 35 deletions(-)
kernel/rtmutex-tester.c +4 -35
index 66cb89b..100e2f0 100644
@@ -9,7 +9,6 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/spinlock.h>
 #include <linux/sysdev.h>
 #include <linux/timer.h>
@@ -27,7 +26,6 @@ struct test_thread_data {
 	int			opcode;
 	int			opdata;
 	int			mutexes[MAX_RT_TEST_MUTEXES];
-	int			bkl;
 	int			event;
 	struct sys_device	sysdev;
 };
@@ -46,8 +44,8 @@ enum test_opcodes {
 	RTTEST_LOCKINTNOWAIT,	/* 6 Lock interruptible no wait in wakeup, data = lockindex */
 	RTTEST_LOCKCONT,	/* 7 Continue locking after the wakeup delay */
 	RTTEST_UNLOCK,		/* 8 Unlock, data = lockindex */
-	RTTEST_LOCKBKL,		/* 9 Lock BKL */
-	RTTEST_UNLOCKBKL,	/* 10 Unlock BKL */
+	RTTEST_WAS_LOCKBKL,	/* 9 Was: Lock BKL */
+	RTTEST_WAS_UNLOCKBKL,	/* 10 Was: Unlock BKL */
 	RTTEST_SIGNAL,		/* 11 Signal other test thread, data = thread id */
 	RTTEST_RESETEVENT = 98,	/* 98 Reset event counter */
 	RTTEST_RESET = 99,	/* 99 Reset all pending operations */
@@ -74,13 +72,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 				td->mutexes[i] = 0;
 			}
 		}
-
-		if (!lockwakeup && td->bkl == 4) {
-#ifdef CONFIG_LOCK_KERNEL
-			unlock_kernel();
-#endif
-			td->bkl = 0;
-		}
 		return 0;
 
 	case RTTEST_RESETEVENT:
@@ -131,25 +122,6 @@ static int handle_op(struct test_thread_data *td, int lockwakeup)
 		td->mutexes[id] = 0;
 		return 0;
 
-	case RTTEST_LOCKBKL:
-		if (td->bkl)
-			return 0;
-		td->bkl = 1;
-#ifdef CONFIG_LOCK_KERNEL
-		lock_kernel();
-#endif
-		td->bkl = 4;
-		return 0;
-
-	case RTTEST_UNLOCKBKL:
-		if (td->bkl != 4)
-			break;
-#ifdef CONFIG_LOCK_KERNEL
-		unlock_kernel();
-#endif
-		td->bkl = 0;
-		return 0;
-
 	default:
 		break;
 	}
@@ -196,7 +168,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		break;
 
-	case RTTEST_LOCKBKL:
 	default:
 		break;
 	}
@@ -229,8 +200,6 @@ void schedule_rt_mutex_test(struct rt_mutex *mutex)
 		td->event = atomic_add_return(1, &rttest_event);
 		return;
 
-	case RTTEST_LOCKBKL:
-		return;
 	default:
 		return;
 	}
@@ -380,11 +349,11 @@ static ssize_t sysfs_test_status(struct sys_device *dev, struct sysdev_attribute
 	spin_lock(&rttest_lock);
 
 	curr += sprintf(curr,
-		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, K: %d, M:",
+		"O: %4d, E:%8d, S: 0x%08lx, P: %4d, N: %4d, B: %p, M:",
 		td->opcode, td->event, tsk->state,
 			(MAX_RT_PRIO - 1) - tsk->prio,
 			(MAX_RT_PRIO - 1) - tsk->normal_prio,
-		tsk->pi_blocked_on, td->bkl);
+		tsk->pi_blocked_on);
 
 	for (i = MAX_RT_TEST_MUTEXES - 1; i >=0 ; i--)
 		curr += sprintf(curr, "%d", td->mutexes[i]);
#49 lore raw

PATCH 19/20 drivers: remove extraneous includes of smp_lock.h

In reply to Arnd Bergmann

These were missed the last time I cleaned this up globally, because of code moving around or new code getting merged.

---
 drivers/scsi/megaraid/megaraid_sas_fp.c     |    1 -
 drivers/scsi/megaraid/megaraid_sas_fusion.c |    1 -
 drivers/staging/easycap/easycap.h           |    1 -
 drivers/staging/easycap/easycap_ioctl.c     |    1 -
 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 -
 22 files changed, 0 insertions(+), 23 deletions(-)
drivers/scsi/megaraid/megaraid_sas_fp.c +0 -1
index 53fa96a..8fe3a45 100644
@@ -39,7 +39,6 @@
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
-#include <linux/smp_lock.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
 #include <linux/fs.h>
drivers/scsi/megaraid/megaraid_sas_fusion.c +0 -1
index c1e09d5..d6e2a66 100644
@@ -38,7 +38,6 @@
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
-#include <linux/smp_lock.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
 #include <linux/fs.h>
drivers/staging/easycap/easycap.h +0 -1
index 8ebf96f..063d447 100644
@@ -65,7 +65,6 @@
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/module.h>
 #include <linux/kref.h>
 #include <linux/usb.h>
drivers/staging/easycap/easycap_ioctl.c +0 -1
index 447953a..c1a0c9c 100644
@@ -25,7 +25,6 @@
 */
 /*****************************************************************************/
 
-#include <linux/smp_lock.h>
 #include "easycap.h"
 #include "easycap_debug.h"
 #include "easycap_standard.h"
drivers/target/target_core_device.c +0 -1
index 317ce58..b0759d5 100644
@@ -33,7 +33,6 @@
 #include <linux/timer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/kthread.h>
 #include <linux/in.h>
 #include <net/sock.h>
drivers/target/target_core_fabric_lib.c +0 -1
index 2628564..a3c695a 100644
@@ -28,7 +28,6 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
 
drivers/target/target_core_file.c +0 -1
index 0aaca88..190ca8a 100644
@@ -33,7 +33,6 @@
 #include <linux/blkdev.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_host.h>
 
drivers/target/target_core_hba.c +0 -1
index 4bbe820..73f7d6d 100644
@@ -31,7 +31,6 @@
 #include <linux/timer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/in.h>
 #include <net/sock.h>
 #include <net/tcp.h>
drivers/target/target_core_iblock.c +0 -1
index c6e0d75..39dac3a 100644
@@ -35,7 +35,6 @@
 #include <linux/blkdev.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/bio.h>
 #include <linux/genhd.h>
 #include <linux/file.h>
drivers/target/target_core_pscsi.c +0 -1
index 742d246..d7606b6 100644
@@ -34,7 +34,6 @@
 #include <linux/blk_types.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/genhd.h>
 #include <linux/cdrom.h>
 #include <linux/file.h>
drivers/target/target_core_rd.c +0 -1
index 979aebf..8dc6d74 100644
@@ -34,7 +34,6 @@
 #include <linux/blkdev.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_host.h>
 
drivers/target/target_core_tpg.c +0 -1
index abfa81a..8f0ba2d 100644
@@ -31,7 +31,6 @@
 #include <linux/timer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/in.h>
 #include <net/sock.h>
 #include <net/tcp.h>
drivers/target/target_core_transport.c +0 -1
index 28b6292..1a5c00b 100644
@@ -34,7 +34,6 @@
 #include <linux/slab.h>
 #include <linux/blkdev.h>
 #include <linux/spinlock.h>
-#include <linux/smp_lock.h>
 #include <linux/kthread.h>
 #include <linux/in.h>
 #include <linux/cdrom.h>
drivers/tty/n_hdlc.c +0 -1
index 47d3228..994745a 100644
@@ -97,7 +97,6 @@
 #include <linux/slab.h>
 #include <linux/tty.h>
 #include <linux/errno.h>
-#include <linux/smp_lock.h>
 #include <linux/string.h>	/* used in new tty drivers */
 #include <linux/signal.h>	/* used in new tty drivers */
 #include <linux/if.h>
drivers/tty/n_r3964.c +0 -1
index 88dda0c..5c6c314 100644
@@ -57,7 +57,6 @@
 #include <linux/ioport.h>
 #include <linux/in.h>
 #include <linux/slab.h>
-#include <linux/smp_lock.h>
 #include <linux/tty.h>
 #include <linux/errno.h>
 #include <linux/string.h>	/* used in new tty drivers */
drivers/tty/pty.c +0 -1
index 923a485..2310cb7 100644
@@ -23,7 +23,6 @@
 #include <linux/major.h>
 #include <linux/mm.h>
 #include <linux/init.h>
-#include <linux/smp_lock.h>
 #include <linux/sysctl.h>
 #include <linux/device.h>
 #include <linux/uaccess.h>
drivers/tty/tty_io.c +0 -1
index 464d09d..f0422d5 100644
@@ -90,7 +90,6 @@
 #include <linux/proc_fs.h>
 #include <linux/init.h>
 #include <linux/module.h>
-#include <linux/smp_lock.h>
 #include <linux/device.h>
 #include <linux/wait.h>
 #include <linux/bitops.h>
drivers/tty/tty_ldisc.c +0 -2
index 4214d58..70808ad 100644
@@ -34,8 +34,6 @@
 #include <linux/vt_kern.h>
 #include <linux/selection.h>
 
-#include <linux/smp_lock.h>	/* For the moment */
-
 #include <linux/kmod.h>
 #include <linux/nsproxy.h>
 
drivers/tty/vt/selection.c +0 -1
index ebae344..e04876b 100644
@@ -26,7 +26,6 @@
 #include <linux/selection.h>
 #include <linux/tiocl.h>
 #include <linux/console.h>
-#include <linux/smp_lock.h>
 
 /* Don't take this from <ctype.h>: 011-015 on the screen aren't spaces */
 #define isspace(c)	((c) == ' ')
drivers/tty/vt/vc_screen.c +0 -1
index eab3a1f..09dc635 100644
@@ -34,7 +34,6 @@
 #include <linux/kbd_kern.h>
 #include <linux/console.h>
 #include <linux/device.h>
-#include <linux/smp_lock.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
 #include <linux/poll.h>
drivers/tty/vt/vt.c +0 -1
index 76407ec..82517ad 100644
@@ -89,7 +89,6 @@
 #include <linux/mutex.h>
 #include <linux/vt_kern.h>
 #include <linux/selection.h>
-#include <linux/smp_lock.h>
 #include <linux/tiocl.h>
 #include <linux/kbd_kern.h>
 #include <linux/consolemap.h>
drivers/tty/vt/vt_ioctl.c +0 -1
index 6b68a0f..1d5f5e4 100644
@@ -27,7 +27,6 @@
 #include <linux/console.h>
 #include <linux/consolemap.h>
 #include <linux/signal.h>
-#include <linux/smp_lock.h>
 #include <linux/timex.h>
 
 #include <asm/io.h>
Signature · 1 line
1.7.1
#50 lore raw

PATCH 20/20 BKL: That's all, folks

In reply to Arnd Bergmann

This removes the implementation of the big kernel lock, at last. A lot of people have worked on this in the past, I so the credit for this patch should be with everyone who participated in the hunt.

The names on the Cc list are the people that were the most active in this, according to the recorded git history, in alphabetical order.

Cc Alan Cox <[email protected]>
Cc Alessio Igor Bogani <[email protected]>
Cc Al Viro <[email protected]>
Cc Andrew Hendry <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Jan Blunck <[email protected]>
Cc John Kacur <[email protected]>
Cc Jonathan Corbet <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Matthew Wilcox <[email protected]>
Cc Oliver Neukum <[email protected]>
Cc Paul Menage <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Trond Myklebust <[email protected]>
---
 include/linux/hardirq.h  |    9 +---
 include/linux/smp_lock.h |   65 ----------------------
 init/Kconfig             |    5 --
 kernel/sched.c           |    7 ---
 lib/Makefile             |    1 -
 lib/kernel_lock.c        |  136 ----------------------------------------------
 6 files changed, 1 insertions(+), 222 deletions(-)
 delete mode 100644 include/linux/smp_lock.h
 delete mode 100644 lib/kernel_lock.c
include/linux/hardirq.h +1 -8
index 32f9fd6..ba36217 100644
@@ -93,13 +93,6 @@
  */
 #define in_nmi()	(preempt_count() & NMI_MASK)
 
-#if defined(CONFIG_PREEMPT) && defined(CONFIG_BKL)
-# include <linux/sched.h>
-# define PREEMPT_INATOMIC_BASE (current->lock_depth >= 0)
-#else
-# define PREEMPT_INATOMIC_BASE 0
-#endif
-
 #if defined(CONFIG_PREEMPT)
 # define PREEMPT_CHECK_OFFSET 1
 #else
@@ -113,7 +106,7 @@
  * used in the general case to determine whether sleeping is possible.
  * Do not use in_atomic() in driver code.
  */
-#define in_atomic()	((preempt_count() & ~PREEMPT_ACTIVE) != PREEMPT_INATOMIC_BASE)
+#define in_atomic()	((preempt_count() & ~PREEMPT_ACTIVE) != 0)
 
 /*
  * Check whether we were atomic before we did preempt_disable():
include/linux/smp_lock.h deleted +0 -65

65 lines not on this page · raw message

init/Kconfig +0 -5
index be788c0..a88d1c9 100644
@@ -69,11 +69,6 @@ config BROKEN_ON_SMP
 	depends on BROKEN || !SMP
 	default y
 
-config LOCK_KERNEL
-	bool
-	depends on (SMP || PREEMPT) && BKL
-	default y
-
 config INIT_ENV_ARG_LIMIT
 	int
 	default 32 if !UML
kernel/sched.c +0 -7
index 18d38e4..8e33e07 100644
@@ -32,7 +32,6 @@
 #include <linux/init.h>
 #include <linux/uaccess.h>
 #include <linux/highmem.h>
-#include <linux/smp_lock.h>
 #include <asm/mmu_context.h>
 #include <linux/interrupt.h>
 #include <linux/capability.h>
@@ -3945,9 +3944,6 @@ need_resched:
 	rcu_note_context_switch(cpu);
 	prev = rq->curr;
 
-	release_kernel_lock(prev);
-need_resched_nonpreemptible:
-
 	schedule_debug(prev);
 
 	if (sched_feat(HRTICK))
@@ -4010,9 +4006,6 @@ need_resched_nonpreemptible:
 
 	post_schedule(rq);
 
-	if (unlikely(reacquire_kernel_lock(prev)))
-		goto need_resched_nonpreemptible;
-
 	preempt_enable_no_resched();
 	if (need_resched())
 		goto need_resched;
lib/Makefile +0 -1
index cbb774f..de6c609 100644
@@ -43,7 +43,6 @@ obj-$(CONFIG_GENERIC_FIND_LAST_BIT) += find_last_bit.o
 CFLAGS_hweight.o = $(subst $(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS))
 obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o
 
-obj-$(CONFIG_LOCK_KERNEL) += kernel_lock.o
 obj-$(CONFIG_BTREE) += btree.o
 obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
 obj-$(CONFIG_DEBUG_LIST) += list_debug.o
lib/kernel_lock.c deleted +0 -136

136 lines not on this page · raw message

Signature · 1 line
1.7.1
Acked-by on v1 20/20
#51 lore raw

Re: [PATCH 20/20] BKL: That's all, folks

Quoted 28 lines · Arnd Bergmann

This removes the implementation of the big kernel lock, at last. A lot of people have worked on this in the past, I so the credit for this patch should be with everyone who participated in the hunt.

The names on the Cc list are the people that were the most active in this, according to the recorded git history, in alphabetical order.

Cc Alan Cox <[email protected]>
Cc Alessio Igor Bogani <[email protected]>
Cc Al Viro <[email protected]>
Cc Andrew Hendry <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Jan Blunck <[email protected]>
Cc John Kacur <[email protected]>
Cc Jonathan Corbet <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Matthew Wilcox <[email protected]>
Cc Oliver Neukum <[email protected]>
Cc Paul Menage <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Trond Myklebust <[email protected]>
Quoted 10 lines
---
 include/linux/hardirq.h  |    9 +---
 include/linux/smp_lock.h |   65 ----------------------
 init/Kconfig             |    5 --
 kernel/sched.c           |    7 ---
 lib/Makefile             |    1 -
 lib/kernel_lock.c        |  136 ----------------------------------------------
 6 files changed, 1 insertions(+), 222 deletions(-)
 delete mode 100644 include/linux/smp_lock.h
 delete mode 100644 lib/kernel_lock.c

Yay!

Ingo

#53 lore raw

Re: [PATCH 20/20] BKL: That's all, folks

Quoted 7 lines · Alan Cox
Quoted 3 lines

Yay!

Nice to see it gone - it seemed such a good idea in Linux 1.3

No need to feel bad about it - there was simply no other way to do it: the BKL basically represented all the single-CPU assumptions that were built into the kernel from 0.10 up to 1.3 (and at least as much new BKL depending code going forward as well ...).

Every last such piece of code had to be eliminated. So the BKL represented the status quo - and eliminating the status quo is always hard, as the code that remains became less and less important :-)

Thanks,

Ingo

Acked-by on v1 20/20
#54 lore raw

Re: [PATCH 20/20] BKL: That's all, folks

In reply to Arnd Bergmann
Quoted 20 lines · Arnd Bergmann

This removes the implementation of the big kernel lock, at last. A lot of people have worked on this in the past, I so the credit for this patch should be with everyone who participated in the hunt.

The names on the Cc list are the people that were the most active in this, according to the recorded git history, in alphabetical order.

Cc Alan Cox <[email protected]>
Cc Alessio Igor Bogani <[email protected]>
Cc Al Viro <[email protected]>
Cc Andrew Hendry <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Jan Blunck <[email protected]>
Quoted 8 lines
Cc John Kacur <[email protected]>
Cc Jonathan Corbet <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Matthew Wilcox <[email protected]>
Cc Oliver Neukum <[email protected]>
Cc Paul Menage <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Trond Myklebust <[email protected]>

Thanks a lot to everyone involved !

#55 lore raw
In reply to Arnd Bergmann
Quoted 9 lines · Arnd Bergmann

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.

Quoted 2 lines

* Move to staging now, kill in 2.6.41 (or later):
appletalk, hpfs

Sounds good to me.

Quoted 5 lines

* 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

#56 lore raw
Quoted 5 lines · Greg KH
Quoted 4 lines · Arnd Bergmann

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.

Quoted 8 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 5 lines

* 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

#57 lore raw

Em 26-01-2011 09:31, Arnd Bergmann escreveu:

Quoted 22 lines · Greg KH
Quoted 5 lines · Arnd Bergmann
Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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.

Quoted 3 lines
Quoted 3 lines

* Work around in an ugly way, but keep alive:
* ufs, ipx, i810, cx25721

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

* 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

#58 lore raw
Quoted 1 line · Mauro Carvalho Chehab

I guess you're meaning cx25821, right?

Right, sorry for the typo.

Quoted 3 lines

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

#59 lore raw

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:[email protected]]
Sent: Wednesday, January 26, 2011 5:45 AM
To: Mauro Carvalho Chehab
Cc: Greg KH; [email protected]; Andrew Hendry; Andrew Morton; Arjan van de Ven; Arnaldo Carvalho de Melo; [email protected]; Chris Wilson; Dave Airlie; [email protected]; Evgeniy Dushistov; Frederic Weisbecker; H. Peter Anvin; Ian Kent; Ingo Molnar; Jeff Layton; [email protected]; [email protected]; [email protected]; Mikulas Patocka; [email protected]; Nick Bowler; Palash Bandyopadhyay; Ross Cohen; Russell King; Stuart Swales; Takahiro Hirofuchi; Thomas Gleixner
Subject: Re: [RFC 00/20] Proposal for remaining BKL users

Quoted 1 line · Mauro Carvalho Chehab

I guess you're meaning cx25821, right?

Right, sorry for the typo.

Quoted 3 lines

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."

**********************************************************************

---------------------------------------------------------------------