2010-07-03 21:48:06

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 0/6] block: BKL removal, version 3

This series removes the big kernel lock from the common
block layer code by pushing it into the individual
device drivers.

Following a suggestion from Christoph Hellwig,
It replaces the earlier attempt that converted the
usage of the BKL into a subsystem-wide mutex.

I'd like to get this into 2.6.36 in order to remove
the BKL from all the core kernel code.

The first larger patches should be fairly uncontroversial,
while patch 5/6 is the most critical one. I could not
see any reason why it should not work, but I'm also not
convinced that I found all the possible problems in it,
so I'd really like to get a second opinion on that one.

Arnd Bergmann (6):
block: push down BKL into .locked_ioctl
block: push down BKL into .open and .release
block: push BKL into blktrace ioctls
block: remove BKL from BLKROSET and BLKFLSBUF
block: remove BKL from partition code
scsi/sd: remove big kernel lock

arch/um/drivers/ubd_kern.c | 7 ++++-
block/compat_ioctl.c | 56 -----------------------------------
block/ioctl.c | 21 +------------
drivers/block/DAC960.c | 13 +++++--
drivers/block/amiflop.c | 27 +++++++++++++++--
drivers/block/aoe/aoeblk.c | 4 ++
drivers/block/ataflop.c | 28 ++++++++++++++++-
drivers/block/brd.c | 5 ++-
drivers/block/cciss.c | 31 +++++++++++++++----
drivers/block/cpqarray.c | 40 ++++++++++++++++++++++---
drivers/block/drbd/drbd_main.c | 4 ++
drivers/block/floppy.c | 20 ++++++++++++-
drivers/block/loop.c | 5 +++
drivers/block/nbd.c | 5 ++-
drivers/block/paride/pcd.c | 21 +++++++++++--
drivers/block/paride/pd.c | 9 +++++-
drivers/block/paride/pf.c | 26 ++++++++++++----
drivers/block/pktcdvd.c | 16 ++++++++--
drivers/block/swim.c | 20 +++++++++++-
drivers/block/swim3.c | 30 +++++++++++++++++-
drivers/block/ub.c | 27 +++++++++++++++--
drivers/block/viodasd.c | 19 +++++++++++-
drivers/block/virtio_blk.c | 15 +++++++++-
drivers/block/xd.c | 15 +++++++++-
drivers/block/xd.h | 2 +-
drivers/block/xen-blkfront.c | 9 +++++-
drivers/block/xsysace.c | 6 ++++
drivers/block/z2ram.c | 13 ++++++--
drivers/cdrom/gdrom.c | 19 ++++++++++--
drivers/cdrom/viocd.c | 21 +++++++++++--
drivers/ide/ide-cd.c | 30 +++++++++++++++----
drivers/ide/ide-disk_ioctl.c | 9 ++++-
drivers/ide/ide-floppy_ioctl.c | 12 +++++--
drivers/ide/ide-gd.c | 19 ++++++++++-
drivers/ide/ide-tape.c | 19 ++++++++++--
drivers/md/dm.c | 7 ++++
drivers/md/md.c | 6 ++++
drivers/memstick/core/mspro_block.c | 9 +++++-
drivers/message/i2o/i2o_block.c | 19 +++++++++++-
drivers/mmc/card/block.c | 5 +++
drivers/mtd/mtd_blkdevs.c | 11 +++++-
drivers/s390/block/dasd.c | 6 ++++
drivers/s390/block/dcssblk.c | 5 +++
drivers/s390/char/tape_block.c | 8 ++++-
drivers/scsi/sd.c | 3 +-
drivers/scsi/sr.c | 22 ++++++++++++-
drivers/staging/hv/blkvsc_drv.c | 5 +++
fs/block_dev.c | 10 +-----
include/linux/blkdev.h | 1 -
include/linux/blktrace_api.h | 12 +++++++
kernel/trace/blktrace.c | 43 ++++++++++++++++++++++++++
51 files changed, 628 insertions(+), 167 deletions(-)


2010-07-03 21:47:35

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/6] block: push down BKL into .locked_ioctl

As a preparation for the removal of the big kernel
lock in the block layer, this removes the BKL
from the common ioctl handling code, moving it
into every single driver still using it.

Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Christoph Hellwig <[email protected]>
---
block/ioctl.c | 11 +----------
drivers/block/amiflop.c | 15 ++++++++++++++-
drivers/block/ataflop.c | 14 +++++++++++++-
drivers/block/brd.c | 5 ++++-
drivers/block/cciss.c | 8 +++++---
drivers/block/cpqarray.c | 18 ++++++++++++++++--
drivers/block/floppy.c | 15 ++++++++++++++-
drivers/block/nbd.c | 5 ++++-
drivers/block/paride/pcd.c | 11 +++++++++--
drivers/block/paride/pd.c | 5 ++++-
drivers/block/paride/pf.c | 6 +++++-
drivers/block/pktcdvd.c | 11 ++++++++---
drivers/block/swim.c | 5 ++++-
drivers/block/swim3.c | 15 ++++++++++++++-
drivers/block/ub.c | 10 ++++++++--
drivers/block/virtio_blk.c | 15 ++++++++++++++-
drivers/block/xd.c | 15 ++++++++++++++-
drivers/block/xd.h | 2 +-
drivers/block/xen-blkfront.c | 2 +-
drivers/cdrom/gdrom.c | 11 +++++++++--
drivers/cdrom/viocd.c | 11 +++++++++--
drivers/ide/ide-cd.c | 16 +++++++++++++++-
drivers/ide/ide-disk_ioctl.c | 9 +++++++--
drivers/ide/ide-floppy_ioctl.c | 12 +++++++++---
drivers/ide/ide-gd.c | 2 +-
drivers/ide/ide-tape.c | 10 ++++++++--
drivers/message/i2o/i2o_block.c | 15 ++++++++++++++-
drivers/mtd/mtd_blkdevs.c | 5 ++++-
drivers/scsi/sd.c | 15 ++++++++++++++-
drivers/scsi/sr.c | 15 ++++++++++++++-
include/linux/blkdev.h | 1 -
31 files changed, 257 insertions(+), 53 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..1cfa8d4 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -163,18 +163,10 @@ int __blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
struct gendisk *disk = bdev->bd_disk;
- int ret;

if (disk->fops->ioctl)
return disk->fops->ioctl(bdev, mode, cmd, arg);

- if (disk->fops->locked_ioctl) {
- lock_kernel();
- ret = disk->fops->locked_ioctl(bdev, mode, cmd, arg);
- unlock_kernel();
- return ret;
- }
-
return -ENOTTY;
}
/*
@@ -185,8 +177,7 @@ int __blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
EXPORT_SYMBOL_GPL(__blkdev_driver_ioctl);

/*
- * always keep this in sync with compat_blkdev_ioctl() and
- * compat_blkdev_locked_ioctl()
+ * always keep this in sync with compat_blkdev_ioctl()
*/
int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
unsigned long arg)
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 832798a..7a51535 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -60,6 +60,7 @@
#include <linux/hdreg.h>
#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/smp_lock.h>
#include <linux/amifdreg.h>
#include <linux/amifd.h>
#include <linux/buffer_head.h>
@@ -1500,6 +1501,18 @@ static int fd_ioctl(struct block_device *bdev, fmode_t mode,
return 0;
}

+static int fd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = fd_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
static void fd_probe(int dev)
{
unsigned long code;
@@ -1638,7 +1651,7 @@ static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
.open = floppy_open,
.release = floppy_release,
- .locked_ioctl = fd_ioctl,
+ .ioctl = fd_unlocked_ioctl,
.getgeo = fd_getgeo,
.media_changed = amiga_floppy_change,
};
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index e35cf59..09ae46f 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -67,6 +67,7 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>

#include <asm/atafd.h>
#include <asm/atafdreg.h>
@@ -1665,6 +1666,17 @@ static int fd_ioctl(struct block_device *bdev, fmode_t mode,
}
}

+static int fd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ lock_kernel();
+ ret = fd_ioctl(bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
+}

/* Initialize the 'unit' variable for drive 'drive' */

@@ -1855,7 +1867,7 @@ static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
.open = floppy_open,
.release = floppy_release,
- .locked_ioctl = fd_ioctl,
+ .ioctl = fd_unlocked_ioctl,
.media_changed = check_floppy_change,
.revalidate_disk= floppy_revalidate,
};
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index f1bf79d..72ff611 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -15,6 +15,7 @@
#include <linux/blkdev.h>
#include <linux/bio.h>
#include <linux/highmem.h>
+#include <linux/smp_lock.h>
#include <linux/radix-tree.h>
#include <linux/buffer_head.h> /* invalidate_bh_lrus() */
#include <linux/slab.h>
@@ -401,6 +402,7 @@ static int brd_ioctl(struct block_device *bdev, fmode_t mode,
* ram device BLKFLSBUF has special semantics, we want to actually
* release and destroy the ramdisk data.
*/
+ lock_kernel();
mutex_lock(&bdev->bd_mutex);
error = -EBUSY;
if (bdev->bd_openers <= 1) {
@@ -417,13 +419,14 @@ static int brd_ioctl(struct block_device *bdev, fmode_t mode,
error = 0;
}
mutex_unlock(&bdev->bd_mutex);
+ unlock_kernel();

return error;
}

static const struct block_device_operations brd_fops = {
.owner = THIS_MODULE,
- .locked_ioctl = brd_ioctl,
+ .ioctl = brd_ioctl,
#ifdef CONFIG_BLK_DEV_XIP
.direct_access = brd_direct_access,
#endif
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 51ceaee..6c3c8c4 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -170,6 +170,8 @@ static void do_cciss_request(struct request_queue *q);
static irqreturn_t do_cciss_intr(int irq, void *dev_id);
static int cciss_open(struct block_device *bdev, fmode_t mode);
static int cciss_release(struct gendisk *disk, fmode_t mode);
+static int do_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg);
static int cciss_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg);
static int cciss_getgeo(struct block_device *bdev, struct hd_geometry *geo);
@@ -223,7 +225,7 @@ static const struct block_device_operations cciss_fops = {
.owner = THIS_MODULE,
.open = cciss_open,
.release = cciss_release,
- .locked_ioctl = cciss_ioctl,
+ .ioctl = do_ioctl,
.getgeo = cciss_getgeo,
#ifdef CONFIG_COMPAT
.compat_ioctl = cciss_compat_ioctl,
@@ -1021,8 +1023,6 @@ static int cciss_release(struct gendisk *disk, fmode_t mode)
return 0;
}

-#ifdef CONFIG_COMPAT
-
static int do_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
@@ -1033,6 +1033,8 @@ static int do_ioctl(struct block_device *bdev, fmode_t mode,
return ret;
}

+#ifdef CONFIG_COMPAT
+
static int cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg);
static int cciss_ioctl32_big_passthru(struct block_device *bdev, fmode_t mode,
diff --git a/drivers/block/cpqarray.c b/drivers/block/cpqarray.c
index abb4ec6..b591518 100644
--- a/drivers/block/cpqarray.c
+++ b/drivers/block/cpqarray.c
@@ -35,6 +35,7 @@
#include <linux/seq_file.h>
#include <linux/init.h>
#include <linux/hdreg.h>
+#include <linux/smp_lock.h>
#include <linux/spinlock.h>
#include <linux/blkdev.h>
#include <linux/genhd.h>
@@ -159,7 +160,7 @@ static int sendcmd(

static int ida_open(struct block_device *bdev, fmode_t mode);
static int ida_release(struct gendisk *disk, fmode_t mode);
-static int ida_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg);
+static int ida_unlocked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg);
static int ida_getgeo(struct block_device *bdev, struct hd_geometry *geo);
static int ida_ctlr_ioctl(ctlr_info_t *h, int dsk, ida_ioctl_t *io);

@@ -197,7 +198,7 @@ static const struct block_device_operations ida_fops = {
.owner = THIS_MODULE,
.open = ida_open,
.release = ida_release,
- .locked_ioctl = ida_ioctl,
+ .ioctl = ida_unlocked_ioctl,
.getgeo = ida_getgeo,
.revalidate_disk= ida_revalidate,
};
@@ -1192,6 +1193,19 @@ out_passthru:
}

}
+
+static int ida_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = ida_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
/*
* ida_ctlr_ioctl is for passing commands to the controller from userspace.
* The command block (io) has already been copied to kernel space for us,
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 90c4038..c61df47 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -178,6 +178,7 @@ static int print_unex = 1;
#include <linux/slab.h>
#include <linux/mm.h>
#include <linux/bio.h>
+#include <linux/smp_lock.h>
#include <linux/string.h>
#include <linux/jiffies.h>
#include <linux/fcntl.h>
@@ -3593,6 +3594,18 @@ static int fd_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd,
return 0;
}

+static int fd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = fd_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
static void __init config_types(void)
{
bool has_drive = false;
@@ -3893,7 +3906,7 @@ static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
.open = floppy_open,
.release = floppy_release,
- .locked_ioctl = fd_ioctl,
+ .ioctl = fd_unlocked_ioctl,
.getgeo = fd_getgeo,
.media_changed = check_floppy_change,
.revalidate_disk = floppy_revalidate,
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 218d091..9789907 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -24,6 +24,7 @@
#include <linux/errno.h>
#include <linux/file.h>
#include <linux/ioctl.h>
+#include <linux/smp_lock.h>
#include <linux/compiler.h>
#include <linux/err.h>
#include <linux/kernel.h>
@@ -716,9 +717,11 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
dprintk(DBG_IOCTL, "%s: nbd_ioctl cmd=%s(0x%x) arg=%lu\n",
lo->disk->disk_name, ioctl_cmd_to_ascii(cmd), cmd, arg);

+ lock_kernel();
mutex_lock(&lo->tx_lock);
error = __nbd_ioctl(bdev, lo, cmd, arg);
mutex_unlock(&lo->tx_lock);
+ unlock_kernel();

return error;
}
@@ -726,7 +729,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
static const struct block_device_operations nbd_fops =
{
.owner = THIS_MODULE,
- .locked_ioctl = nbd_ioctl,
+ .ioctl = nbd_ioctl,
};

/*
diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index 71acf4e..daba7a6 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -138,6 +138,7 @@ enum {D_PRT, D_PRO, D_UNI, D_MOD, D_SLV, D_DLY};
#include <linux/cdrom.h>
#include <linux/spinlock.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <asm/uaccess.h>

static DEFINE_SPINLOCK(pcd_lock);
@@ -238,7 +239,13 @@ static int pcd_block_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
struct pcd_unit *cd = bdev->bd_disk->private_data;
- return cdrom_ioctl(&cd->info, bdev, mode, cmd, arg);
+ int ret;
+
+ lock_kernel();
+ ret = cdrom_ioctl(&cd->info, bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
}

static int pcd_block_media_changed(struct gendisk *disk)
@@ -251,7 +258,7 @@ static const struct block_device_operations pcd_bdops = {
.owner = THIS_MODULE,
.open = pcd_block_open,
.release = pcd_block_release,
- .locked_ioctl = pcd_block_ioctl,
+ .ioctl = pcd_block_ioctl,
.media_changed = pcd_block_media_changed,
};

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index c1e5cd0..b22943a 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -153,6 +153,7 @@ enum {D_PRT, D_PRO, D_UNI, D_MOD, D_GEO, D_SBY, D_DLY, D_SLV};
#include <linux/blkdev.h>
#include <linux/blkpg.h>
#include <linux/kernel.h>
+#include <linux/smp_lock.h>
#include <asm/uaccess.h>
#include <linux/workqueue.h>

@@ -768,8 +769,10 @@ static int pd_ioctl(struct block_device *bdev, fmode_t mode,

switch (cmd) {
case CDROMEJECT:
+ lock_kernel();
if (disk->access == 1)
pd_special_command(disk, pd_eject);
+ unlock_kernel();
return 0;
default:
return -EINVAL;
@@ -812,7 +815,7 @@ static const struct block_device_operations pd_fops = {
.owner = THIS_MODULE,
.open = pd_open,
.release = pd_release,
- .locked_ioctl = pd_ioctl,
+ .ioctl = pd_ioctl,
.getgeo = pd_getgeo,
.media_changed = pd_check_media,
.revalidate_disk= pd_revalidate
diff --git a/drivers/block/paride/pf.c b/drivers/block/paride/pf.c
index c059aab..38b4d56 100644
--- a/drivers/block/paride/pf.c
+++ b/drivers/block/paride/pf.c
@@ -152,6 +152,7 @@ enum {D_PRT, D_PRO, D_UNI, D_MOD, D_SLV, D_LUN, D_DLY};
#include <linux/spinlock.h>
#include <linux/blkdev.h>
#include <linux/blkpg.h>
+#include <linux/smp_lock.h>
#include <asm/uaccess.h>

static DEFINE_SPINLOCK(pf_spin_lock);
@@ -266,7 +267,7 @@ static const struct block_device_operations pf_fops = {
.owner = THIS_MODULE,
.open = pf_open,
.release = pf_release,
- .locked_ioctl = pf_ioctl,
+ .ioctl = pf_ioctl,
.getgeo = pf_getgeo,
.media_changed = pf_check_media,
};
@@ -342,7 +343,10 @@ static int pf_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, u

if (pf->access != 1)
return -EBUSY;
+ lock_kernel();
pf_eject(pf);
+ unlock_kernel();
+
return 0;
}

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 8a549db..1f70aec 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -57,6 +57,7 @@
#include <linux/seq_file.h>
#include <linux/miscdevice.h>
#include <linux/freezer.h>
+#include <linux/smp_lock.h>
#include <linux/mutex.h>
#include <linux/slab.h>
#include <scsi/scsi_cmnd.h>
@@ -2762,10 +2763,12 @@ out_mem:
static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg)
{
struct pktcdvd_device *pd = bdev->bd_disk->private_data;
+ int ret;

VPRINTK("pkt_ioctl: cmd %x, dev %d:%d\n", cmd,
MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));

+ lock_kernel();
switch (cmd) {
case CDROMEJECT:
/*
@@ -2783,12 +2786,14 @@ static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd,
case CDROM_LAST_WRITTEN:
case CDROM_SEND_PACKET:
case SCSI_IOCTL_SEND_COMMAND:
- return __blkdev_driver_ioctl(pd->bdev, mode, cmd, arg);
+ ret = __blkdev_driver_ioctl(pd->bdev, mode, cmd, arg);
+ break;

default:
VPRINTK(DRIVER_NAME": Unknown ioctl for %s (%x)\n", pd->name, cmd);
- return -ENOTTY;
+ ret = -ENOTTY;
}
+ unlock_kernel();

return 0;
}
@@ -2812,7 +2817,7 @@ static const struct block_device_operations pktcdvd_ops = {
.owner = THIS_MODULE,
.open = pkt_open,
.release = pkt_close,
- .locked_ioctl = pkt_ioctl,
+ .ioctl = pkt_ioctl,
.media_changed = pkt_media_changed,
};

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index e463657..f04f74e 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -20,6 +20,7 @@
#include <linux/fd.h>
#include <linux/slab.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/hdreg.h>
#include <linux/kernel.h>
#include <linux/delay.h>
@@ -690,7 +691,9 @@ static int floppy_ioctl(struct block_device *bdev, fmode_t mode,
case FDEJECT:
if (fs->ref_count != 1)
return -EBUSY;
+ lock_kernel();
err = floppy_eject(fs);
+ unlock_kernel();
return err;

case FDGETPRM:
@@ -753,7 +756,7 @@ static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
.open = floppy_open,
.release = floppy_release,
- .locked_ioctl = floppy_ioctl,
+ .ioctl = floppy_ioctl,
.getgeo = floppy_getgeo,
.media_changed = floppy_check_change,
.revalidate_disk = floppy_revalidate,
diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index ed6fb91..9d6bf06 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -25,6 +25,7 @@
#include <linux/ioctl.h>
#include <linux/blkdev.h>
#include <linux/interrupt.h>
+#include <linux/smp_lock.h>
#include <linux/module.h>
#include <linux/spinlock.h>
#include <asm/io.h>
@@ -867,6 +868,18 @@ static int floppy_ioctl(struct block_device *bdev, fmode_t mode,
return -ENOTTY;
}

+static int floppy_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = floppy_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
static int floppy_open(struct block_device *bdev, fmode_t mode)
{
struct floppy_state *fs = bdev->bd_disk->private_data;
@@ -997,7 +1010,7 @@ static int floppy_revalidate(struct gendisk *disk)
static const struct block_device_operations floppy_fops = {
.open = floppy_open,
.release = floppy_release,
- .locked_ioctl = floppy_ioctl,
+ .ioctl = floppy_unlocked_ioctl,
.media_changed = floppy_check_change,
.revalidate_disk= floppy_revalidate,
};
diff --git a/drivers/block/ub.c b/drivers/block/ub.c
index 0536b5b..d6e14d2 100644
--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -28,6 +28,7 @@
#include <linux/timer.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
+#include <linux/smp_lock.h>
#include <scsi/scsi.h>

#define DRV_NAME "ub"
@@ -1729,8 +1730,13 @@ static int ub_bd_ioctl(struct block_device *bdev, fmode_t mode,
{
struct gendisk *disk = bdev->bd_disk;
void __user *usermem = (void __user *) arg;
+ int ret;
+
+ lock_kernel();
+ ret = scsi_cmd_ioctl(disk->queue, disk, mode, cmd, usermem);
+ unlock_kernel();

- return scsi_cmd_ioctl(disk->queue, disk, mode, cmd, usermem);
+ return ret;
}

/*
@@ -1794,7 +1800,7 @@ static const struct block_device_operations ub_bd_fops = {
.owner = THIS_MODULE,
.open = ub_bd_open,
.release = ub_bd_release,
- .locked_ioctl = ub_bd_ioctl,
+ .ioctl = ub_bd_ioctl,
.media_changed = ub_bd_media_changed,
.revalidate_disk = ub_bd_revalidate,
};
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 258bc2a..fb970eb 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -2,6 +2,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/hdreg.h>
#include <linux/virtio.h>
#include <linux/virtio_blk.h>
@@ -245,6 +246,18 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
(void __user *)data);
}

+static int virtblk_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = virtblk_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
/* We provide getgeo only to please some old bootloader/partitioning tools */
static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
{
@@ -271,7 +284,7 @@ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
}

static const struct block_device_operations virtblk_fops = {
- .locked_ioctl = virtblk_ioctl,
+ .ioctl = virtblk_unlocked_ioctl,
.owner = THIS_MODULE,
.getgeo = virtblk_getgeo,
};
diff --git a/drivers/block/xd.c b/drivers/block/xd.c
index 18a80ff..7f00543 100644
--- a/drivers/block/xd.c
+++ b/drivers/block/xd.c
@@ -46,6 +46,7 @@
#include <linux/init.h>
#include <linux/wait.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/blkpg.h>
#include <linux/delay.h>
#include <linux/io.h>
@@ -133,7 +134,7 @@ static int xd_getgeo(struct block_device *bdev, struct hd_geometry *geo);

static const struct block_device_operations xd_fops = {
.owner = THIS_MODULE,
- .locked_ioctl = xd_ioctl,
+ .ioctl = xd_unlocked_ioctl,
.getgeo = xd_getgeo,
};
static DECLARE_WAIT_QUEUE_HEAD(xd_wait_int);
@@ -375,6 +376,18 @@ static int xd_ioctl(struct block_device *bdev, fmode_t mode, u_int cmd, u_long a
}
}

+static int xd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long param)
+{
+ int ret;
+
+ lock_kernel();
+ ret = xd_ioctl(bdev, mode, cmd, param);
+ unlock_kernel();
+
+ return ret;
+}
+
/* xd_readwrite: handle a read/write request */
static int xd_readwrite (u_char operation,XD_INFO *p,char *buffer,u_int block,u_int count)
{
diff --git a/drivers/block/xd.h b/drivers/block/xd.h
index 37cacef..bded505 100644
--- a/drivers/block/xd.h
+++ b/drivers/block/xd.h
@@ -105,7 +105,7 @@ static u_char xd_detect (u_char *controller, unsigned int *address);
static u_char xd_initdrives (void (*init_drive)(u_char drive));

static void do_xd_request (struct request_queue * q);
-static int xd_ioctl (struct block_device *bdev,fmode_t mode,unsigned int cmd,unsigned long arg);
+static int xd_unlocked_ioctl (struct block_device *bdev,fmode_t mode,unsigned int cmd,unsigned long arg);
static int xd_readwrite (u_char operation,XD_INFO *disk,char *buffer,u_int block,u_int count);
static void xd_recalibrate (u_char drive);

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 82ed403..96a70bb 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1046,7 +1046,7 @@ static const struct block_device_operations xlvbd_block_fops =
.open = blkif_open,
.release = blkif_release,
.getgeo = blkif_getgeo,
- .locked_ioctl = blkif_ioctl,
+ .ioctl = blkif_ioctl,
};


diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 03c71f7..7d7b894 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -32,6 +32,7 @@
#include <linux/blkdev.h>
#include <linux/interrupt.h>
#include <linux/device.h>
+#include <linux/smp_lock.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/platform_device.h>
@@ -509,7 +510,13 @@ static int gdrom_bdops_mediachanged(struct gendisk *disk)
static int gdrom_bdops_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
- return cdrom_ioctl(gd.cd_info, bdev, mode, cmd, arg);
+ int ret;
+
+ lock_kernel();
+ ret = cdrom_ioctl(gd.cd_info, bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
}

static const struct block_device_operations gdrom_bdops = {
@@ -517,7 +524,7 @@ static const struct block_device_operations gdrom_bdops = {
.open = gdrom_bdops_open,
.release = gdrom_bdops_release,
.media_changed = gdrom_bdops_mediachanged,
- .locked_ioctl = gdrom_bdops_ioctl,
+ .ioctl = gdrom_bdops_ioctl,
};

static irqreturn_t gdrom_command_interrupt(int irq, void *dev_id)
diff --git a/drivers/cdrom/viocd.c b/drivers/cdrom/viocd.c
index 451cd70..203c87c 100644
--- a/drivers/cdrom/viocd.c
+++ b/drivers/cdrom/viocd.c
@@ -40,6 +40,7 @@
#include <linux/module.h>
#include <linux/completion.h>
#include <linux/proc_fs.h>
+#include <linux/smp_lock.h>
#include <linux/seq_file.h>
#include <linux/scatterlist.h>

@@ -168,7 +169,13 @@ static int viocd_blk_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
struct disk_info *di = bdev->bd_disk->private_data;
- return cdrom_ioctl(&di->viocd_info, bdev, mode, cmd, arg);
+ int ret;
+
+ lock_kernel();
+ ret = cdrom_ioctl(&di->viocd_info, bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
}

static int viocd_blk_media_changed(struct gendisk *disk)
@@ -181,7 +188,7 @@ static const struct block_device_operations viocd_fops = {
.owner = THIS_MODULE,
.open = viocd_blk_open,
.release = viocd_blk_release,
- .locked_ioctl = viocd_blk_ioctl,
+ .ioctl = viocd_blk_ioctl,
.media_changed = viocd_blk_media_changed,
};

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 64207df..03c93ae 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -31,6 +31,7 @@
#include <linux/delay.h>
#include <linux/timer.h>
#include <linux/seq_file.h>
+#include <linux/smp_lock.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/errno.h>
@@ -1670,6 +1671,19 @@ static int idecd_ioctl(struct block_device *bdev, fmode_t mode,
return err;
}

+static int idecd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ lock_kernel();
+ ret = idecd_ioctl(bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
+}
+
+
static int idecd_media_changed(struct gendisk *disk)
{
struct cdrom_info *info = ide_drv_g(disk, cdrom_info);
@@ -1690,7 +1704,7 @@ static const struct block_device_operations idecd_ops = {
.owner = THIS_MODULE,
.open = idecd_open,
.release = idecd_release,
- .locked_ioctl = idecd_ioctl,
+ .ioctl = idecd_unlocked_ioctl,
.media_changed = idecd_media_changed,
.revalidate_disk = idecd_revalidate_disk
};
diff --git a/drivers/ide/ide-disk_ioctl.c b/drivers/ide/ide-disk_ioctl.c
index 7b783dd..ec94c66 100644
--- a/drivers/ide/ide-disk_ioctl.c
+++ b/drivers/ide/ide-disk_ioctl.c
@@ -1,6 +1,7 @@
#include <linux/kernel.h>
#include <linux/ide.h>
#include <linux/hdreg.h>
+#include <linux/smp_lock.h>

#include "ide-disk.h"

@@ -18,9 +19,13 @@ int ide_disk_ioctl(ide_drive_t *drive, struct block_device *bdev, fmode_t mode,
{
int err;

+ lock_kernel();
err = ide_setting_ioctl(drive, bdev, cmd, arg, ide_disk_ioctl_settings);
if (err != -EOPNOTSUPP)
- return err;
+ goto out;

- return generic_ide_ioctl(drive, bdev, cmd, arg);
+ err = generic_ide_ioctl(drive, bdev, cmd, arg);
+out:
+ unlock_kernel();
+ return err;
}
diff --git a/drivers/ide/ide-floppy_ioctl.c b/drivers/ide/ide-floppy_ioctl.c
index 9c22882..fd3d05a 100644
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/ide.h>
#include <linux/cdrom.h>
+#include <linux/smp_lock.h>

#include <asm/unaligned.h>

@@ -275,12 +276,15 @@ int ide_floppy_ioctl(ide_drive_t *drive, struct block_device *bdev,
void __user *argp = (void __user *)arg;
int err;

- if (cmd == CDROMEJECT || cmd == CDROM_LOCKDOOR)
- return ide_floppy_lockdoor(drive, &pc, arg, cmd);
+ lock_kernel();
+ if (cmd == CDROMEJECT || cmd == CDROM_LOCKDOOR) {
+ err = ide_floppy_lockdoor(drive, &pc, arg, cmd);
+ goto out;
+ }

err = ide_floppy_format_ioctl(drive, &pc, mode, cmd, argp);
if (err != -ENOTTY)
- return err;
+ goto out;

/*
* skip SCSI_IOCTL_SEND_COMMAND (deprecated)
@@ -293,5 +297,7 @@ int ide_floppy_ioctl(ide_drive_t *drive, struct block_device *bdev,
if (err == -ENOTTY)
err = generic_ide_ioctl(drive, bdev, cmd, arg);

+out:
+ unlock_kernel();
return err;
}
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index c102d23..883f0c9 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -323,7 +323,7 @@ static const struct block_device_operations ide_gd_ops = {
.owner = THIS_MODULE,
.open = ide_gd_open,
.release = ide_gd_release,
- .locked_ioctl = ide_gd_ioctl,
+ .ioctl = ide_gd_ioctl,
.getgeo = ide_gd_getgeo,
.media_changed = ide_gd_media_changed,
.unlock_native_capacity = ide_gd_unlock_native_capacity,
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b072328..2487916 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -32,6 +32,7 @@
#include <linux/errno.h>
#include <linux/genhd.h>
#include <linux/seq_file.h>
+#include <linux/smp_lock.h>
#include <linux/slab.h>
#include <linux/pci.h>
#include <linux/ide.h>
@@ -1926,9 +1927,14 @@ static int idetape_ioctl(struct block_device *bdev, fmode_t mode,
{
struct ide_tape_obj *tape = ide_drv_g(bdev->bd_disk, ide_tape_obj);
ide_drive_t *drive = tape->drive;
- int err = generic_ide_ioctl(drive, bdev, cmd, arg);
+ int err;
+
+ lock_kernel();
+ err = generic_ide_ioctl(drive, bdev, cmd, arg);
if (err == -EINVAL)
err = idetape_blkdev_ioctl(drive, cmd, arg);
+ unlock_kernel();
+
return err;
}

@@ -1936,7 +1942,7 @@ static const struct block_device_operations idetape_block_ops = {
.owner = THIS_MODULE,
.open = idetape_open,
.release = idetape_release,
- .locked_ioctl = idetape_ioctl,
+ .ioctl = idetape_ioctl,
};

static int ide_tape_probe(ide_drive_t *drive)
diff --git a/drivers/message/i2o/i2o_block.c b/drivers/message/i2o/i2o_block.c
index fc593fb..1f3b080 100644
--- a/drivers/message/i2o/i2o_block.c
+++ b/drivers/message/i2o/i2o_block.c
@@ -53,6 +53,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/i2o.h>
+#include <linux/smp_lock.h>

#include <linux/mempool.h>

@@ -678,6 +679,18 @@ static int i2o_block_ioctl(struct block_device *bdev, fmode_t mode,
return -ENOTTY;
};

+static int i2o_block_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ lock_kernel();
+ ret = i2o_block_ioctl(bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
+}
+
/**
* i2o_block_media_changed - Have we seen a media change?
* @disk: gendisk which should be verified
@@ -930,7 +943,7 @@ static const struct block_device_operations i2o_block_fops = {
.owner = THIS_MODULE,
.open = i2o_block_open,
.release = i2o_block_release,
- .locked_ioctl = i2o_block_ioctl,
+ .ioctl = i2o_block_unlocked_ioctl,
.getgeo = i2o_block_getgeo,
.media_changed = i2o_block_media_changed
};
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 03e19c1..3ddc95b 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -15,6 +15,7 @@
#include <linux/blkdev.h>
#include <linux/blkpg.h>
#include <linux/spinlock.h>
+#include <linux/smp_lock.h>
#include <linux/hdreg.h>
#include <linux/init.h>
#include <linux/mutex.h>
@@ -237,6 +238,7 @@ static int blktrans_ioctl(struct block_device *bdev, fmode_t mode,
if (!dev)
return ret;

+ lock_kernel();
mutex_lock(&dev->lock);

if (!dev->mtd)
@@ -250,6 +252,7 @@ static int blktrans_ioctl(struct block_device *bdev, fmode_t mode,
}
unlock:
mutex_unlock(&dev->lock);
+ unlock_kernel();
blktrans_dev_put(dev);
return ret;
}
@@ -258,7 +261,7 @@ static const struct block_device_operations mtd_blktrans_ops = {
.owner = THIS_MODULE,
.open = blktrans_open,
.release = blktrans_release,
- .locked_ioctl = blktrans_ioctl,
+ .ioctl = blktrans_ioctl,
.getgeo = blktrans_getgeo,
};

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8802e48..ac1284a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -46,6 +46,7 @@
#include <linux/blkdev.h>
#include <linux/blkpg.h>
#include <linux/delay.h>
+#include <linux/smp_lock.h>
#include <linux/mutex.h>
#include <linux/string_helpers.h>
#include <linux/async.h>
@@ -915,6 +916,18 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
return scsi_ioctl(sdp, cmd, p);
}

+static int sd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ lock_kernel();
+ ret = sd_ioctl(bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
+}
+
static void set_media_not_present(struct scsi_disk *sdkp)
{
sdkp->media_present = 0;
@@ -1095,7 +1108,7 @@ static const struct block_device_operations sd_fops = {
.owner = THIS_MODULE,
.open = sd_open,
.release = sd_release,
- .locked_ioctl = sd_ioctl,
+ .ioctl = sd_unlocked_ioctl,
.getgeo = sd_getgeo,
#ifdef CONFIG_COMPAT
.compat_ioctl = sd_compat_ioctl,
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 0a90abc..ad62aa8 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -44,6 +44,7 @@
#include <linux/init.h>
#include <linux/blkdev.h>
#include <linux/mutex.h>
+#include <linux/smp_lock.h>
#include <linux/slab.h>
#include <asm/uaccess.h>

@@ -520,6 +521,18 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
return scsi_ioctl(sdev, cmd, argp);
}

+static int sr_block_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
+ unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ lock_kernel();
+ ret = sr_block_ioctl(bdev, mode, cmd, arg);
+ unlock_kernel();
+
+ return ret;
+}
+
static int sr_block_media_changed(struct gendisk *disk)
{
struct scsi_cd *cd = scsi_cd(disk);
@@ -531,7 +544,7 @@ static const struct block_device_operations sr_bdops =
.owner = THIS_MODULE,
.open = sr_block_open,
.release = sr_block_release,
- .locked_ioctl = sr_block_ioctl,
+ .ioctl = sr_block_unlocked_ioctl,
.media_changed = sr_block_media_changed,
/*
* No compat_ioctl for now because sr_block_ioctl never
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 09a8402..547854c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1333,7 +1333,6 @@ static inline int blk_integrity_rq(struct request *rq)
struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
int (*release) (struct gendisk *, fmode_t);
- int (*locked_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*direct_access) (struct block_device *, sector_t,
--
1.7.1

2010-07-03 21:48:08

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 5/6] block: remove BKL from partition code

I don't see any reason why we need the BKL here.
The functions blkdev_get, blkdev_put, blkpg_ioctl
and blkdev_reread_part are the only remaining users
of the big kernel lock in the block layer, and they
all access the same fields of the bdev and gendisk
structures, yet they always do so under the protection
of bdev->bd_mutex.

The open and close block_device_operations have all
been converted to grab the BKL themselves, where
necessary, so as far I can tell it should be safe
to remove.

If it is not, please explain why we still need it.

Signed-off-by: Arnd Bergmann <[email protected]>
---
block/ioctl.c | 4 ----
fs/block_dev.c | 10 ++--------
2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 60f477c..09fd7f1 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -296,14 +296,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
bd_release(bdev);
return ret;
case BLKPG:
- lock_kernel();
ret = blkpg_ioctl(bdev, (struct blkpg_ioctl_arg __user *) arg);
- unlock_kernel();
break;
case BLKRRPART:
- lock_kernel();
ret = blkdev_reread_part(bdev);
- unlock_kernel();
break;
case BLKGETSIZE:
size = bdev->bd_inode->i_size;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 99d6af8..693c2bf 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1345,13 +1345,12 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
return ret;
}

- lock_kernel();
restart:

ret = -ENXIO;
disk = get_gendisk(bdev->bd_dev, &partno);
if (!disk)
- goto out_unlock_kernel;
+ goto out;

mutex_lock_nested(&bdev->bd_mutex, for_part);
if (!bdev->bd_openers) {
@@ -1431,7 +1430,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
if (for_part)
bdev->bd_part_count++;
mutex_unlock(&bdev->bd_mutex);
- unlock_kernel();
return 0;

out_clear:
@@ -1444,9 +1442,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_contains = NULL;
out_unlock_bdev:
mutex_unlock(&bdev->bd_mutex);
- out_unlock_kernel:
- unlock_kernel();
-
+ out:
if (disk)
module_put(disk->fops->owner);
put_disk(disk);
@@ -1515,7 +1511,6 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
struct block_device *victim = NULL;

mutex_lock_nested(&bdev->bd_mutex, for_part);
- lock_kernel();
if (for_part)
bdev->bd_part_count--;

@@ -1540,7 +1535,6 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
victim = bdev->bd_contains;
bdev->bd_contains = NULL;
}
- unlock_kernel();
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
if (victim)
--
1.7.1

2010-07-03 21:48:11

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/6] block: push down BKL into .open and .release

The open and release block_device_operations are currently
called with the BKL held. In order to change that, we must
first make sure that all drivers that currently rely
on this have no regressions.

This blindly pushes the BKL into all .open and .release
operations for all block drivers to prepare for the
next step. The drivers can subsequently replace the BKL
with their own locks or remove it completely when it can
be shown that it is not needed.

Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/um/drivers/ubd_kern.c | 7 ++++++-
drivers/block/DAC960.c | 13 +++++++++----
drivers/block/amiflop.c | 12 ++++++++++--
drivers/block/aoe/aoeblk.c | 4 ++++
drivers/block/ataflop.c | 14 +++++++++++++-
drivers/block/cciss.c | 23 ++++++++++++++++++++---
drivers/block/cpqarray.c | 22 +++++++++++++++++++---
drivers/block/drbd/drbd_main.c | 4 ++++
drivers/block/floppy.c | 5 +++++
drivers/block/loop.c | 5 +++++
drivers/block/paride/pcd.c | 10 +++++++++-
drivers/block/paride/pd.c | 4 ++++
drivers/block/paride/pf.c | 20 +++++++++++++++-----
drivers/block/pktcdvd.c | 5 +++++
drivers/block/swim.c | 15 ++++++++++++++-
drivers/block/swim3.c | 15 ++++++++++++++-
drivers/block/ub.c | 17 ++++++++++++++++-
drivers/block/viodasd.c | 19 ++++++++++++++++++-
drivers/block/xen-blkfront.c | 7 +++++++
drivers/block/xsysace.c | 6 ++++++
drivers/block/z2ram.c | 13 ++++++++++---
drivers/cdrom/gdrom.c | 8 +++++++-
drivers/cdrom/viocd.c | 10 +++++++++-
drivers/ide/ide-cd.c | 14 +++++++++-----
drivers/ide/ide-gd.c | 17 ++++++++++++++++-
drivers/ide/ide-tape.c | 9 ++++++++-
drivers/md/dm.c | 7 +++++++
drivers/md/md.c | 6 ++++++
drivers/memstick/core/mspro_block.c | 9 ++++++++-
drivers/message/i2o/i2o_block.c | 4 ++++
drivers/mmc/card/block.c | 5 +++++
drivers/mtd/mtd_blkdevs.c | 6 +++++-
drivers/s390/block/dasd.c | 6 ++++++
drivers/s390/block/dcssblk.c | 5 +++++
drivers/s390/char/tape_block.c | 8 +++++++-
drivers/scsi/sd.c | 5 +++++
drivers/scsi/sr.c | 7 ++++++-
drivers/staging/hv/blkvsc_drv.c | 5 +++++
38 files changed, 331 insertions(+), 40 deletions(-)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index da992a3..1bcd208 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -33,6 +33,7 @@
#include "linux/mm.h"
#include "linux/slab.h"
#include "linux/vmalloc.h"
+#include "linux/smp_lock.h"
#include "linux/blkpg.h"
#include "linux/genhd.h"
#include "linux/spinlock.h"
@@ -1098,6 +1099,7 @@ static int ubd_open(struct block_device *bdev, fmode_t mode)
struct ubd *ubd_dev = disk->private_data;
int err = 0;

+ lock_kernel();
if(ubd_dev->count == 0){
err = ubd_open_dev(ubd_dev);
if(err){
@@ -1115,7 +1117,8 @@ static int ubd_open(struct block_device *bdev, fmode_t mode)
if(--ubd_dev->count == 0) ubd_close_dev(ubd_dev);
err = -EROFS;
}*/
- out:
+out:
+ unlock_kernel();
return err;
}

@@ -1123,8 +1126,10 @@ static int ubd_release(struct gendisk *disk, fmode_t mode)
{
struct ubd *ubd_dev = disk->private_data;

+ lock_kernel();
if(--ubd_dev->count == 0)
ubd_close_dev(ubd_dev);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/DAC960.c b/drivers/block/DAC960.c
index c5f22bb..4e2c367 100644
--- a/drivers/block/DAC960.c
+++ b/drivers/block/DAC960.c
@@ -79,23 +79,28 @@ static int DAC960_open(struct block_device *bdev, fmode_t mode)
struct gendisk *disk = bdev->bd_disk;
DAC960_Controller_T *p = disk->queue->queuedata;
int drive_nr = (long)disk->private_data;
+ int ret = -ENXIO;

+ lock_kernel();
if (p->FirmwareType == DAC960_V1_Controller) {
if (p->V1.LogicalDriveInformation[drive_nr].
LogicalDriveState == DAC960_V1_LogicalDrive_Offline)
- return -ENXIO;
+ goto out;
} else {
DAC960_V2_LogicalDeviceInfo_T *i =
p->V2.LogicalDeviceInformation[drive_nr];
if (!i || i->LogicalDeviceState == DAC960_V2_LogicalDevice_Offline)
- return -ENXIO;
+ goto out;
}

check_disk_change(bdev);

if (!get_capacity(p->disks[drive_nr]))
- return -ENXIO;
- return 0;
+ goto out;
+ ret = 0;
+out:
+ unlock_kernel();
+ return ret;
}

static int DAC960_getgeo(struct block_device *bdev, struct hd_geometry *geo)
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 7a51535..8af7af7 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1555,10 +1555,13 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
int old_dev;
unsigned long flags;

+ lock_kernel();
old_dev = fd_device[drive];

- if (fd_ref[drive] && old_dev != system)
+ if (fd_ref[drive] && old_dev != system) {
+ unlock_kernel();
return -EBUSY;
+ }

if (mode & (FMODE_READ|FMODE_WRITE)) {
check_disk_change(bdev);
@@ -1571,8 +1574,10 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
fd_deselect (drive);
rel_fdc();

- if (wrprot)
+ if (wrprot) {
+ unlock_kernel();
return -EROFS;
+ }
}
}

@@ -1589,6 +1594,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
unit[drive].type->name, data_types[system].name);

+ unlock_kernel();
return 0;
}

@@ -1597,6 +1603,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
struct amiga_floppy_struct *p = disk->private_data;
int drive = p - unit;

+ lock_kernel();
if (unit[drive].dirty == 1) {
del_timer (flush_track_timer + drive);
non_int_flush_track (drive);
@@ -1610,6 +1617,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
/* the mod_use counter is handled this way */
floppy_off (drive | 0x40000000);
#endif
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index 035cefe..3a6da77 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/genhd.h>
#include <linux/netdevice.h>
+#include <linux/smp_lock.h>
#include "aoe.h"

static struct kmem_cache *buf_pool_cache;
@@ -124,13 +125,16 @@ aoeblk_open(struct block_device *bdev, fmode_t mode)
struct aoedev *d = bdev->bd_disk->private_data;
ulong flags;

+ lock_kernel();
spin_lock_irqsave(&d->lock, flags);
if (d->flags & DEVFL_UP) {
d->nopen++;
spin_unlock_irqrestore(&d->lock, flags);
+ unlock_kernel();
return 0;
}
spin_unlock_irqrestore(&d->lock, flags);
+ unlock_kernel();
return -ENODEV;
}

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 09ae46f..f3eb6d9 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1850,22 +1850,34 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
return 0;
}

+static int floppy_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = floppy_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}

static int floppy_release(struct gendisk *disk, fmode_t mode)
{
struct atari_floppy_struct *p = disk->private_data;
+ lock_kernel();
if (p->ref < 0)
p->ref = 0;
else if (!p->ref--) {
printk(KERN_ERR "floppy_release with fd_ref == 0");
p->ref = 0;
}
+ unlock_kernel();
return 0;
}

static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
- .open = floppy_open,
+ .open = floppy_unlocked_open,
.release = floppy_release,
.ioctl = fd_unlocked_ioctl,
.media_changed = check_floppy_change,
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 6c3c8c4..f8671c1 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -169,6 +169,7 @@ static LIST_HEAD(scan_q);
static void do_cciss_request(struct request_queue *q);
static irqreturn_t do_cciss_intr(int irq, void *dev_id);
static int cciss_open(struct block_device *bdev, fmode_t mode);
+static int cciss_unlocked_open(struct block_device *bdev, fmode_t mode);
static int cciss_release(struct gendisk *disk, fmode_t mode);
static int do_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg);
@@ -223,7 +224,7 @@ static int cciss_compat_ioctl(struct block_device *, fmode_t,

static const struct block_device_operations cciss_fops = {
.owner = THIS_MODULE,
- .open = cciss_open,
+ .open = cciss_unlocked_open,
.release = cciss_release,
.ioctl = do_ioctl,
.getgeo = cciss_getgeo,
@@ -1006,13 +1007,28 @@ static int cciss_open(struct block_device *bdev, fmode_t mode)
return 0;
}

+static int cciss_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = cciss_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
/*
* Close. Sync first.
*/
static int cciss_release(struct gendisk *disk, fmode_t mode)
{
- ctlr_info_t *host = get_host(disk);
- drive_info_struct *drv = get_drv(disk);
+ ctlr_info_t *host;
+ drive_info_struct *drv;
+
+ lock_kernel();
+ host = get_host(disk);
+ drv = get_drv(disk);

#ifdef CCISS_DEBUG
printk(KERN_DEBUG "cciss_release %s\n", disk->disk_name);
@@ -1020,6 +1036,7 @@ static int cciss_release(struct gendisk *disk, fmode_t mode)

drv->usage_count--;
host->usage_count--;
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/cpqarray.c b/drivers/block/cpqarray.c
index b591518..59d55b9 100644
--- a/drivers/block/cpqarray.c
+++ b/drivers/block/cpqarray.c
@@ -158,7 +158,7 @@ static int sendcmd(
unsigned int blkcnt,
unsigned int log_unit );

-static int ida_open(struct block_device *bdev, fmode_t mode);
+static int ida_unlocked_open(struct block_device *bdev, fmode_t mode);
static int ida_release(struct gendisk *disk, fmode_t mode);
static int ida_unlocked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg);
static int ida_getgeo(struct block_device *bdev, struct hd_geometry *geo);
@@ -196,7 +196,7 @@ static inline ctlr_info_t *get_host(struct gendisk *disk)

static const struct block_device_operations ida_fops = {
.owner = THIS_MODULE,
- .open = ida_open,
+ .open = ida_unlocked_open,
.release = ida_release,
.ioctl = ida_unlocked_ioctl,
.getgeo = ida_getgeo,
@@ -841,13 +841,29 @@ static int ida_open(struct block_device *bdev, fmode_t mode)
return 0;
}

+static int ida_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = ida_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
/*
* Close. Sync first.
*/
static int ida_release(struct gendisk *disk, fmode_t mode)
{
- ctlr_info_t *host = get_host(disk);
+ ctlr_info_t *host;
+
+ lock_kernel();
+ host = get_host(disk);
host->usage_count--;
+ unlock_kernel();
+
return 0;
}

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 7258c95..a7c6d8d 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2604,6 +2604,7 @@ static int drbd_open(struct block_device *bdev, fmode_t mode)
unsigned long flags;
int rv = 0;

+ lock_kernel();
spin_lock_irqsave(&mdev->req_lock, flags);
/* to have a stable mdev->state.role
* and no race with updating open_cnt */
@@ -2618,6 +2619,7 @@ static int drbd_open(struct block_device *bdev, fmode_t mode)
if (!rv)
mdev->open_cnt++;
spin_unlock_irqrestore(&mdev->req_lock, flags);
+ unlock_kernel();

return rv;
}
@@ -2625,7 +2627,9 @@ static int drbd_open(struct block_device *bdev, fmode_t mode)
static int drbd_release(struct gendisk *gd, fmode_t mode)
{
struct drbd_conf *mdev = gd->private_data;
+ lock_kernel();
mdev->open_cnt--;
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index c61df47..73f1fd9 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3662,6 +3662,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
{
int drive = (long)disk->private_data;

+ lock_kernel();
mutex_lock(&open_lock);
if (UDRS->fd_ref < 0)
UDRS->fd_ref = 0;
@@ -3672,6 +3673,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
if (!UDRS->fd_ref)
opened_bdev[drive] = NULL;
mutex_unlock(&open_lock);
+ unlock_kernel();

return 0;
}
@@ -3689,6 +3691,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
int res = -EBUSY;
char *tmp;

+ lock_kernel();
mutex_lock(&open_lock);
old_dev = UDRS->fd_device;
if (opened_bdev[drive] && opened_bdev[drive] != bdev)
@@ -3765,6 +3768,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
goto out;
}
mutex_unlock(&open_lock);
+ unlock_kernel();
return 0;
out:
if (UDRS->fd_ref < 0)
@@ -3775,6 +3779,7 @@ out:
opened_bdev[drive] = NULL;
out2:
mutex_unlock(&open_lock);
+ unlock_kernel();
return res;
}

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 6120922..b9f5026 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -67,6 +67,7 @@
#include <linux/compat.h>
#include <linux/suspend.h>
#include <linux/freezer.h>
+#include <linux/smp_lock.h>
#include <linux/writeback.h>
#include <linux/buffer_head.h> /* for invalidate_bdev() */
#include <linux/completion.h>
@@ -1408,9 +1409,11 @@ static int lo_open(struct block_device *bdev, fmode_t mode)
{
struct loop_device *lo = bdev->bd_disk->private_data;

+ lock_kernel();
mutex_lock(&lo->lo_ctl_mutex);
lo->lo_refcnt++;
mutex_unlock(&lo->lo_ctl_mutex);
+ unlock_kernel();

return 0;
}
@@ -1420,6 +1423,7 @@ static int lo_release(struct gendisk *disk, fmode_t mode)
struct loop_device *lo = disk->private_data;
int err;

+ lock_kernel();
mutex_lock(&lo->lo_ctl_mutex);

if (--lo->lo_refcnt)
@@ -1444,6 +1448,7 @@ static int lo_release(struct gendisk *disk, fmode_t mode)
out:
mutex_unlock(&lo->lo_ctl_mutex);
out_unlocked:
+ lock_kernel();
return 0;
}

diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index daba7a6..76f8565 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -225,13 +225,21 @@ static char *pcd_buf; /* buffer for request in progress */
static int pcd_block_open(struct block_device *bdev, fmode_t mode)
{
struct pcd_unit *cd = bdev->bd_disk->private_data;
- return cdrom_open(&cd->info, bdev, mode);
+ int ret;
+
+ lock_kernel();
+ ret = cdrom_open(&cd->info, bdev, mode);
+ unlock_kernel();
+
+ return ret;
}

static int pcd_block_release(struct gendisk *disk, fmode_t mode)
{
struct pcd_unit *cd = disk->private_data;
+ lock_kernel();
cdrom_release(&cd->info, mode);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index b22943a..ad8d3a8 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -736,12 +736,14 @@ static int pd_open(struct block_device *bdev, fmode_t mode)
{
struct pd_unit *disk = bdev->bd_disk->private_data;

+ lock_kernel();
disk->access++;

if (disk->removable) {
pd_special_command(disk, pd_media_check);
pd_special_command(disk, pd_door_lock);
}
+ unlock_kernel();
return 0;
}

@@ -783,8 +785,10 @@ static int pd_release(struct gendisk *p, fmode_t mode)
{
struct pd_unit *disk = p->private_data;

+ lock_kernel();
if (!--disk->access && disk->removable)
pd_special_command(disk, pd_door_unlock);
+ unlock_kernel();

return 0;
}
diff --git a/drivers/block/paride/pf.c b/drivers/block/paride/pf.c
index 38b4d56..4457b49 100644
--- a/drivers/block/paride/pf.c
+++ b/drivers/block/paride/pf.c
@@ -300,20 +300,26 @@ static void __init pf_init_units(void)
static int pf_open(struct block_device *bdev, fmode_t mode)
{
struct pf_unit *pf = bdev->bd_disk->private_data;
+ int ret;

+ lock_kernel();
pf_identify(pf);

+ ret = -ENODEV;
if (pf->media_status == PF_NM)
- return -ENODEV;
+ goto out;

+ ret = -EROFS;
if ((pf->media_status == PF_RO) && (mode & FMODE_WRITE))
- return -EROFS;
+ goto out;

+ ret = 0;
pf->access++;
if (pf->removable)
pf_lock(pf, 1);
-
- return 0;
+out:
+ unlock_kernel();
+ return ret;
}

static int pf_getgeo(struct block_device *bdev, struct hd_geometry *geo)
@@ -354,14 +360,18 @@ static int pf_release(struct gendisk *disk, fmode_t mode)
{
struct pf_unit *pf = disk->private_data;

- if (pf->access <= 0)
+ lock_kernel();
+ if (pf->access <= 0) {
+ unlock_kernel();
return -EINVAL;
+ }

pf->access--;

if (!pf->access && pf->removable)
pf_lock(pf, 0);

+ unlock_kernel();
return 0;

}
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 1f70aec..4f2e12f 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2383,6 +2383,7 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)

VPRINTK(DRIVER_NAME": entering open\n");

+ lock_kernel();
mutex_lock(&ctl_mutex);
pd = pkt_find_dev_from_minor(MINOR(bdev->bd_dev));
if (!pd) {
@@ -2410,6 +2411,7 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)
}

mutex_unlock(&ctl_mutex);
+ unlock_kernel();
return 0;

out_dec:
@@ -2417,6 +2419,7 @@ out_dec:
out:
VPRINTK(DRIVER_NAME": failed open (%d)\n", ret);
mutex_unlock(&ctl_mutex);
+ unlock_kernel();
return ret;
}

@@ -2425,6 +2428,7 @@ static int pkt_close(struct gendisk *disk, fmode_t mode)
struct pktcdvd_device *pd = disk->private_data;
int ret = 0;

+ lock_kernel();
mutex_lock(&ctl_mutex);
pd->refcnt--;
BUG_ON(pd->refcnt < 0);
@@ -2433,6 +2437,7 @@ static int pkt_close(struct gendisk *disk, fmode_t mode)
pkt_release_dev(pd, flush);
}
mutex_unlock(&ctl_mutex);
+ unlock_kernel();
return ret;
}

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index f04f74e..2e46815 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -662,11 +662,23 @@ out:
return err;
}

+static int floppy_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = floppy_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
static int floppy_release(struct gendisk *disk, fmode_t mode)
{
struct floppy_state *fs = disk->private_data;
struct swim __iomem *base = fs->swd->base;

+ lock_kernel();
if (fs->ref_count < 0)
fs->ref_count = 0;
else if (fs->ref_count > 0)
@@ -674,6 +686,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)

if (fs->ref_count == 0)
swim_motor(base, OFF);
+ unlock_kernel();

return 0;
}
@@ -754,7 +767,7 @@ static int floppy_revalidate(struct gendisk *disk)

static const struct block_device_operations floppy_fops = {
.owner = THIS_MODULE,
- .open = floppy_open,
+ .open = floppy_unlocked_open,
.release = floppy_release,
.ioctl = floppy_ioctl,
.getgeo = floppy_getgeo,
diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 9d6bf06..7c4559e 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -949,15 +949,28 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
return 0;
}

+static int floppy_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = floppy_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
static int floppy_release(struct gendisk *disk, fmode_t mode)
{
struct floppy_state *fs = disk->private_data;
struct swim3 __iomem *sw = fs->swim3;
+ lock_kernel();
if (fs->ref_count > 0 && --fs->ref_count == 0) {
swim3_action(fs, MOTOR_OFF);
out_8(&sw->control_bic, 0xff);
swim3_select(fs, RELAX);
}
+ unlock_kernel();
return 0;
}

@@ -1008,7 +1021,7 @@ static int floppy_revalidate(struct gendisk *disk)
}

static const struct block_device_operations floppy_fops = {
- .open = floppy_open,
+ .open = floppy_unlocked_open,
.release = floppy_release,
.ioctl = floppy_unlocked_ioctl,
.media_changed = floppy_check_change,
diff --git a/drivers/block/ub.c b/drivers/block/ub.c
index d6e14d2..6701886 100644
--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -1711,6 +1711,18 @@ err_open:
return rc;
}

+static int ub_bd_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = ub_bd_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
+
/*
*/
static int ub_bd_release(struct gendisk *disk, fmode_t mode)
@@ -1718,7 +1730,10 @@ static int ub_bd_release(struct gendisk *disk, fmode_t mode)
struct ub_lun *lun = disk->private_data;
struct ub_dev *sc = lun->udev;

+ lock_kernel();
ub_put(sc);
+ unlock_kernel();
+
return 0;
}

@@ -1798,7 +1813,7 @@ static int ub_bd_media_changed(struct gendisk *disk)

static const struct block_device_operations ub_bd_fops = {
.owner = THIS_MODULE,
- .open = ub_bd_open,
+ .open = ub_bd_unlocked_open,
.release = ub_bd_release,
.ioctl = ub_bd_ioctl,
.media_changed = ub_bd_media_changed,
diff --git a/drivers/block/viodasd.c b/drivers/block/viodasd.c
index 788d938..3cbc85c 100644
--- a/drivers/block/viodasd.c
+++ b/drivers/block/viodasd.c
@@ -41,6 +41,7 @@
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/string.h>
+#include <linux/smp_lock.h>
#include <linux/dma-mapping.h>
#include <linux/completion.h>
#include <linux/device.h>
@@ -175,6 +176,18 @@ static int viodasd_open(struct block_device *bdev, fmode_t mode)
return 0;
}

+static int viodasd_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = viodasd_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
+
/*
* External release entry point.
*/
@@ -183,6 +196,7 @@ static int viodasd_release(struct gendisk *disk, fmode_t mode)
struct viodasd_device *d = disk->private_data;
HvLpEvent_Rc hvrc;

+ lock_kernel();
/* Send the event to OS/400. We DON'T expect a response */
hvrc = HvCallEvent_signalLpEventFast(viopath_hostLp,
HvLpEvent_Type_VirtualIo,
@@ -195,6 +209,9 @@ static int viodasd_release(struct gendisk *disk, fmode_t mode)
0, 0, 0);
if (hvrc != 0)
pr_warning("HV close call failed %d\n", (int)hvrc);
+
+ unlock_kernel();
+
return 0;
}

@@ -219,7 +236,7 @@ static int viodasd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
*/
static const struct block_device_operations viodasd_fops = {
.owner = THIS_MODULE,
- .open = viodasd_open,
+ .open = viodasd_unlocked_open,
.release = viodasd_release,
.getgeo = viodasd_getgeo,
};
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 96a70bb..dfb3dce 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -41,6 +41,7 @@
#include <linux/cdrom.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/smp_lock.h>
#include <linux/scatterlist.h>

#include <xen/xen.h>
@@ -1019,13 +1020,18 @@ static int blkfront_is_ready(struct xenbus_device *dev)
static int blkif_open(struct block_device *bdev, fmode_t mode)
{
struct blkfront_info *info = bdev->bd_disk->private_data;
+
+ lock_kernel();
info->users++;
+ unlock_kernel();
+
return 0;
}

static int blkif_release(struct gendisk *disk, fmode_t mode)
{
struct blkfront_info *info = disk->private_data;
+ lock_kernel();
info->users--;
if (info->users == 0) {
/* Check whether we have been instructed to close. We will
@@ -1037,6 +1043,7 @@ static int blkif_release(struct gendisk *disk, fmode_t mode)
if (state == XenbusStateClosing && info->is_ready)
blkfront_closing(dev);
}
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index a7b83c0..8d435c9 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -89,6 +89,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/ata.h>
#include <linux/hdreg.h>
#include <linux/platform_device.h>
@@ -901,11 +902,14 @@ static int ace_open(struct block_device *bdev, fmode_t mode)

dev_dbg(ace->dev, "ace_open() users=%i\n", ace->users + 1);

+ lock_kernel();
spin_lock_irqsave(&ace->lock, flags);
ace->users++;
spin_unlock_irqrestore(&ace->lock, flags);

check_disk_change(bdev);
+ unlock_kernel();
+
return 0;
}

@@ -917,6 +921,7 @@ static int ace_release(struct gendisk *disk, fmode_t mode)

dev_dbg(ace->dev, "ace_release() users=%i\n", ace->users - 1);

+ lock_kernel();
spin_lock_irqsave(&ace->lock, flags);
ace->users--;
if (ace->users == 0) {
@@ -924,6 +929,7 @@ static int ace_release(struct gendisk *disk, fmode_t mode)
ace_out(ace, ACE_CTRL, val & ~ACE_CTRL_LOCKREQ);
}
spin_unlock_irqrestore(&ace->lock, flags);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 9114654..d75b2bb 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -33,6 +33,7 @@
#include <linux/module.h>
#include <linux/blkdev.h>
#include <linux/bitops.h>
+#include <linux/smp_lock.h>
#include <linux/slab.h>

#include <asm/setup.h>
@@ -153,6 +154,7 @@ static int z2_open(struct block_device *bdev, fmode_t mode)

device = MINOR(bdev->bd_dev);

+ lock_kernel();
if ( current_device != -1 && current_device != device )
{
rc = -EBUSY;
@@ -294,20 +296,25 @@ static int z2_open(struct block_device *bdev, fmode_t mode)
set_capacity(z2ram_gendisk, z2ram_size >> 9);
}

+ unlock_kernel();
return 0;

err_out_kfree:
kfree(z2ram_map);
err_out:
+ unlock_kernel();
return rc;
}

static int
z2_release(struct gendisk *disk, fmode_t mode)
{
- if ( current_device == -1 )
- return 0;
-
+ lock_kernel();
+ if ( current_device == -1 ) {
+ unlock_kernel();
+ return 0;
+ }
+ unlock_kernel();
/*
* FIXME: unmap memory
*/
diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index 7d7b894..b5bacd1 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -493,12 +493,18 @@ static struct cdrom_device_ops gdrom_ops = {

static int gdrom_bdops_open(struct block_device *bdev, fmode_t mode)
{
- return cdrom_open(gd.cd_info, bdev, mode);
+ int ret;
+ lock_kernel();
+ ret = cdrom_open(gd.cd_info, bdev, mode);
+ unlock_kernel();
+ return ret;
}

static int gdrom_bdops_release(struct gendisk *disk, fmode_t mode)
{
+ lock_kernel();
cdrom_release(gd.cd_info, mode);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/cdrom/viocd.c b/drivers/cdrom/viocd.c
index 203c87c..74d7e55 100644
--- a/drivers/cdrom/viocd.c
+++ b/drivers/cdrom/viocd.c
@@ -155,13 +155,21 @@ static const struct file_operations proc_viocd_operations = {
static int viocd_blk_open(struct block_device *bdev, fmode_t mode)
{
struct disk_info *di = bdev->bd_disk->private_data;
- return cdrom_open(&di->viocd_info, bdev, mode);
+ int ret;
+
+ lock_kernel();
+ ret = cdrom_open(&di->viocd_info, bdev, mode);
+ unlock_kernel();
+
+ return ret;
}

static int viocd_blk_release(struct gendisk *disk, fmode_t mode)
{
struct disk_info *di = disk->private_data;
+ lock_kernel();
cdrom_release(&di->viocd_info, mode);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 03c93ae..a7fb6ea 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1585,17 +1585,19 @@ static struct ide_driver ide_cdrom_driver = {

static int idecd_open(struct block_device *bdev, fmode_t mode)
{
- struct cdrom_info *info = ide_cd_get(bdev->bd_disk);
- int rc = -ENOMEM;
+ struct cdrom_info *info;
+ int rc = -ENXIO;

+ lock_kernel();
+ info = ide_cd_get(bdev->bd_disk);
if (!info)
- return -ENXIO;
+ goto out;

rc = cdrom_open(&info->devinfo, bdev, mode);
-
if (rc < 0)
ide_cd_put(info);
-
+out:
+ unlock_kernel();
return rc;
}

@@ -1603,9 +1605,11 @@ static int idecd_release(struct gendisk *disk, fmode_t mode)
{
struct cdrom_info *info = ide_drv_g(disk, cdrom_info);

+ lock_kernel();
cdrom_release(&info->devinfo, mode);

ide_cd_put(info);
+ unlock_kernel();

return 0;
}
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index 883f0c9..137337a 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -1,3 +1,4 @@
+#include <linux/smp_lock.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/string.h>
@@ -237,6 +238,18 @@ out_put_idkp:
return ret;
}

+static int ide_gd_unlocked_open(struct block_device *bdev, fmode_t mode)
+{
+ int ret;
+
+ lock_kernel();
+ ret = ide_gd_open(bdev, mode);
+ unlock_kernel();
+
+ return ret;
+}
+
+
static int ide_gd_release(struct gendisk *disk, fmode_t mode)
{
struct ide_disk_obj *idkp = ide_drv_g(disk, ide_disk_obj);
@@ -244,6 +257,7 @@ static int ide_gd_release(struct gendisk *disk, fmode_t mode)

ide_debug_log(IDE_DBG_FUNC, "enter");

+ lock_kernel();
if (idkp->openers == 1)
drive->disk_ops->flush(drive);

@@ -255,6 +269,7 @@ static int ide_gd_release(struct gendisk *disk, fmode_t mode)
idkp->openers--;

ide_disk_put(idkp);
+ unlock_kernel();

return 0;
}
@@ -321,7 +336,7 @@ static int ide_gd_ioctl(struct block_device *bdev, fmode_t mode,

static const struct block_device_operations ide_gd_ops = {
.owner = THIS_MODULE,
- .open = ide_gd_open,
+ .open = ide_gd_unlocked_open,
.release = ide_gd_release,
.ioctl = ide_gd_ioctl,
.getgeo = ide_gd_getgeo,
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2487916..c50061e 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1906,7 +1906,11 @@ static const struct file_operations idetape_fops = {

static int idetape_open(struct block_device *bdev, fmode_t mode)
{
- struct ide_tape_obj *tape = ide_tape_get(bdev->bd_disk, false, 0);
+ struct ide_tape_obj *tape;
+
+ lock_kernel();
+ tape = ide_tape_get(bdev->bd_disk, false, 0);
+ unlock_kernel();

if (!tape)
return -ENXIO;
@@ -1918,7 +1922,10 @@ static int idetape_release(struct gendisk *disk, fmode_t mode)
{
struct ide_tape_obj *tape = ide_drv_g(disk, ide_tape_obj);

+ lock_kernel();
ide_tape_put(tape);
+ unlock_kernel();
+
return 0;
}

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index d21e128..40951a2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -15,6 +15,7 @@
#include <linux/blkpg.h>
#include <linux/bio.h>
#include <linux/buffer_head.h>
+#include <linux/smp_lock.h>
#include <linux/mempool.h>
#include <linux/slab.h>
#include <linux/idr.h>
@@ -338,6 +339,7 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
{
struct mapped_device *md;

+ lock_kernel();
spin_lock(&_minor_lock);

md = bdev->bd_disk->private_data;
@@ -355,6 +357,7 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)

out:
spin_unlock(&_minor_lock);
+ unlock_kernel();

return md ? 0 : -ENXIO;
}
@@ -362,8 +365,12 @@ out:
static int dm_blk_close(struct gendisk *disk, fmode_t mode)
{
struct mapped_device *md = disk->private_data;
+
+ lock_kernel();
atomic_dec(&md->open_count);
dm_put(md);
+ unlock_kernel();
+
return 0;
}

diff --git a/drivers/md/md.c b/drivers/md/md.c
index cb20d0b..a98f590 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -36,6 +36,7 @@
#include <linux/blkdev.h>
#include <linux/sysctl.h>
#include <linux/seq_file.h>
+#include <linux/smp_lock.h>
#include <linux/buffer_head.h> /* for invalidate_bdev */
#include <linux/poll.h>
#include <linux/ctype.h>
@@ -5902,6 +5903,7 @@ static int md_open(struct block_device *bdev, fmode_t mode)
mddev_t *mddev = mddev_find(bdev->bd_dev);
int err;

+ lock_kernel();
if (mddev->gendisk != bdev->bd_disk) {
/* we are racing with mddev_put which is discarding this
* bd_disk.
@@ -5910,6 +5912,7 @@ static int md_open(struct block_device *bdev, fmode_t mode)
/* Wait until bdev->bd_disk is definitely gone */
flush_scheduled_work();
/* Then retry the open from the top */
+ unlock_kernel();
return -ERESTARTSYS;
}
BUG_ON(mddev != bdev->bd_disk->private_data);
@@ -5923,6 +5926,7 @@ static int md_open(struct block_device *bdev, fmode_t mode)

check_disk_size_change(mddev->gendisk, bdev);
out:
+ unlock_kernel();
return err;
}

@@ -5931,8 +5935,10 @@ static int md_release(struct gendisk *disk, fmode_t mode)
mddev_t *mddev = disk->private_data;

BUG_ON(!mddev);
+ lock_kernel();
atomic_dec(&mddev->openers);
mddev_put(mddev);
+ unlock_kernel();

return 0;
}
diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 8327e24..5b1c2cf 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -18,6 +18,7 @@
#include <linux/kthread.h>
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/smp_lock.h>
#include <linux/memstick.h>

#define DRIVER_NAME "mspro_block"
@@ -179,6 +180,7 @@ static int mspro_block_bd_open(struct block_device *bdev, fmode_t mode)
struct mspro_block_data *msb = disk->private_data;
int rc = -ENXIO;

+ lock_kernel();
mutex_lock(&mspro_block_disk_lock);

if (msb && msb->card) {
@@ -190,6 +192,7 @@ static int mspro_block_bd_open(struct block_device *bdev, fmode_t mode)
}

mutex_unlock(&mspro_block_disk_lock);
+ unlock_kernel();

return rc;
}
@@ -221,7 +224,11 @@ static int mspro_block_disk_release(struct gendisk *disk)

static int mspro_block_bd_release(struct gendisk *disk, fmode_t mode)
{
- return mspro_block_disk_release(disk);
+ int ret;
+ lock_kernel();
+ ret = mspro_block_disk_release(disk);
+ unlock_kernel();
+ return ret;
}

static int mspro_block_bd_getgeo(struct block_device *bdev,
diff --git a/drivers/message/i2o/i2o_block.c b/drivers/message/i2o/i2o_block.c
index 1f3b080..5a9e477 100644
--- a/drivers/message/i2o/i2o_block.c
+++ b/drivers/message/i2o/i2o_block.c
@@ -578,6 +578,7 @@ static int i2o_block_open(struct block_device *bdev, fmode_t mode)
if (!dev->i2o_dev)
return -ENODEV;

+ lock_kernel();
if (dev->power > 0x1f)
i2o_block_device_power(dev, 0x02);

@@ -586,6 +587,7 @@ static int i2o_block_open(struct block_device *bdev, fmode_t mode)
i2o_block_device_lock(dev->i2o_dev, -1);

osm_debug("Ready.\n");
+ unlock_kernel();

return 0;
};
@@ -616,6 +618,7 @@ static int i2o_block_release(struct gendisk *disk, fmode_t mode)
if (!dev->i2o_dev)
return 0;

+ lock_kernel();
i2o_block_device_flush(dev->i2o_dev);

i2o_block_device_unlock(dev->i2o_dev, -1);
@@ -626,6 +629,7 @@ static int i2o_block_release(struct gendisk *disk, fmode_t mode)
operation = 0x24;

i2o_block_device_power(dev, operation);
+ unlock_kernel();

return 0;
}
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index cb9fbc8..8433cde 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -29,6 +29,7 @@
#include <linux/kdev_t.h>
#include <linux/blkdev.h>
#include <linux/mutex.h>
+#include <linux/smp_lock.h>
#include <linux/scatterlist.h>
#include <linux/string_helpers.h>

@@ -107,6 +108,7 @@ static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
struct mmc_blk_data *md = mmc_blk_get(bdev->bd_disk);
int ret = -ENXIO;

+ lock_kernel();
if (md) {
if (md->usage == 2)
check_disk_change(bdev);
@@ -117,6 +119,7 @@ static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
ret = -EROFS;
}
}
+ unlock_kernel();

return ret;
}
@@ -125,7 +128,9 @@ static int mmc_blk_release(struct gendisk *disk, fmode_t mode)
{
struct mmc_blk_data *md = disk->private_data;

+ lock_kernel();
mmc_blk_put(md);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 3ddc95b..7133918 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -165,8 +165,9 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
int ret;

if (!dev)
- return -ERESTARTSYS;
+ return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/

+ lock_kernel();
mutex_lock(&dev->lock);

if (!dev->mtd) {
@@ -183,6 +184,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
unlock:
mutex_unlock(&dev->lock);
blktrans_dev_put(dev);
+ unlock_kernel();
return ret;
}

@@ -194,6 +196,7 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
if (!dev)
return ret;

+ lock_kernel();
mutex_lock(&dev->lock);

/* Release one reference, we sure its not the last one here*/
@@ -206,6 +209,7 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
unlock:
mutex_unlock(&dev->lock);
blktrans_dev_put(dev);
+ unlock_kernel();
return ret;
}

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 33975e9..ae9da0d 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -21,6 +21,7 @@
#include <linux/hdreg.h>
#include <linux/async.h>
#include <linux/mutex.h>
+#include <linux/smp_lock.h>

#include <asm/ccwdev.h>
#include <asm/ebcdic.h>
@@ -2235,6 +2236,7 @@ static int dasd_open(struct block_device *bdev, fmode_t mode)
if (!block)
return -ENODEV;

+ lock_kernel();
base = block->base;
atomic_inc(&block->open_count);
if (test_bit(DASD_FLAG_OFFLINE, &base->flags)) {
@@ -2269,12 +2271,14 @@ static int dasd_open(struct block_device *bdev, fmode_t mode)
goto out;
}

+ unlock_kernel();
return 0;

out:
module_put(base->discipline->owner);
unlock:
atomic_dec(&block->open_count);
+ unlock_kernel();
return rc;
}

@@ -2282,8 +2286,10 @@ static int dasd_release(struct gendisk *disk, fmode_t mode)
{
struct dasd_block *block = disk->private_data;

+ lock_kernel();
atomic_dec(&block->open_count);
module_put(block->base->discipline->owner);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 9b43ae9..2bd72aa 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -14,6 +14,7 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/completion.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
@@ -775,6 +776,7 @@ dcssblk_open(struct block_device *bdev, fmode_t mode)
struct dcssblk_dev_info *dev_info;
int rc;

+ lock_kernel();
dev_info = bdev->bd_disk->private_data;
if (NULL == dev_info) {
rc = -ENODEV;
@@ -784,6 +786,7 @@ dcssblk_open(struct block_device *bdev, fmode_t mode)
bdev->bd_block_size = 4096;
rc = 0;
out:
+ unlock_kernel();
return rc;
}

@@ -794,6 +797,7 @@ dcssblk_release(struct gendisk *disk, fmode_t mode)
struct segment_info *entry;
int rc;

+ lock_kernel();
if (!dev_info) {
rc = -ENODEV;
goto out;
@@ -811,6 +815,7 @@ dcssblk_release(struct gendisk *disk, fmode_t mode)
up_write(&dcssblk_devices_sem);
rc = 0;
out:
+ unlock_kernel();
return rc;
}

diff --git a/drivers/s390/char/tape_block.c b/drivers/s390/char/tape_block.c
index 097da8c..b7de025 100644
--- a/drivers/s390/char/tape_block.c
+++ b/drivers/s390/char/tape_block.c
@@ -16,6 +16,7 @@
#include <linux/fs.h>
#include <linux/module.h>
#include <linux/blkdev.h>
+#include <linux/smp_lock.h>
#include <linux/interrupt.h>
#include <linux/buffer_head.h>
#include <linux/kernel.h>
@@ -361,6 +362,7 @@ tapeblock_open(struct block_device *bdev, fmode_t mode)
struct tape_device * device;
int rc;

+ lock_kernel();
device = tape_get_device(disk->private_data);

if (device->required_tapemarks) {
@@ -384,12 +386,14 @@ tapeblock_open(struct block_device *bdev, fmode_t mode)
* is called.
*/
tape_state_set(device, TS_BLKUSE);
+ unlock_kernel();
return 0;

release:
tape_release(device);
put_device:
tape_put_device(device);
+ unlock_kernel();
return rc;
}

@@ -403,10 +407,12 @@ static int
tapeblock_release(struct gendisk *disk, fmode_t mode)
{
struct tape_device *device = disk->private_data;
-
+
+ lock_kernel();
tape_state_set(device, TS_IN_USE);
tape_release(device);
tape_put_device(device);
+ unlock_kernel();

return 0;
}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ac1284a..92702b3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -758,6 +758,7 @@ static int sd_open(struct block_device *bdev, fmode_t mode)

SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));

+ lock_kernel();
sdev = sdkp->device;

/*
@@ -801,10 +802,12 @@ static int sd_open(struct block_device *bdev, fmode_t mode)
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
}

+ unlock_kernel();
return 0;

error_out:
scsi_disk_put(sdkp);
+ unlock_kernel();
return retval;
}

@@ -826,6 +829,7 @@ static int sd_release(struct gendisk *disk, fmode_t mode)

SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_release\n"));

+ lock_kernel();
if (!--sdkp->openers && sdev->removable) {
if (scsi_block_when_processing_errors(sdev))
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
@@ -836,6 +840,7 @@ static int sd_release(struct gendisk *disk, fmode_t mode)
* XXX is followed by a "rmmod sd_mod"?
*/
scsi_disk_put(sdkp);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ad62aa8..906f8e7 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -467,22 +467,27 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)

static int sr_block_open(struct block_device *bdev, fmode_t mode)
{
- struct scsi_cd *cd = scsi_cd_get(bdev->bd_disk);
+ struct scsi_cd *cd;
int ret = -ENXIO;

+ lock_kernel();
+ cd = scsi_cd_get(bdev->bd_disk);
if (cd) {
ret = cdrom_open(&cd->cdi, bdev, mode);
if (ret)
scsi_cd_put(cd);
}
+ unlock_kernel();
return ret;
}

static int sr_block_release(struct gendisk *disk, fmode_t mode)
{
struct scsi_cd *cd = scsi_cd(disk);
+ lock_kernel();
cdrom_release(&cd->cdi, mode);
scsi_cd_put(cd);
+ unlock_kernel();
return 0;
}

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 61bd0be..d3947ca 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -25,6 +25,7 @@
#include <linux/major.h>
#include <linux/delay.h>
#include <linux/hdreg.h>
+#include <linux/smp_lock.h>
#include <linux/slab.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -1324,6 +1325,7 @@ static int blkvsc_open(struct block_device *bdev, fmode_t mode)
DPRINT_DBG(BLKVSC_DRV, "- users %d disk %s\n", blkdev->users,
blkdev->gd->disk_name);

+ lock_kernel();
spin_lock(&blkdev->lock);

if (!blkdev->users && blkdev->device_type == DVD_TYPE) {
@@ -1335,6 +1337,7 @@ static int blkvsc_open(struct block_device *bdev, fmode_t mode)
blkdev->users++;

spin_unlock(&blkdev->lock);
+ unlock_kernel();
return 0;
}

@@ -1345,6 +1348,7 @@ static int blkvsc_release(struct gendisk *disk, fmode_t mode)
DPRINT_DBG(BLKVSC_DRV, "- users %d disk %s\n", blkdev->users,
blkdev->gd->disk_name);

+ lock_kernel();
spin_lock(&blkdev->lock);
if (blkdev->users == 1) {
spin_unlock(&blkdev->lock);
@@ -1355,6 +1359,7 @@ static int blkvsc_release(struct gendisk *disk, fmode_t mode)
blkdev->users--;

spin_unlock(&blkdev->lock);
+ unlock_kernel();
return 0;
}

--
1.7.1

2010-07-03 21:48:14

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 4/6] block: remove BKL from BLKROSET and BLKFLSBUF

We only call the functions set_device_ro(),
invalidate_bdev(), sync_filesystem() and sync_blockdev()
while holding the BKL in these commands. All
of these are also done in other code paths without
the BKL, which leads me to the conclusion that
the BKL is not needed here either.

The reason we hold it here is that it was originally
pushed down into the ioctl function from vfs_ioctl.

Signed-off-by: Arnd Bergmann <[email protected]>
---
block/ioctl.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 9d91e83..60f477c 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -197,10 +197,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
if (ret != -EINVAL && ret != -ENOTTY)
return ret;

- lock_kernel();
fsync_bdev(bdev);
invalidate_bdev(bdev);
- unlock_kernel();
return 0;

case BLKROSET:
@@ -212,9 +210,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
return -EACCES;
if (get_user(n, (int __user *)(arg)))
return -EFAULT;
- lock_kernel();
set_device_ro(bdev, n);
- unlock_kernel();
return 0;

case BLKDISCARD: {
--
1.7.1

2010-07-03 21:47:57

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/6] block: push BKL into blktrace ioctls

The blktrace driver currently needs the BKL, but
we should not need to take that in the block layer,
so just push it down into the driver itself.

Signed-off-by: Arnd Bergmann <[email protected]>
---
block/compat_ioctl.c | 56 ------------------------------------------
block/ioctl.c | 2 -
include/linux/blktrace_api.h | 12 +++++++++
kernel/trace/blktrace.c | 43 ++++++++++++++++++++++++++++++++
4 files changed, 55 insertions(+), 58 deletions(-)

diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
index f26051f..d530856 100644
--- a/block/compat_ioctl.c
+++ b/block/compat_ioctl.c
@@ -535,56 +535,6 @@ out:
return err;
}

-struct compat_blk_user_trace_setup {
- char name[32];
- u16 act_mask;
- u32 buf_size;
- u32 buf_nr;
- compat_u64 start_lba;
- compat_u64 end_lba;
- u32 pid;
-};
-#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup)
-
-static int compat_blk_trace_setup(struct block_device *bdev, char __user *arg)
-{
- struct blk_user_trace_setup buts;
- struct compat_blk_user_trace_setup cbuts;
- struct request_queue *q;
- char b[BDEVNAME_SIZE];
- int ret;
-
- q = bdev_get_queue(bdev);
- if (!q)
- return -ENXIO;
-
- if (copy_from_user(&cbuts, arg, sizeof(cbuts)))
- return -EFAULT;
-
- bdevname(bdev, b);
-
- buts = (struct blk_user_trace_setup) {
- .act_mask = cbuts.act_mask,
- .buf_size = cbuts.buf_size,
- .buf_nr = cbuts.buf_nr,
- .start_lba = cbuts.start_lba,
- .end_lba = cbuts.end_lba,
- .pid = cbuts.pid,
- };
- memcpy(&buts.name, &cbuts.name, 32);
-
- mutex_lock(&bdev->bd_mutex);
- ret = do_blk_trace_setup(q, b, bdev->bd_dev, bdev, &buts);
- mutex_unlock(&bdev->bd_mutex);
- if (ret)
- return ret;
-
- if (copy_to_user(arg, &buts.name, 32))
- return -EFAULT;
-
- return 0;
-}
-
static int compat_blkdev_driver_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
@@ -802,16 +752,10 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
return compat_put_u64(arg, bdev->bd_inode->i_size);

case BLKTRACESETUP32:
- lock_kernel();
- ret = compat_blk_trace_setup(bdev, compat_ptr(arg));
- unlock_kernel();
- return ret;
case BLKTRACESTART: /* compatible */
case BLKTRACESTOP: /* compatible */
case BLKTRACETEARDOWN: /* compatible */
- lock_kernel();
ret = blk_trace_ioctl(bdev, cmd, compat_ptr(arg));
- unlock_kernel();
return ret;
default:
if (disk->fops->compat_ioctl)
diff --git a/block/ioctl.c b/block/ioctl.c
index 1cfa8d4..9d91e83 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -320,9 +320,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
case BLKTRACESTOP:
case BLKTRACESETUP:
case BLKTRACETEARDOWN:
- lock_kernel();
ret = blk_trace_ioctl(bdev, cmd, (char __user *) arg);
- unlock_kernel();
break;
default:
ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h
index 416bf62..d4dd16c 100644
--- a/include/linux/blktrace_api.h
+++ b/include/linux/blktrace_api.h
@@ -5,6 +5,7 @@
#ifdef __KERNEL__
#include <linux/blkdev.h>
#include <linux/relay.h>
+#include <linux/compat.h>
#endif

/*
@@ -203,6 +204,17 @@ extern int blk_trace_init_sysfs(struct device *dev);

extern struct attribute_group blk_trace_attr_group;

+struct compat_blk_user_trace_setup {
+ char name[32];
+ u16 act_mask;
+ u32 buf_size;
+ u32 buf_nr;
+ compat_u64 start_lba;
+ compat_u64 end_lba;
+ u32 pid;
+};
+#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup)
+
#else /* !CONFIG_BLK_DEV_IO_TRACE */
# define blk_trace_ioctl(bdev, cmd, arg) (-ENOTTY)
# define blk_trace_shutdown(q) do { } while (0)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 638711c..ef4a294 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -549,6 +549,41 @@ int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
}
EXPORT_SYMBOL_GPL(blk_trace_setup);

+#if defined(CONFIG_COMPAT) && defined(CONFIG_X86_64)
+static int compat_blk_trace_setup(struct request_queue *q, char *name,
+ dev_t dev, struct block_device *bdev,
+ char __user *arg)
+{
+ struct blk_user_trace_setup buts;
+ struct compat_blk_user_trace_setup cbuts;
+ int ret;
+
+ if (copy_from_user(&cbuts, arg, sizeof(cbuts)))
+ return -EFAULT;
+
+ buts = (struct blk_user_trace_setup) {
+ .act_mask = cbuts.act_mask,
+ .buf_size = cbuts.buf_size,
+ .buf_nr = cbuts.buf_nr,
+ .start_lba = cbuts.start_lba,
+ .end_lba = cbuts.end_lba,
+ .pid = cbuts.pid,
+ };
+ memcpy(&buts.name, &cbuts.name, 32);
+
+ ret = do_blk_trace_setup(q, name, dev, bdev, &buts);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(arg, &buts.name, 32)) {
+ blk_trace_remove(q);
+ return -EFAULT;
+ }
+
+ return 0;
+}
+#endif
+
int blk_trace_startstop(struct request_queue *q, int start)
{
int ret;
@@ -601,6 +636,7 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
if (!q)
return -ENXIO;

+ lock_kernel();
mutex_lock(&bdev->bd_mutex);

switch (cmd) {
@@ -608,6 +644,12 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
bdevname(bdev, b);
ret = blk_trace_setup(q, b, bdev->bd_dev, bdev, arg);
break;
+#if defined(CONFIG_COMPAT) && defined(CONFIG_X86_64)
+ case BLKTRACESETUP32:
+ bdevname(bdev, b);
+ ret = compat_blk_trace_setup(q, b, bdev->bd_dev, bdev, arg);
+ break;
+#endif
case BLKTRACESTART:
start = 1;
case BLKTRACESTOP:
@@ -622,6 +664,7 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
}

mutex_unlock(&bdev->bd_mutex);
+ unlock_kernel();
return ret;
}

--
1.7.1

2010-07-03 21:48:01

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 6/6] scsi/sd: remove big kernel lock

Every user of the BKL in the sd driver is the
result of the pushdown from the block layer
into the open/close/ioctl functions.

None of them seem to rely on the BKL, since they
do not touch global data without holding
another lock, and the open/close functions are
still protected from concurrent execution using
the bdev->bd_mutex.

Signed-off-by: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: "James E.J. Bottomley" <[email protected]>
---
drivers/scsi/sd.c | 19 +------------------
1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 92702b3..2733068 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -758,7 +758,6 @@ static int sd_open(struct block_device *bdev, fmode_t mode)

SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));

- lock_kernel();
sdev = sdkp->device;

/*
@@ -802,12 +801,10 @@ static int sd_open(struct block_device *bdev, fmode_t mode)
scsi_set_medium_removal(sdev, SCSI_REMOVAL_PREVENT);
}

- unlock_kernel();
return 0;

error_out:
scsi_disk_put(sdkp);
- unlock_kernel();
return retval;
}

@@ -829,7 +826,6 @@ static int sd_release(struct gendisk *disk, fmode_t mode)

SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_release\n"));

- lock_kernel();
if (!--sdkp->openers && sdev->removable) {
if (scsi_block_when_processing_errors(sdev))
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
@@ -840,7 +836,6 @@ static int sd_release(struct gendisk *disk, fmode_t mode)
* XXX is followed by a "rmmod sd_mod"?
*/
scsi_disk_put(sdkp);
- unlock_kernel();
return 0;
}

@@ -921,18 +916,6 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
return scsi_ioctl(sdp, cmd, p);
}

-static int sd_unlocked_ioctl(struct block_device *bdev, fmode_t mode,
- unsigned int cmd, unsigned long arg)
-{
- int ret;
-
- lock_kernel();
- ret = sd_ioctl(bdev, mode, cmd, arg);
- unlock_kernel();
-
- return ret;
-}
-
static void set_media_not_present(struct scsi_disk *sdkp)
{
sdkp->media_present = 0;
@@ -1113,7 +1096,7 @@ static const struct block_device_operations sd_fops = {
.owner = THIS_MODULE,
.open = sd_open,
.release = sd_release,
- .ioctl = sd_unlocked_ioctl,
+ .ioctl = sd_ioctl,
.getgeo = sd_getgeo,
#ifdef CONFIG_COMPAT
.compat_ioctl = sd_compat_ioctl,
--
1.7.1

2010-07-04 07:30:53

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/6] block: push down BKL into .locked_ioctl

> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index 8a549db..1f70aec 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -57,6 +57,7 @@
> #include <linux/seq_file.h>
> #include <linux/miscdevice.h>
> #include <linux/freezer.h>
> +#include <linux/smp_lock.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> #include <scsi/scsi_cmnd.h>
> @@ -2762,10 +2763,12 @@ out_mem:
> static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg)
> {
> struct pktcdvd_device *pd = bdev->bd_disk->private_data;
> + int ret;
>
> VPRINTK("pkt_ioctl: cmd %x, dev %d:%d\n", cmd,
> MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));
>
> + lock_kernel();
> switch (cmd) {
> case CDROMEJECT:
> /*
> @@ -2783,12 +2786,14 @@ static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd,
> case CDROM_LAST_WRITTEN:
> case CDROM_SEND_PACKET:
> case SCSI_IOCTL_SEND_COMMAND:
> - return __blkdev_driver_ioctl(pd->bdev, mode, cmd, arg);
> + ret = __blkdev_driver_ioctl(pd->bdev, mode, cmd, arg);
> + break;
>
> default:
> VPRINTK(DRIVER_NAME": Unknown ioctl for %s (%x)\n", pd->name, cmd);
> - return -ENOTTY;
> + ret = -ENOTTY;
> }
> + unlock_kernel();
>
> return 0;
> }
You are loosing the return result here in the two error situations above.
Initialise ret to 0 and return ret seems the easy way to do it.

The rest looked ok - I only looked at the patches.

Sam

2010-07-04 08:01:49

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

>
> static int DAC960_getgeo(struct block_device *bdev, struct hd_geometry *geo)
> diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
> index 7a51535..8af7af7 100644
> --- a/drivers/block/amiflop.c
> +++ b/drivers/block/amiflop.c
> @@ -1555,10 +1555,13 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> int old_dev;
> unsigned long flags;
>
> + lock_kernel();
> old_dev = fd_device[drive];
>
> - if (fd_ref[drive] && old_dev != system)
> + if (fd_ref[drive] && old_dev != system) {
> + unlock_kernel();
> return -EBUSY;
> + }
>
> if (mode & (FMODE_READ|FMODE_WRITE)) {
> check_disk_change(bdev);
> @@ -1571,8 +1574,10 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> fd_deselect (drive);
> rel_fdc();
>
> - if (wrprot)
> + if (wrprot) {
> + unlock_kernel();
> return -EROFS;
> + }
> }
> }
>
> @@ -1589,6 +1594,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
> unit[drive].type->name, data_types[system].name);
>
> + unlock_kernel();
> return 0;
> }

Using goto for errorhandling here would have been nicer.
Also did you forget to include smp_locks.h?



>
> @@ -1597,6 +1603,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
> struct amiga_floppy_struct *p = disk->private_data;
> int drive = p - unit;
>
> + lock_kernel();
> if (unit[drive].dirty == 1) {
> del_timer (flush_track_timer + drive);
> non_int_flush_track (drive);
> @@ -1610,6 +1617,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
> /* the mod_use counter is handled this way */
> floppy_off (drive | 0x40000000);
> #endif
> + unlock_kernel();
> return 0;
> }

Cooked up the following replacement patch:
[Not build tested]

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 832798a..25d63cf 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -67,6 +67,7 @@
#include <linux/elevator.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
+#include <linux/smp_lock.h>

#include <asm/setup.h>
#include <asm/uaccess.h>
@@ -1541,11 +1542,16 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
int system = (MINOR(bdev->bd_dev) & 4) >> 2;
int old_dev;
unsigned long flags;
+ int err;

+ err = 0;
+ lock_kernel();
old_dev = fd_device[drive];

- if (fd_ref[drive] && old_dev != system)
- return -EBUSY;
+ if (fd_ref[drive] && old_dev != system) {
+ err = -EBUSY;
+ goto out;
+ }

if (mode & (FMODE_READ|FMODE_WRITE)) {
check_disk_change(bdev);
@@ -1558,8 +1564,10 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
fd_deselect (drive);
rel_fdc();

- if (wrprot)
- return -EROFS;
+ if (wrprot) {
+ err = -EROFS;
+ goto out;
+ }
}
}

@@ -1576,7 +1584,9 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
unit[drive].type->name, data_types[system].name);

- return 0;
+out:
+ unlock_kernel();
+ return err;
}

static int floppy_release(struct gendisk *disk, fmode_t mode)
@@ -1584,6 +1594,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
struct amiga_floppy_struct *p = disk->private_data;
int drive = p - unit;

+ lock_kernel();
if (unit[drive].dirty == 1) {
del_timer (flush_track_timer + drive);
non_int_flush_track (drive);
@@ -1597,6 +1608,7 @@ static int floppy_release(struct gendisk *disk, fmode_t mode)
/* the mod_use counter is handled this way */
floppy_off (drive | 0x40000000);
#endif
+ unlock_kernel();
return 0;
}


Feel free to use this as is.


> +++ b/drivers/block/aoe/aoeblk.c
> @@ -12,6 +12,7 @@
> #include <linux/slab.h>
> #include <linux/genhd.h>
> #include <linux/netdevice.h>
> +#include <linux/smp_lock.h>
> #include "aoe.h"
>
> static struct kmem_cache *buf_pool_cache;
> @@ -124,13 +125,16 @@ aoeblk_open(struct block_device *bdev, fmode_t mode)
> struct aoedev *d = bdev->bd_disk->private_data;
> ulong flags;
>
> + lock_kernel();
> spin_lock_irqsave(&d->lock, flags);
> if (d->flags & DEVFL_UP) {
> d->nopen++;
> spin_unlock_irqrestore(&d->lock, flags);
> + unlock_kernel();
> return 0;
> }
> spin_unlock_irqrestore(&d->lock, flags);
> + unlock_kernel();
> return -ENODEV;
> }

"goto out" would be cleaner.

> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index 1f70aec..4f2e12f 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -2383,6 +2383,7 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)
>
> VPRINTK(DRIVER_NAME": entering open\n");
>
> + lock_kernel();
> mutex_lock(&ctl_mutex);
> pd = pkt_find_dev_from_minor(MINOR(bdev->bd_dev));
> if (!pd) {
> @@ -2410,6 +2411,7 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)
> }
>
> mutex_unlock(&ctl_mutex);
> + unlock_kernel();
> return 0;
>
> out_dec:
> @@ -2417,6 +2419,7 @@ out_dec:
> out:
> VPRINTK(DRIVER_NAME": failed open (%d)\n", ret);
> mutex_unlock(&ctl_mutex);
> + unlock_kernel();
> return ret;
> }
>
> @@ -2425,6 +2428,7 @@ static int pkt_close(struct gendisk *disk, fmode_t mode)
> struct pktcdvd_device *pd = disk->private_data;
> int ret = 0;
>
> + lock_kernel();
> mutex_lock(&ctl_mutex);
> pd->refcnt--;
> BUG_ON(pd->refcnt < 0);
> @@ -2433,6 +2437,7 @@ static int pkt_close(struct gendisk *disk, fmode_t mode)
> pkt_release_dev(pd, flush);
> }
> mutex_unlock(&ctl_mutex);
> + unlock_kernel();
> return ret;
> }

Improved "goto" errorhandling would be nicer.
Is smp_lock.h included by other files?

Replacement patch (not build tested...):

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 8a549db..30d585d 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -59,6 +59,8 @@
#include <linux/freezer.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/smp_lock.h>
+
#include <scsi/scsi_cmnd.h>
#include <scsi/scsi_ioctl.h>
#include <scsi/scsi.h>
@@ -2382,11 +2384,12 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)

VPRINTK(DRIVER_NAME": entering open\n");

+ lock_kernel();
mutex_lock(&ctl_mutex);
pd = pkt_find_dev_from_minor(MINOR(bdev->bd_dev));
if (!pd) {
ret = -ENODEV;
- goto out;
+ goto out_fail;
}
BUG_ON(pd->refcnt < 0);

@@ -2408,14 +2411,16 @@ static int pkt_open(struct block_device *bdev, fmode_t mode)
set_blocksize(bdev, CD_FRAMESIZE);
}

- mutex_unlock(&ctl_mutex);
- return 0;
+ ret = 0;
+ goto out;

out_dec:
pd->refcnt--;
-out:
+out_fail:
VPRINTK(DRIVER_NAME": failed open (%d)\n", ret);
+out:
mutex_unlock(&ctl_mutex);
+ unlock_kernel();
return ret;
}

Lot's of other places could benefit from improved goto error handling.
The driver maintainers should do this (if there is a maintainer).
I stopped with the small replacement patches.

I did not find anything else going through this patch.

Sam

2010-07-04 17:58:28

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

On Sun, 4 Jul 2010 10:01:46 +0200
Sam Ravnborg <[email protected]> wrote:
> Lot's of other places could benefit from improved goto error handling.
> The driver maintainers should do this (if there is a maintainer).
> I stopped with the small replacement patches.

it's the floppy driver for amigas
not sure if any of those are still alive at all running linux.....

I think time can be more usefully spend on other drivers...

this guy probably should move to staging instead


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2010-07-04 20:27:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

On Sun, Jul 4, 2010 at 19:33, Arjan van de Ven <[email protected]> wrote:
> On Sun, 4 Jul 2010 10:01:46 +0200
> Sam Ravnborg <[email protected]> wrote:
>> Lot's of other places could benefit from improved goto error handling.
>> The driver maintainers should do this (if there is a maintainer).
>> I stopped with the small replacement patches.
>
> it's the floppy driver for amigas
> not sure if any of those are still alive at all running linux.....

Sure there are. Whether the floppy drives in them are still actively used is
another question. SSH and NFS over Ethernet are much more convenient ;-)

Last time I tried (hmm, this or previous century?), my floppy drive had problems
reading the floppy, probably my drive is dying. Oh well...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2010-07-04 21:00:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/6] block: push down BKL into .locked_ioctl

On Sunday 04 July 2010 09:30:50 Sam Ravnborg wrote:
> > default:
> > VPRINTK(DRIVER_NAME": Unknown ioctl for %s (%x)\n", pd->name, cmd);
> > - return -ENOTTY;
> > + ret = -ENOTTY;
> > }
> > + unlock_kernel();
> >
> > return 0;
> > }
> You are loosing the return result here in the two error situations above.
> Initialise ret to 0 and return ret seems the easy way to do it.
>
> The rest looked ok - I only looked at the patches.

Good catch, thanks!

I've updated the patch and pushed it out to my git tree.

Arnd

2010-07-04 21:09:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

On Sunday 04 July 2010 10:01:46 Sam Ravnborg wrote:
> > --- a/drivers/block/amiflop.c
> > +++ b/drivers/block/amiflop.c
> > @@ -1555,10 +1555,13 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> > int old_dev;
> > unsigned long flags;
> >
> > + lock_kernel();
> > old_dev = fd_device[drive];
> >
> > - if (fd_ref[drive] && old_dev != system)
> > + if (fd_ref[drive] && old_dev != system) {
> > + unlock_kernel();
> > return -EBUSY;
> > + }
> >
> > if (mode & (FMODE_READ|FMODE_WRITE)) {
> > check_disk_change(bdev);
> > @@ -1571,8 +1574,10 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> > fd_deselect (drive);
> > rel_fdc();
> >
> > - if (wrprot)
> > + if (wrprot) {
> > + unlock_kernel();
> > return -EROFS;
> > + }
> > }
> > }
> >
> > @@ -1589,6 +1594,7 @@ static int floppy_open(struct block_device *bdev, fmode_t mode)
> > printk(KERN_INFO "fd%d: accessing %s-disk with %s-layout\n",drive,
> > unit[drive].type->name, data_types[system].name);
> >
> > + unlock_kernel();
> > return 0;
> > }
>
> Using goto for errorhandling here would have been nicer.

I tried to minimize the chance for breaking stuff in code I cannot easily
test build. As shown by the bug you found in my pktcdvd patch, changing the
control flow of a function has a higher potential of introducing bugs,
so I didn't do it for drivers that people don't care much about any more.

> Also did you forget to include smp_locks.h?

No, that was already there from the first patch.

> Lot's of other places could benefit from improved goto error handling.
> The driver maintainers should do this (if there is a maintainer).

If that makes Jens accept my patches, I'd gladly change them this way.
I agree that it's cleaner and I always write my own code like that,
but when modifying (mostly) unmaintained code, my preference is on
trying to ensure the patch is correct.


> I did not find anything else going through this patch.

Ok, thanks a lot for looking through this!

Arnd

2010-07-07 01:50:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

I might be missing something, but this doesn't actually push the
BKL anyway. It adds duplicate BKL locking inside the ->open and
->release methods without actually removing it in the callers.

That's a pretty pointless thing to do. If you're not feeling
confident enough about touching block_dev.c I can look into that
part of the pushdown, but this patch on it's own is not actually
useful.

2010-07-07 01:50:39

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/6] block: push BKL into blktrace ioctls

On Sat, Jul 03, 2010 at 11:47:17PM +0200, Arnd Bergmann wrote:
> The blktrace driver currently needs the BKL, but
> we should not need to take that in the block layer,
> so just push it down into the driver itself.

Does blktrace actually need it? Anyway, pushing it down is a good
start.


Reviewed-by: Christoph Hellwig <[email protected]>

2010-07-07 01:52:10

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/6] block: push down BKL into .locked_ioctl

Looking at this again I don't like the foo_unlock_ioctl naming you're
adding. Just rename the existing ioctl routines to foo_locked_ioctl
or move the lock_kernel/unlock_kernel calls into the function, which
would be even better.

2010-07-07 01:52:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/6] block: remove BKL from BLKROSET and BLKFLSBUF

On Sat, Jul 03, 2010 at 11:47:18PM +0200, Arnd Bergmann wrote:
> We only call the functions set_device_ro(),
> invalidate_bdev(), sync_filesystem() and sync_blockdev()
> while holding the BKL in these commands. All
> of these are also done in other code paths without
> the BKL, which leads me to the conclusion that
> the BKL is not needed here either.
>
> The reason we hold it here is that it was originally
> pushed down into the ioctl function from vfs_ioctl.

Looks good,


Reviewed-by: Christoph Hellwig <[email protected]>

2010-07-07 02:04:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/6] block: remove BKL from partition code

On Sat, Jul 03, 2010 at 11:47:19PM +0200, Arnd Bergmann wrote:
> I don't see any reason why we need the BKL here.
> The functions blkdev_get, blkdev_put, blkpg_ioctl
> and blkdev_reread_part are the only remaining users
> of the big kernel lock in the block layer, and they
> all access the same fields of the bdev and gendisk
> structures, yet they always do so under the protection
> of bdev->bd_mutex.
>
> The open and close block_device_operations have all
> been converted to grab the BKL themselves, where
> necessary, so as far I can tell it should be safe
> to remove.

The content of the patch really does not match the subject at all,
which is a rather bad thing.

Anyway, dropping from blkdev_reread_part and blkpg_ioctl is easy
enough, and fine from a quick audit of the functions.

Dropping it from blkdev_get/put also seems fine from a quick
glance. But that should be part of pushing the BKL into
->open/release.

2010-07-07 02:06:26

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi/sd: remove big kernel lock

This looks fine, although relying on the bd_mutex held in the caller
for ->openers seems a bit fragile. At least add a big comment about
this fact.

2010-07-07 13:28:36

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/6] block: push down BKL into .locked_ioctl

On Wednesday 07 July 2010, Christoph Hellwig wrote:
>
> Looking at this again I don't like the foo_unlock_ioctl naming you're
> adding. Just rename the existing ioctl routines to foo_locked_ioctl
> or move the lock_kernel/unlock_kernel calls into the function, which
> would be even better.

Good point. I followed the naming that I used for the file_operations
conversion, but since the block_device_operations don't use .unlocked_ioctl,
the naming you suggested is better.

I've now changed the sr.c and sd.c to use include the lock_kernel() call
directly in their ioctl functions. For the various floppy drivers, I'd
prefer using separate wrappers because these drivers are mostly legacy
and any subtle bugs introduced could probably go unnoticed for years.

As I mentioned to Sam, I'm more confident with the wrapper than rewriting
a longish function that I can't test.

I also now changed the i2o_block driver ioctl function but split that out
into a separate patch, since I found two more existing issues with
that function that I'm addressing at the same time.

Arnd

2010-07-07 13:49:27

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/6] block: push BKL into blktrace ioctls

On Wednesday 07 July 2010, Christoph Hellwig wrote:
> On Sat, Jul 03, 2010 at 11:47:17PM +0200, Arnd Bergmann wrote:
> > The blktrace driver currently needs the BKL, but
> > we should not need to take that in the block layer,
> > so just push it down into the driver itself.
>
> Does blktrace actually need it? Anyway, pushing it down is a good
> start.

I'm rather sure that the blktrace code does not need the BKL
specifically, but it probably needs some form of serialization.
Most of the blktrace code holds both the BKL and bdev->bd_mutex,
so the right solution is probably to remove the BKL and rely
on bd_mutex.

It should be straightforward to do for anyone who understands
the blktrace code.

Arnd

2010-07-07 13:54:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi/sd: remove big kernel lock

On Wednesday 07 July 2010, Christoph Hellwig wrote:
>
> This looks fine, although relying on the bd_mutex held in the caller
> for ->openers seems a bit fragile. At least add a big comment about
> this fact.

Ok, added the comment.

Arnd

2010-07-07 14:04:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/6] block: push down BKL into .open and .release

On Wednesday 07 July 2010, Christoph Hellwig wrote:
> I might be missing something, but this doesn't actually push the
> BKL anyway. It adds duplicate BKL locking inside the ->open and
> ->release methods without actually removing it in the callers.
>
> That's a pretty pointless thing to do. If you're not feeling
> confident enough about touching block_dev.c I can look into that
> part of the pushdown, but this patch on it's own is not actually
> useful.

My idea was that the patch in its current form would be less
controversial if it can't break anything, but you're certainly
right that the description doesn't match what the patch does.

I'm now integrating the half of patch 5, as you suggested.
I would much appreciate if you could verify my assumptions
about the blkdev_get/blkdev_put functions in the combined patch.

Arnd