* [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
@ 2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [PATCH 2/5] btrfs: close_bdev_exclusive() should use the same @flags as the matching open_bdev_exclusive() Tejun Heo
` (10 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-01 16:15 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Cc: Tejun Heo, linux-mtd
There are two bdev exclusive open bugs.
* open_bdev_exclusive() must not be called with NULL holder. Use dev
as the holder.
* open_by_devnum() doesn't open the bdev exclusively but
block2mtd_free_device() always assumes it. Explicitly claim the
bdev.
The latter is rather clumsy but will be simplified with future
blkdev_get/put() cleanups.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: linux-mtd@lists.infradead.org
---
drivers/mtd/devices/block2mtd.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 2cf0cc6..806a9ed 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -246,7 +246,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
return NULL;
/* Get a handle on the device */
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, NULL);
+ bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, dev);
#ifndef MODULE
if (IS_ERR(bdev)) {
@@ -256,6 +256,15 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
dev_t devt = name_to_dev_t(devname);
if (devt) {
bdev = open_by_devnum(devt, FMODE_WRITE | FMODE_READ);
+ if (!IS_ERR(bdev)) {
+ int ret;
+ ret = bd_claim(bdev, dev);
+ if (ret) {
+ blkdev_put(bdev,
+ FMODE_READ | FMODE_WRITE);
+ bdev = ERR_PTR(ret);
+ }
+ }
}
}
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 2/5] btrfs: close_bdev_exclusive() should use the same @flags as the matching open_bdev_exclusive()
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
2010-11-01 16:15 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Tejun Heo
@ 2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [PATCH 3/5] block: simplify holder symlink handling Tejun Heo
` (9 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-01 16:15 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Cc: Tejun Heo
In the failure path of __btrfs_open_devices(), close_bdev_exclusive()
is called with @flags which doesn't match the one used during
open_bdev_exclusive(). Fix it.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Chris Mason <chris.mason@oracle.com>
---
fs/btrfs/volumes.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index cc04dc1..d395962 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -638,7 +638,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
error_brelse:
brelse(bh);
error_close:
- close_bdev_exclusive(bdev, FMODE_READ);
+ close_bdev_exclusive(bdev, flags);
error:
continue;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 3/5] block: simplify holder symlink handling
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
2010-11-01 16:15 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Tejun Heo
2010-11-01 16:15 ` [PATCH 2/5] btrfs: close_bdev_exclusive() should use the same @flags as the matching open_bdev_exclusive() Tejun Heo
@ 2010-11-01 16:15 ` Tejun Heo
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
` (8 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-01 16:15 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Cc: Tejun Heo
Code to manage symlinks in /sys/block/*/{holders|slaves} are overly
complex with multiple holder considerations, redundant extra
references to all involved kobjects, unused generic kobject holder
support and unnecessary mixup with bd_claim/release functionalities.
Strip it down to what's necessary (single gendisk holder) and make it
use a separate interface. This is a step for cleaning up
bd_claim/release. This patch makes dm-table slightly more complex but
it will be simplified again with further changes.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Neil Brown <neilb@suse.de>
Cc: dm-devel@redhat.com
---
drivers/md/dm-table.c | 23 +++-
drivers/md/md.c | 4 +-
fs/block_dev.c | 322 +++++++------------------------------------------
include/linux/fs.h | 16 ++-
4 files changed, 74 insertions(+), 291 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 90267f8..2c876ff 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -328,12 +328,22 @@ static int open_dev(struct dm_dev_internal *d, dev_t dev,
bdev = open_by_devnum(dev, d->dm_dev.mode);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
- r = bd_claim_by_disk(bdev, _claim_ptr, dm_disk(md));
- if (r)
+
+ r = bd_claim(bdev, _claim_ptr);
+ if (r) {
blkdev_put(bdev, d->dm_dev.mode);
- else
- d->dm_dev.bdev = bdev;
- return r;
+ return r;
+ }
+
+ r = bd_link_disk_holder(bdev, dm_disk(md));
+ if (r) {
+ bd_release(bdev);
+ blkdev_put(bdev, d->dm_dev.mode);
+ return r;
+ }
+
+ d->dm_dev.bdev = bdev;
+ return 0;
}
/*
@@ -344,7 +354,8 @@ static void close_dev(struct dm_dev_internal *d, struct mapped_device *md)
if (!d->dm_dev.bdev)
return;
- bd_release_from_disk(d->dm_dev.bdev, dm_disk(md));
+ bd_unlink_disk_holder(d->dm_dev.bdev);
+ bd_release(d->dm_dev.bdev);
blkdev_put(d->dm_dev.bdev, d->dm_dev.mode);
d->dm_dev.bdev = NULL;
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4e957f3..c47644f 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1880,7 +1880,7 @@ static int bind_rdev_to_array(mdk_rdev_t * rdev, mddev_t * mddev)
rdev->sysfs_state = sysfs_get_dirent_safe(rdev->kobj.sd, "state");
list_add_rcu(&rdev->same_set, &mddev->disks);
- bd_claim_by_disk(rdev->bdev, rdev->bdev->bd_holder, mddev->gendisk);
+ bd_link_disk_holder(rdev->bdev, mddev->gendisk);
/* May as well allow recovery to be retried once */
mddev->recovery_disabled = 0;
@@ -1907,7 +1907,7 @@ static void unbind_rdev_from_array(mdk_rdev_t * rdev)
MD_BUG();
return;
}
- bd_release_from_disk(rdev->bdev, rdev->mddev->gendisk);
+ bd_unlink_disk_holder(rdev->bdev);
list_del_rcu(&rdev->same_set);
printk(KERN_INFO "md: unbind<%s>\n", bdevname(rdev->bdev,b));
rdev->mddev = NULL;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 06e8ff1..9329068 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -426,9 +426,6 @@ static void init_once(void *foo)
mutex_init(&bdev->bd_mutex);
INIT_LIST_HEAD(&bdev->bd_inodes);
INIT_LIST_HEAD(&bdev->bd_list);
-#ifdef CONFIG_SYSFS
- INIT_LIST_HEAD(&bdev->bd_holder_list);
-#endif
inode_init_once(&ei->vfs_inode);
/* Initialize mutex for freeze. */
mutex_init(&bdev->bd_fsfreeze_mutex);
@@ -881,314 +878,83 @@ void bd_release(struct block_device *bdev)
EXPORT_SYMBOL(bd_release);
#ifdef CONFIG_SYSFS
-/*
- * Functions for bd_claim_by_kobject / bd_release_from_kobject
- *
- * If a kobject is passed to bd_claim_by_kobject()
- * and the kobject has a parent directory,
- * following symlinks are created:
- * o from the kobject to the claimed bdev
- * o from "holders" directory of the bdev to the parent of the kobject
- * bd_release_from_kobject() removes these symlinks.
- *
- * Example:
- * If /dev/dm-0 maps to /dev/sda, kobject corresponding to
- * /sys/block/dm-0/slaves is passed to bd_claim_by_kobject(), then:
- * /sys/block/dm-0/slaves/sda --> /sys/block/sda
- * /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
- */
-
static int add_symlink(struct kobject *from, struct kobject *to)
{
- if (!from || !to)
- return 0;
return sysfs_create_link(from, to, kobject_name(to));
}
static void del_symlink(struct kobject *from, struct kobject *to)
{
- if (!from || !to)
- return;
sysfs_remove_link(from, kobject_name(to));
}
-/*
- * 'struct bd_holder' contains pointers to kobjects symlinked by
- * bd_claim_by_kobject.
- * It's connected to bd_holder_list which is protected by bdev->bd_sem.
- */
-struct bd_holder {
- struct list_head list; /* chain of holders of the bdev */
- int count; /* references from the holder */
- struct kobject *sdir; /* holder object, e.g. "/block/dm-0/slaves" */
- struct kobject *hdev; /* e.g. "/block/dm-0" */
- struct kobject *hdir; /* e.g. "/block/sda/holders" */
- struct kobject *sdev; /* e.g. "/block/sda" */
-};
-
-/*
- * Get references of related kobjects at once.
- * Returns 1 on success. 0 on failure.
- *
- * Should call bd_holder_release_dirs() after successful use.
- */
-static int bd_holder_grab_dirs(struct block_device *bdev,
- struct bd_holder *bo)
-{
- if (!bdev || !bo)
- return 0;
-
- bo->sdir = kobject_get(bo->sdir);
- if (!bo->sdir)
- return 0;
-
- bo->hdev = kobject_get(bo->sdir->parent);
- if (!bo->hdev)
- goto fail_put_sdir;
-
- bo->sdev = kobject_get(&part_to_dev(bdev->bd_part)->kobj);
- if (!bo->sdev)
- goto fail_put_hdev;
-
- bo->hdir = kobject_get(bdev->bd_part->holder_dir);
- if (!bo->hdir)
- goto fail_put_sdev;
-
- return 1;
-
-fail_put_sdev:
- kobject_put(bo->sdev);
-fail_put_hdev:
- kobject_put(bo->hdev);
-fail_put_sdir:
- kobject_put(bo->sdir);
-
- return 0;
-}
-
-/* Put references of related kobjects at once. */
-static void bd_holder_release_dirs(struct bd_holder *bo)
-{
- kobject_put(bo->hdir);
- kobject_put(bo->sdev);
- kobject_put(bo->hdev);
- kobject_put(bo->sdir);
-}
-
-static struct bd_holder *alloc_bd_holder(struct kobject *kobj)
-{
- struct bd_holder *bo;
-
- bo = kzalloc(sizeof(*bo), GFP_KERNEL);
- if (!bo)
- return NULL;
-
- bo->count = 1;
- bo->sdir = kobj;
-
- return bo;
-}
-
-static void free_bd_holder(struct bd_holder *bo)
-{
- kfree(bo);
-}
-
/**
- * find_bd_holder - find matching struct bd_holder from the block device
+ * bd_link_disk_holder - create symlinks between holding disk and slave bdev
+ * @bdev: the claimed slave bdev
+ * @disk: the holding disk
*
- * @bdev: struct block device to be searched
- * @bo: target struct bd_holder
+ * This functions creates the following sysfs symlinks.
*
- * Returns matching entry with @bo in @bdev->bd_holder_list.
- * If found, increment the reference count and return the pointer.
- * If not found, returns NULL.
- */
-static struct bd_holder *find_bd_holder(struct block_device *bdev,
- struct bd_holder *bo)
-{
- struct bd_holder *tmp;
-
- list_for_each_entry(tmp, &bdev->bd_holder_list, list)
- if (tmp->sdir == bo->sdir) {
- tmp->count++;
- return tmp;
- }
-
- return NULL;
-}
-
-/**
- * add_bd_holder - create sysfs symlinks for bd_claim() relationship
+ * - from "slaves" directory of the holder @disk to the claimed @bdev
+ * - from "holders" directory of the @bdev to the holder @disk
*
- * @bdev: block device to be bd_claimed
- * @bo: preallocated and initialized by alloc_bd_holder()
+ * For example, if /dev/dm-0 maps to /dev/sda and disk for dm-0 is
+ * passed to bd_link_disk_holder(), then:
*
- * Add @bo to @bdev->bd_holder_list, create symlinks.
+ * /sys/block/dm-0/slaves/sda --> /sys/block/sda
+ * /sys/block/sda/holders/dm-0 --> /sys/block/dm-0
*
- * Returns 0 if symlinks are created.
- * Returns -ve if something fails.
- */
-static int add_bd_holder(struct block_device *bdev, struct bd_holder *bo)
-{
- int err;
-
- if (!bo)
- return -EINVAL;
-
- if (!bd_holder_grab_dirs(bdev, bo))
- return -EBUSY;
-
- err = add_symlink(bo->sdir, bo->sdev);
- if (err)
- return err;
-
- err = add_symlink(bo->hdir, bo->hdev);
- if (err) {
- del_symlink(bo->sdir, bo->sdev);
- return err;
- }
-
- list_add_tail(&bo->list, &bdev->bd_holder_list);
- return 0;
-}
-
-/**
- * del_bd_holder - delete sysfs symlinks for bd_claim() relationship
- *
- * @bdev: block device to be bd_claimed
- * @kobj: holder's kobject
- *
- * If there is matching entry with @kobj in @bdev->bd_holder_list
- * and no other bd_claim() from the same kobject,
- * remove the struct bd_holder from the list, delete symlinks for it.
- *
- * Returns a pointer to the struct bd_holder when it's removed from the list
- * and ready to be freed.
- * Returns NULL if matching claim isn't found or there is other bd_claim()
- * by the same kobject.
- */
-static struct bd_holder *del_bd_holder(struct block_device *bdev,
- struct kobject *kobj)
-{
- struct bd_holder *bo;
-
- list_for_each_entry(bo, &bdev->bd_holder_list, list) {
- if (bo->sdir == kobj) {
- bo->count--;
- BUG_ON(bo->count < 0);
- if (!bo->count) {
- list_del(&bo->list);
- del_symlink(bo->sdir, bo->sdev);
- del_symlink(bo->hdir, bo->hdev);
- bd_holder_release_dirs(bo);
- return bo;
- }
- break;
- }
- }
-
- return NULL;
-}
-
-/**
- * bd_claim_by_kobject - bd_claim() with additional kobject signature
- *
- * @bdev: block device to be claimed
- * @holder: holder's signature
- * @kobj: holder's kobject
+ * The caller must have claimed @bdev before calling this function and
+ * ensure that both @bdev and @disk are valid during the creation and
+ * lifetime of these symlinks.
*
- * Do bd_claim() and if it succeeds, create sysfs symlinks between
- * the bdev and the holder's kobject.
- * Use bd_release_from_kobject() when relesing the claimed bdev.
+ * CONTEXT:
+ * Might sleep.
*
- * Returns 0 on success. (same as bd_claim())
- * Returns errno on failure.
+ * RETURNS:
+ * 0 on success, -errno on failure.
*/
-static int bd_claim_by_kobject(struct block_device *bdev, void *holder,
- struct kobject *kobj)
+int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
{
- int err;
- struct bd_holder *bo, *found;
-
- if (!kobj)
- return -EINVAL;
-
- bo = alloc_bd_holder(kobj);
- if (!bo)
- return -ENOMEM;
+ int ret = 0;
mutex_lock(&bdev->bd_mutex);
- err = bd_claim(bdev, holder);
- if (err)
- goto fail;
+ WARN_ON_ONCE(!bdev->bd_holder || bdev->bd_holder_disk);
- found = find_bd_holder(bdev, bo);
- if (found)
- goto fail;
+ /* FIXME: remove the following once add_disk() handles errors */
+ if (WARN_ON(!disk->slave_dir || !bdev->bd_part->holder_dir))
+ goto out_unlock;
- err = add_bd_holder(bdev, bo);
- if (err)
- bd_release(bdev);
- else
- bo = NULL;
-fail:
- mutex_unlock(&bdev->bd_mutex);
- free_bd_holder(bo);
- return err;
-}
+ ret = add_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+ if (ret)
+ goto out_unlock;
-/**
- * bd_release_from_kobject - bd_release() with additional kobject signature
- *
- * @bdev: block device to be released
- * @kobj: holder's kobject
- *
- * Do bd_release() and remove sysfs symlinks created by bd_claim_by_kobject().
- */
-static void bd_release_from_kobject(struct block_device *bdev,
- struct kobject *kobj)
-{
- if (!kobj)
- return;
+ ret = add_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
+ if (ret) {
+ del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+ goto out_unlock;
+ }
- mutex_lock(&bdev->bd_mutex);
- bd_release(bdev);
- free_bd_holder(del_bd_holder(bdev, kobj));
+ bdev->bd_holder_disk = disk;
+out_unlock:
mutex_unlock(&bdev->bd_mutex);
+ return ret;
}
+EXPORT_SYMBOL_GPL(bd_link_disk_holder);
-/**
- * bd_claim_by_disk - wrapper function for bd_claim_by_kobject()
- *
- * @bdev: block device to be claimed
- * @holder: holder's signature
- * @disk: holder's gendisk
- *
- * Call bd_claim_by_kobject() with getting @disk->slave_dir.
- */
-int bd_claim_by_disk(struct block_device *bdev, void *holder,
- struct gendisk *disk)
+void bd_unlink_disk_holder(struct block_device *bdev)
{
- return bd_claim_by_kobject(bdev, holder, kobject_get(disk->slave_dir));
-}
-EXPORT_SYMBOL_GPL(bd_claim_by_disk);
+ struct gendisk *disk = bdev->bd_holder_disk;
-/**
- * bd_release_from_disk - wrapper function for bd_release_from_kobject()
- *
- * @bdev: block device to be claimed
- * @disk: holder's gendisk
- *
- * Call bd_release_from_kobject() and put @disk->slave_dir.
- */
-void bd_release_from_disk(struct block_device *bdev, struct gendisk *disk)
-{
- bd_release_from_kobject(bdev, disk->slave_dir);
- kobject_put(disk->slave_dir);
+ bdev->bd_holder_disk = NULL;
+ if (!disk)
+ return;
+
+ del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
+ del_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
}
-EXPORT_SYMBOL_GPL(bd_release_from_disk);
+EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
#endif
/*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1eb2939..b361848 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -663,7 +663,7 @@ struct block_device {
void * bd_holder;
int bd_holders;
#ifdef CONFIG_SYSFS
- struct list_head bd_holder_list;
+ struct gendisk * bd_holder_disk; /* for sysfs slave linkng */
#endif
struct block_device * bd_contains;
unsigned bd_block_size;
@@ -2043,11 +2043,17 @@ extern int blkdev_put(struct block_device *, fmode_t);
extern int bd_claim(struct block_device *, void *);
extern void bd_release(struct block_device *);
#ifdef CONFIG_SYSFS
-extern int bd_claim_by_disk(struct block_device *, void *, struct gendisk *);
-extern void bd_release_from_disk(struct block_device *, struct gendisk *);
+extern int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
+extern void bd_unlink_disk_holder(struct block_device *bdev);
#else
-#define bd_claim_by_disk(bdev, holder, disk) bd_claim(bdev, holder)
-#define bd_release_from_disk(bdev, disk) bd_release(bdev)
+static inline int bd_link_disk_holder(struct block_device *bdev,
+ struct gendisk *disk)
+{
+ return 0;
+}
+static inline void bd_unlink_disk_holder(struct block_device *bdev)
+{
+}
#endif
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (2 preceding siblings ...)
2010-11-01 16:15 ` [PATCH 3/5] block: simplify holder symlink handling Tejun Heo
@ 2010-11-01 16:15 ` Tejun Heo
2010-11-03 15:06 ` Jan Kara
2010-11-03 16:10 ` Christoph Hellwig
2010-11-01 16:15 ` [PATCH 5/5] block: reorganize claim/release implementation Tejun Heo
` (7 subsequent siblings)
11 siblings, 2 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-01 16:15 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Cc: Tejun Heo
Over time, block layer has accumulated a set of APIs dealing with bdev
open, close, claim and release.
* blkdev_get/put() are the primary open and close functions.
* bd_claim/release() deal with exclusive open.
* open/close_bdev_exclusive() are combination of open and claim and
the other way around, respectively.
* bd_link/unlink_disk_holder() to create and remove holder/slave
symlinks.
* open_by_devnum() wraps bdget() + blkdev_get().
The interface is a bit confusing and the decoupling of open and claim
makes it impossible to properly guarantee exclusive access as
in-kernel open + claim sequence can disturb the existing exclusive
open even before the block layer knows the current open if for another
exclusive access. Reorganize the interface such that,
* blkdev_get() is extended to include exclusive access management.
@holder argument is added and, if is @FMODE_EXCL specified, it will
gain exclusive access atomically w.r.t. other exclusive accesses.
* blkdev_put() is similarly extended. It now takes @mode argument and
if @FMODE_EXCL is set, it releases an exclusive access. Also, when
the last exclusive claim is released, the holder/slave symlinks are
removed automatically.
* bd_claim/release() and close_bdev_exclusive() are no longer
necessary and either made static or removed.
* bd_link_disk_holder() remains the same but bd_unlink_disk_holder()
is no longer necessary and removed.
* open_bdev_exclusive() becomes a simple wrapper around lookup_bdev()
and blkdev_get(). It also has an unexpected extra bdev_read_only()
test which probably should be moved into blkdev_get().
* open_by_devnum() is modified to take @holder argument and pass it to
blkdev_get().
Most of bdev open/close operations are unified into blkdev_get/put()
and most exclusive accesses are tested atomically at the open time (as
it should). This cleans up code and removes some, both valid and
invalid, but unnecessary all the same, corner cases.
open_bdev_exclusive() and open_by_devnum() can use further cleanup -
rename to blkdev_get_by_path() and blkdev_get_by_devt() and drop
special features. Well, let's leave them for another day.
Most conversions are straight-forward. drbd conversion is a bit more
involved as there was some reordering, but the logic should stay the
same.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Peter Osterlund <petero2@telia.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: Alex Elder <aelder@sgi.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: dm-devel@redhat.com
Cc: drbd-dev@lists.linbit.com
Cc: Neil Brown <neilb@suse.de>
Cc: Leo Chen <leochen@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Joern Engel <joern@logfs.org>
Cc: KONISHI Ryusuke <konishi.ryusuke@lab.ntt.co.jp>
Cc: reiserfs-devel@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
---
block/ioctl.c | 5 +-
drivers/block/drbd/drbd_int.h | 2 -
drivers/block/drbd/drbd_main.c | 7 +--
drivers/block/drbd/drbd_nl.c | 105 ++++++++++------------------
drivers/block/pktcdvd.c | 22 ++----
drivers/char/raw.c | 14 +---
drivers/md/dm-table.c | 15 +---
drivers/md/md.c | 14 +---
drivers/mtd/devices/block2mtd.c | 17 +----
drivers/s390/block/dasd_genhd.c | 2 +-
fs/block_dev.c | 149 +++++++++++++--------------------------
fs/btrfs/volumes.c | 14 ++--
fs/ext3/super.c | 12 +---
fs/ext4/super.c | 12 +---
fs/gfs2/ops_fstype.c | 4 +-
fs/jfs/jfs_logmgr.c | 17 ++---
fs/logfs/dev_bdev.c | 4 +-
fs/nilfs2/super.c | 4 +-
fs/ocfs2/cluster/heartbeat.c | 2 +-
fs/partitions/check.c | 2 +-
fs/reiserfs/journal.c | 17 ++---
fs/super.c | 14 ++--
fs/xfs/linux-2.6/xfs_super.c | 2 +-
include/linux/fs.h | 14 +---
kernel/power/swap.c | 5 +-
mm/swapfile.c | 7 +-
26 files changed, 163 insertions(+), 319 deletions(-)
diff --git a/block/ioctl.c b/block/ioctl.c
index d724ceb..cc46d49 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -294,11 +294,12 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
return -EINVAL;
if (get_user(n, (int __user *) arg))
return -EFAULT;
- if (!(mode & FMODE_EXCL) && bd_claim(bdev, &bdev) < 0)
+ if (!(mode & FMODE_EXCL) &&
+ blkdev_get(bdev, mode | FMODE_EXCL, &bdev) < 0)
return -EBUSY;
ret = set_blocksize(bdev, n);
if (!(mode & FMODE_EXCL))
- bd_release(bdev);
+ blkdev_put(bdev, mode | FMODE_EXCL);
return ret;
case BLKPG:
ret = blkpg_ioctl(bdev, (struct blkpg_ioctl_arg __user *) arg);
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
index 9bdcf43..0590b9f 100644
--- a/drivers/block/drbd/drbd_int.h
+++ b/drivers/block/drbd/drbd_int.h
@@ -923,8 +923,6 @@ struct drbd_md {
struct drbd_backing_dev {
struct block_device *backing_bdev;
struct block_device *md_bdev;
- struct file *lo_file;
- struct file *md_file;
struct drbd_md md;
struct disk_conf dc; /* The user provided config... */
sector_t known_size; /* last known size of that backing device */
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 25c7a73..7ec1a82 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -3361,11 +3361,8 @@ void drbd_free_bc(struct drbd_backing_dev *ldev)
if (ldev == NULL)
return;
- bd_release(ldev->backing_bdev);
- bd_release(ldev->md_bdev);
-
- fput(ldev->lo_file);
- fput(ldev->md_file);
+ blkdev_put(ldev->backing_bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
+ blkdev_put(ldev->md_bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
kfree(ldev);
}
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index 87925e9..fd03460 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -855,7 +855,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
sector_t max_possible_sectors;
sector_t min_md_device_sectors;
struct drbd_backing_dev *nbc = NULL; /* new_backing_conf */
- struct inode *inode, *inode2;
+ struct block_device *bdev;
struct lru_cache *resync_lru = NULL;
union drbd_state ns, os;
unsigned int max_seg_s;
@@ -902,46 +902,40 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
}
}
- nbc->lo_file = filp_open(nbc->dc.backing_dev, O_RDWR, 0);
- if (IS_ERR(nbc->lo_file)) {
+ bdev = open_bdev_exclusive(nbc->dc.backing_dev,
+ FMODE_READ | FMODE_WRITE, mdev);
+ if (IS_ERR(bdev)) {
dev_err(DEV, "open(\"%s\") failed with %ld\n", nbc->dc.backing_dev,
- PTR_ERR(nbc->lo_file));
- nbc->lo_file = NULL;
+ PTR_ERR(bdev));
retcode = ERR_OPEN_DISK;
goto fail;
}
+ nbc->backing_bdev = bdev;
- inode = nbc->lo_file->f_dentry->d_inode;
-
- if (!S_ISBLK(inode->i_mode)) {
- retcode = ERR_DISK_NOT_BDEV;
- goto fail;
- }
-
- nbc->md_file = filp_open(nbc->dc.meta_dev, O_RDWR, 0);
- if (IS_ERR(nbc->md_file)) {
+ /*
+ * meta_dev_idx >= 0: external fixed size, possibly multiple
+ * drbd sharing one meta device. TODO in that case, paranoia
+ * check that [md_bdev, meta_dev_idx] is not yet used by some
+ * other drbd minor! (if you use drbd.conf + drbdadm, that
+ * should check it for you already; but if you don't, or
+ * someone fooled it, we need to double check here)
+ */
+ bdev = open_bdev_exclusive(nbc->dc.meta_dev,
+ FMODE_READ | FMODE_WRITE,
+ (nbc->dc.meta_dev_idx < 0) ?
+ (void *)mdev : (void *)drbd_m_holder);
+ if (IS_ERR(bdev)) {
dev_err(DEV, "open(\"%s\") failed with %ld\n", nbc->dc.meta_dev,
- PTR_ERR(nbc->md_file));
- nbc->md_file = NULL;
+ PTR_ERR(bdev));
retcode = ERR_OPEN_MD_DISK;
goto fail;
}
+ nbc->md_bdev = bdev;
- inode2 = nbc->md_file->f_dentry->d_inode;
-
- if (!S_ISBLK(inode2->i_mode)) {
- retcode = ERR_MD_NOT_BDEV;
- goto fail;
- }
-
- nbc->backing_bdev = inode->i_bdev;
- if (bd_claim(nbc->backing_bdev, mdev)) {
- printk(KERN_ERR "drbd: bd_claim(%p,%p); failed [%p;%p;%u]\n",
- nbc->backing_bdev, mdev,
- nbc->backing_bdev->bd_holder,
- nbc->backing_bdev->bd_contains->bd_holder,
- nbc->backing_bdev->bd_holders);
- retcode = ERR_BDCLAIM_DISK;
+ if ((nbc->backing_bdev == nbc->md_bdev) !=
+ (nbc->dc.meta_dev_idx == DRBD_MD_INDEX_INTERNAL ||
+ nbc->dc.meta_dev_idx == DRBD_MD_INDEX_FLEX_INT)) {
+ retcode = ERR_MD_IDX_INVALID;
goto fail;
}
@@ -950,28 +944,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
offsetof(struct bm_extent, lce));
if (!resync_lru) {
retcode = ERR_NOMEM;
- goto release_bdev_fail;
- }
-
- /* meta_dev_idx >= 0: external fixed size,
- * possibly multiple drbd sharing one meta device.
- * TODO in that case, paranoia check that [md_bdev, meta_dev_idx] is
- * not yet used by some other drbd minor!
- * (if you use drbd.conf + drbdadm,
- * that should check it for you already; but if you don't, or someone
- * fooled it, we need to double check here) */
- nbc->md_bdev = inode2->i_bdev;
- if (bd_claim(nbc->md_bdev, (nbc->dc.meta_dev_idx < 0) ? (void *)mdev
- : (void *) drbd_m_holder)) {
- retcode = ERR_BDCLAIM_MD_DISK;
- goto release_bdev_fail;
- }
-
- if ((nbc->backing_bdev == nbc->md_bdev) !=
- (nbc->dc.meta_dev_idx == DRBD_MD_INDEX_INTERNAL ||
- nbc->dc.meta_dev_idx == DRBD_MD_INDEX_FLEX_INT)) {
- retcode = ERR_MD_IDX_INVALID;
- goto release_bdev2_fail;
+ goto fail;
}
/* RT - for drbd_get_max_capacity() DRBD_MD_INDEX_FLEX_INT */
@@ -982,7 +955,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
(unsigned long long) drbd_get_max_capacity(nbc),
(unsigned long long) nbc->dc.disk_size);
retcode = ERR_DISK_TO_SMALL;
- goto release_bdev2_fail;
+ goto fail;
}
if (nbc->dc.meta_dev_idx < 0) {
@@ -999,7 +972,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
dev_warn(DEV, "refusing attach: md-device too small, "
"at least %llu sectors needed for this meta-disk type\n",
(unsigned long long) min_md_device_sectors);
- goto release_bdev2_fail;
+ goto fail;
}
/* Make sure the new disk is big enough
@@ -1007,7 +980,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
if (drbd_get_max_capacity(nbc) <
drbd_get_capacity(mdev->this_bdev)) {
retcode = ERR_DISK_TO_SMALL;
- goto release_bdev2_fail;
+ goto fail;
}
nbc->known_size = drbd_get_capacity(nbc->backing_bdev);
@@ -1030,7 +1003,7 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
retcode = _drbd_request_state(mdev, NS(disk, D_ATTACHING), CS_VERBOSE);
drbd_resume_io(mdev);
if (retcode < SS_SUCCESS)
- goto release_bdev2_fail;
+ goto fail;
if (!get_ldev_if_state(mdev, D_ATTACHING))
goto force_diskless;
@@ -1264,18 +1237,14 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
force_diskless:
drbd_force_state(mdev, NS(disk, D_DISKLESS));
drbd_md_sync(mdev);
- release_bdev2_fail:
- if (nbc)
- bd_release(nbc->md_bdev);
- release_bdev_fail:
- if (nbc)
- bd_release(nbc->backing_bdev);
fail:
if (nbc) {
- if (nbc->lo_file)
- fput(nbc->lo_file);
- if (nbc->md_file)
- fput(nbc->md_file);
+ if (nbc->backing_bdev)
+ blkdev_put(nbc->backing_bdev,
+ FMODE_READ | FMODE_WRITE | FMODE_EXCL);
+ if (nbc->md_bdev)
+ blkdev_put(nbc->md_bdev,
+ FMODE_READ | FMODE_WRITE | FMODE_EXCL);
kfree(nbc);
}
lc_destroy(resync_lru);
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 19b3568..77d70ee 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2296,15 +2296,12 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
* so bdget() can't fail.
*/
bdget(pd->bdev->bd_dev);
- if ((ret = blkdev_get(pd->bdev, FMODE_READ)))
+ if ((ret = blkdev_get(pd->bdev, FMODE_READ | FMODE_EXCL, pd)))
goto out;
- if ((ret = bd_claim(pd->bdev, pd)))
- goto out_putdev;
-
if ((ret = pkt_get_last_written(pd, &lba))) {
printk(DRIVER_NAME": pkt_get_last_written failed\n");
- goto out_unclaim;
+ goto out_putdev;
}
set_capacity(pd->disk, lba << 2);
@@ -2314,7 +2311,7 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
q = bdev_get_queue(pd->bdev);
if (write) {
if ((ret = pkt_open_write(pd)))
- goto out_unclaim;
+ goto out_putdev;
/*
* Some CDRW drives can not handle writes larger than one packet,
* even if the size is a multiple of the packet size.
@@ -2329,23 +2326,21 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
}
if ((ret = pkt_set_segment_merging(pd, q)))
- goto out_unclaim;
+ goto out_putdev;
if (write) {
if (!pkt_grow_pktlist(pd, CONFIG_CDROM_PKTCDVD_BUFFERS)) {
printk(DRIVER_NAME": not enough memory for buffers\n");
ret = -ENOMEM;
- goto out_unclaim;
+ goto out_putdev;
}
printk(DRIVER_NAME": %lukB available on disc\n", lba << 1);
}
return 0;
-out_unclaim:
- bd_release(pd->bdev);
out_putdev:
- blkdev_put(pd->bdev, FMODE_READ);
+ blkdev_put(pd->bdev, FMODE_READ | FMODE_EXCL);
out:
return ret;
}
@@ -2362,8 +2357,7 @@ static void pkt_release_dev(struct pktcdvd_device *pd, int flush)
pkt_lock_door(pd, 0);
pkt_set_speed(pd, MAX_SPEED, MAX_SPEED);
- bd_release(pd->bdev);
- blkdev_put(pd->bdev, FMODE_READ);
+ blkdev_put(pd->bdev, FMODE_READ | FMODE_EXCL);
pkt_shrink_pktlist(pd);
}
@@ -2733,7 +2727,7 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
bdev = bdget(dev);
if (!bdev)
return -ENOMEM;
- ret = blkdev_get(bdev, FMODE_READ | FMODE_NDELAY);
+ ret = blkdev_get(bdev, FMODE_READ | FMODE_NDELAY, NULL);
if (ret)
return ret;
diff --git a/drivers/char/raw.c b/drivers/char/raw.c
index bfe25ea..b4b9d5a 100644
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -65,15 +65,12 @@ static int raw_open(struct inode *inode, struct file *filp)
if (!bdev)
goto out;
igrab(bdev->bd_inode);
- err = blkdev_get(bdev, filp->f_mode);
+ err = blkdev_get(bdev, filp->f_mode | FMODE_EXCL, raw_open);
if (err)
goto out;
- err = bd_claim(bdev, raw_open);
- if (err)
- goto out1;
err = set_blocksize(bdev, bdev_logical_block_size(bdev));
if (err)
- goto out2;
+ goto out1;
filp->f_flags |= O_DIRECT;
filp->f_mapping = bdev->bd_inode->i_mapping;
if (++raw_devices[minor].inuse == 1)
@@ -83,10 +80,8 @@ static int raw_open(struct inode *inode, struct file *filp)
mutex_unlock(&raw_mutex);
return 0;
-out2:
- bd_release(bdev);
out1:
- blkdev_put(bdev, filp->f_mode);
+ blkdev_put(bdev, filp->f_mode | FMODE_EXCL);
out:
mutex_unlock(&raw_mutex);
return err;
@@ -110,8 +105,7 @@ static int raw_release(struct inode *inode, struct file *filp)
}
mutex_unlock(&raw_mutex);
- bd_release(bdev);
- blkdev_put(bdev, filp->f_mode);
+ blkdev_put(bdev, filp->f_mode | FMODE_EXCL);
return 0;
}
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 2c876ff..9e88ca0 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -325,20 +325,13 @@ static int open_dev(struct dm_dev_internal *d, dev_t dev,
BUG_ON(d->dm_dev.bdev);
- bdev = open_by_devnum(dev, d->dm_dev.mode);
+ bdev = open_by_devnum(dev, d->dm_dev.mode | FMODE_EXCL, _claim_ptr);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
- r = bd_claim(bdev, _claim_ptr);
- if (r) {
- blkdev_put(bdev, d->dm_dev.mode);
- return r;
- }
-
r = bd_link_disk_holder(bdev, dm_disk(md));
if (r) {
- bd_release(bdev);
- blkdev_put(bdev, d->dm_dev.mode);
+ blkdev_put(bdev, d->dm_dev.mode | FMODE_EXCL);
return r;
}
@@ -354,9 +347,7 @@ static void close_dev(struct dm_dev_internal *d, struct mapped_device *md)
if (!d->dm_dev.bdev)
return;
- bd_unlink_disk_holder(d->dm_dev.bdev);
- bd_release(d->dm_dev.bdev);
- blkdev_put(d->dm_dev.bdev, d->dm_dev.mode);
+ blkdev_put(d->dm_dev.bdev, d->dm_dev.mode | FMODE_EXCL);
d->dm_dev.bdev = NULL;
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c47644f..6af951f 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1907,7 +1907,6 @@ static void unbind_rdev_from_array(mdk_rdev_t * rdev)
MD_BUG();
return;
}
- bd_unlink_disk_holder(rdev->bdev);
list_del_rcu(&rdev->same_set);
printk(KERN_INFO "md: unbind<%s>\n", bdevname(rdev->bdev,b));
rdev->mddev = NULL;
@@ -1935,19 +1934,13 @@ static int lock_rdev(mdk_rdev_t *rdev, dev_t dev, int shared)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE);
+ bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ shared ? (mdk_rdev_t *)lock_rdev : rdev);
if (IS_ERR(bdev)) {
printk(KERN_ERR "md: could not open %s.\n",
__bdevname(dev, b));
return PTR_ERR(bdev);
}
- err = bd_claim(bdev, shared ? (mdk_rdev_t *)lock_rdev : rdev);
- if (err) {
- printk(KERN_ERR "md: could not bd_claim %s.\n",
- bdevname(bdev, b));
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
- return err;
- }
if (!shared)
set_bit(AllReserved, &rdev->flags);
rdev->bdev = bdev;
@@ -1960,8 +1953,7 @@ static void unlock_rdev(mdk_rdev_t *rdev)
rdev->bdev = NULL;
if (!bdev)
MD_BUG();
- bd_release(bdev);
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
void md_autodetect_dev(dev_t dev);
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 806a9ed..84a4045 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -224,7 +224,7 @@ static void block2mtd_free_device(struct block2mtd_dev *dev)
if (dev->blkdev) {
invalidate_mapping_pages(dev->blkdev->bd_inode->i_mapping,
0, -1);
- close_bdev_exclusive(dev->blkdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(dev->blkdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
kfree(dev);
@@ -254,18 +254,9 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
to resolve the device name by other means. */
dev_t devt = name_to_dev_t(devname);
- if (devt) {
- bdev = open_by_devnum(devt, FMODE_WRITE | FMODE_READ);
- if (!IS_ERR(bdev)) {
- int ret;
- ret = bd_claim(bdev, dev);
- if (ret) {
- blkdev_put(bdev,
- FMODE_READ | FMODE_WRITE);
- bdev = ERR_PTR(ret);
- }
- }
- }
+ if (devt)
+ bdev = open_by_devnum(devt,
+ FMODE_READ|FMODE_WRITE|FMODE_EXCL, dev);
}
#endif
diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index 30a1ca3..5505bc0 100644
--- a/drivers/s390/block/dasd_genhd.c
+++ b/drivers/s390/block/dasd_genhd.c
@@ -103,7 +103,7 @@ int dasd_scan_partitions(struct dasd_block *block)
struct block_device *bdev;
bdev = bdget_disk(block->gdp, 0);
- if (!bdev || blkdev_get(bdev, FMODE_READ) < 0)
+ if (!bdev || blkdev_get(bdev, FMODE_READ, NULL) < 0)
return -ENODEV;
/*
* See fs/partition/check.c:register_disk,rescan_partitions
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9329068..fc48912 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -660,7 +660,7 @@ static bool bd_may_claim(struct block_device *bdev, struct block_device *whole,
else if (bdev->bd_contains == bdev)
return true; /* is a whole device which isn't held */
- else if (whole->bd_holder == bd_claim)
+ else if (whole->bd_holder == bd_may_claim)
return true; /* is a partition of a device that is being partitioned */
else if (whole->bd_holder != NULL)
return false; /* is a partition of a held device */
@@ -807,10 +807,10 @@ static void __bd_claim(struct block_device *bdev, struct block_device *whole,
{
/* note that for a whole device bd_holders
* will be incremented twice, and bd_holder will
- * be set to bd_claim before being set to holder
+ * be set to bd_may_claim before being set to holder
*/
whole->bd_holders++;
- whole->bd_holder = bd_claim;
+ whole->bd_holder = bd_may_claim;
bdev->bd_holders++;
bdev->bd_holder = holder;
}
@@ -835,37 +835,7 @@ static void bd_finish_claiming(struct block_device *bdev,
__bd_abort_claiming(whole, holder); /* not actually an abort */
}
-/**
- * bd_claim - claim a block device
- * @bdev: block device to claim
- * @holder: holder trying to claim @bdev
- *
- * Try to claim @bdev which must have been opened successfully.
- *
- * CONTEXT:
- * Might sleep.
- *
- * RETURNS:
- * 0 if successful, -EBUSY if @bdev is already claimed.
- */
-int bd_claim(struct block_device *bdev, void *holder)
-{
- struct block_device *whole = bdev->bd_contains;
- int res;
-
- might_sleep();
-
- spin_lock(&bdev_lock);
- res = bd_prepare_to_claim(bdev, whole, holder);
- if (res == 0)
- __bd_claim(bdev, whole, holder);
- spin_unlock(&bdev_lock);
-
- return res;
-}
-EXPORT_SYMBOL(bd_claim);
-
-void bd_release(struct block_device *bdev)
+static void bd_release(struct block_device *bdev)
{
spin_lock(&bdev_lock);
if (!--bdev->bd_contains->bd_holders)
@@ -875,8 +845,6 @@ void bd_release(struct block_device *bdev)
spin_unlock(&bdev_lock);
}
-EXPORT_SYMBOL(bd_release);
-
#ifdef CONFIG_SYSFS
static int add_symlink(struct kobject *from, struct kobject *to)
{
@@ -943,7 +911,7 @@ out_unlock:
}
EXPORT_SYMBOL_GPL(bd_link_disk_holder);
-void bd_unlink_disk_holder(struct block_device *bdev)
+static void bd_unlink_disk_holder(struct block_device *bdev)
{
struct gendisk *disk = bdev->bd_holder_disk;
@@ -954,7 +922,9 @@ void bd_unlink_disk_holder(struct block_device *bdev)
del_symlink(disk->slave_dir, &part_to_dev(bdev->bd_part)->kobj);
del_symlink(bdev->bd_part->holder_dir, &disk_to_dev(disk)->kobj);
}
-EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
+#else
+static inline void bd_unlink_disk_holder(struct block_device *bdev)
+{ }
#endif
/*
@@ -964,12 +934,12 @@ EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
* to be used for internal purposes. If you ever need it - reconsider
* your API.
*/
-struct block_device *open_by_devnum(dev_t dev, fmode_t mode)
+struct block_device *open_by_devnum(dev_t dev, fmode_t mode, void *holder)
{
struct block_device *bdev = bdget(dev);
int err = -ENOMEM;
if (bdev)
- err = blkdev_get(bdev, mode);
+ err = blkdev_get(bdev, mode, holder);
return err ? ERR_PTR(err) : bdev;
}
@@ -1235,17 +1205,37 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
return ret;
}
-int blkdev_get(struct block_device *bdev, fmode_t mode)
+int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
{
- return __blkdev_get(bdev, mode, 0);
+ struct block_device *whole = NULL;
+ int res;
+
+ WARN_ON_ONCE((mode & FMODE_EXCL) && !holder);
+
+ if ((mode & FMODE_EXCL) && holder) {
+ whole = bd_start_claiming(bdev, holder);
+ if (IS_ERR(whole)) {
+ bdput(bdev);
+ return PTR_ERR(whole);
+ }
+ }
+
+ res = __blkdev_get(bdev, mode, 0);
+
+ if (whole) {
+ if (res == 0)
+ bd_finish_claiming(bdev, whole, holder);
+ else
+ bd_abort_claiming(whole, holder);
+ }
+
+ return res;
}
EXPORT_SYMBOL(blkdev_get);
static int blkdev_open(struct inode * inode, struct file * filp)
{
- struct block_device *whole = NULL;
struct block_device *bdev;
- int res;
/*
* Preserve backwards compatibility and allow large file access
@@ -1266,26 +1256,9 @@ static int blkdev_open(struct inode * inode, struct file * filp)
if (bdev == NULL)
return -ENOMEM;
- if (filp->f_mode & FMODE_EXCL) {
- whole = bd_start_claiming(bdev, filp);
- if (IS_ERR(whole)) {
- bdput(bdev);
- return PTR_ERR(whole);
- }
- }
-
filp->f_mapping = bdev->bd_inode->i_mapping;
- res = blkdev_get(bdev, filp->f_mode);
-
- if (whole) {
- if (res == 0)
- bd_finish_claiming(bdev, whole, filp);
- else
- bd_abort_claiming(whole, filp);
- }
-
- return res;
+ return blkdev_get(bdev, filp->f_mode, filp);
}
static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
@@ -1329,6 +1302,13 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
int blkdev_put(struct block_device *bdev, fmode_t mode)
{
+ if (mode & FMODE_EXCL) {
+ mutex_lock(&bdev->bd_mutex);
+ bd_release(bdev);
+ if (!bdev->bd_holders)
+ bd_unlink_disk_holder(bdev);
+ mutex_unlock(&bdev->bd_mutex);
+ }
return __blkdev_put(bdev, mode, 0);
}
EXPORT_SYMBOL(blkdev_put);
@@ -1336,8 +1316,7 @@ EXPORT_SYMBOL(blkdev_put);
static int blkdev_close(struct inode * inode, struct file * filp)
{
struct block_device *bdev = I_BDEV(filp->f_mapping->host);
- if (bdev->bd_holder == filp)
- bd_release(bdev);
+
return blkdev_put(bdev, filp->f_mode);
}
@@ -1494,55 +1473,27 @@ EXPORT_SYMBOL(lookup_bdev);
*/
struct block_device *open_bdev_exclusive(const char *path, fmode_t mode, void *holder)
{
- struct block_device *bdev, *whole;
+ struct block_device *bdev;
int error;
bdev = lookup_bdev(path);
if (IS_ERR(bdev))
return bdev;
- whole = bd_start_claiming(bdev, holder);
- if (IS_ERR(whole)) {
- bdput(bdev);
- return whole;
- }
-
- error = blkdev_get(bdev, mode);
+ error = blkdev_get(bdev, mode | FMODE_EXCL, holder);
if (error)
- goto out_abort_claiming;
+ return ERR_PTR(error);
- error = -EACCES;
- if ((mode & FMODE_WRITE) && bdev_read_only(bdev))
- goto out_blkdev_put;
+ if ((mode & FMODE_WRITE) && bdev_read_only(bdev)) {
+ blkdev_put(bdev, mode);
+ return ERR_PTR(-EACCES);
+ }
- bd_finish_claiming(bdev, whole, holder);
return bdev;
-
-out_blkdev_put:
- blkdev_put(bdev, mode);
-out_abort_claiming:
- bd_abort_claiming(whole, holder);
- return ERR_PTR(error);
}
EXPORT_SYMBOL(open_bdev_exclusive);
-/**
- * close_bdev_exclusive - close a blockdevice opened by open_bdev_exclusive()
- *
- * @bdev: blockdevice to close
- * @mode: mode, must match that used to open.
- *
- * This is the counterpart to open_bdev_exclusive().
- */
-void close_bdev_exclusive(struct block_device *bdev, fmode_t mode)
-{
- bd_release(bdev);
- blkdev_put(bdev, mode);
-}
-
-EXPORT_SYMBOL(close_bdev_exclusive);
-
int __invalidate_device(struct block_device *bdev)
{
struct super_block *sb = get_super(bdev);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d395962..f1b729d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -489,7 +489,7 @@ again:
continue;
if (device->bdev) {
- close_bdev_exclusive(device->bdev, device->mode);
+ blkdev_put(device->bdev, device->mode | FMODE_EXCL);
device->bdev = NULL;
fs_devices->open_devices--;
}
@@ -523,7 +523,7 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
list_for_each_entry(device, &fs_devices->devices, dev_list) {
if (device->bdev) {
- close_bdev_exclusive(device->bdev, device->mode);
+ blkdev_put(device->bdev, device->mode | FMODE_EXCL);
fs_devices->open_devices--;
}
if (device->writeable) {
@@ -638,7 +638,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
error_brelse:
brelse(bh);
error_close:
- close_bdev_exclusive(bdev, flags);
+ blkdev_put(bdev, flags | FMODE_EXCL);
error:
continue;
}
@@ -716,7 +716,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
brelse(bh);
error_close:
- close_bdev_exclusive(bdev, flags);
+ blkdev_put(bdev, flags | FMODE_EXCL);
error:
mutex_unlock(&uuid_mutex);
return ret;
@@ -1244,7 +1244,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
root->fs_info->fs_devices->latest_bdev = next_device->bdev;
if (device->bdev) {
- close_bdev_exclusive(device->bdev, device->mode);
+ blkdev_put(device->bdev, device->mode | FMODE_EXCL);
device->bdev = NULL;
device->fs_devices->open_devices--;
}
@@ -1287,7 +1287,7 @@ error_brelse:
brelse(bh);
error_close:
if (bdev)
- close_bdev_exclusive(bdev, FMODE_READ);
+ blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
out:
mutex_unlock(&root->fs_info->volume_mutex);
mutex_unlock(&uuid_mutex);
@@ -1565,7 +1565,7 @@ out:
mutex_unlock(&root->fs_info->volume_mutex);
return ret;
error:
- close_bdev_exclusive(bdev, 0);
+ blkdev_put(bdev, FMODE_EXCL);
if (seeding_dev) {
mutex_unlock(&uuid_mutex);
up_write(&sb->s_umount);
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 2fedaf8..23e7513 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -347,7 +347,7 @@ static struct block_device *ext3_blkdev_get(dev_t dev, struct super_block *sb)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE);
+ bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
if (IS_ERR(bdev))
goto fail;
return bdev;
@@ -364,8 +364,7 @@ fail:
*/
static int ext3_blkdev_put(struct block_device *bdev)
{
- bd_release(bdev);
- return blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
+ return blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
static int ext3_blkdev_remove(struct ext3_sb_info *sbi)
@@ -2136,13 +2135,6 @@ static journal_t *ext3_get_dev_journal(struct super_block *sb,
if (bdev == NULL)
return NULL;
- if (bd_claim(bdev, sb)) {
- ext3_msg(sb, KERN_ERR,
- "error: failed to claim external journal device");
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
- return NULL;
- }
-
blocksize = sb->s_blocksize;
hblock = bdev_logical_block_size(bdev);
if (blocksize < hblock) {
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 40131b7..612ff3d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -647,7 +647,7 @@ static struct block_device *ext4_blkdev_get(dev_t dev, struct super_block *sb)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE);
+ bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
if (IS_ERR(bdev))
goto fail;
return bdev;
@@ -663,8 +663,7 @@ fail:
*/
static int ext4_blkdev_put(struct block_device *bdev)
{
- bd_release(bdev);
- return blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
+ return blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
static int ext4_blkdev_remove(struct ext4_sb_info *sbi)
@@ -3732,13 +3731,6 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
if (bdev == NULL)
return NULL;
- if (bd_claim(bdev, sb)) {
- ext4_msg(sb, KERN_ERR,
- "failed to claim external journal device");
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
- return NULL;
- }
-
blocksize = sb->s_blocksize;
hblock = bdev_logical_block_size(bdev);
if (blocksize < hblock) {
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 3eb1393..c1f0763 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1298,7 +1298,7 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
goto error_bdev;
if (s->s_root)
- close_bdev_exclusive(bdev, mode);
+ blkdev_put(bdev, mode | FMODE_EXCL);
memset(&args, 0, sizeof(args));
args.ar_quota = GFS2_QUOTA_DEFAULT;
@@ -1342,7 +1342,7 @@ error_super:
deactivate_locked_super(s);
return ERR_PTR(error);
error_bdev:
- close_bdev_exclusive(bdev, mode);
+ blkdev_put(bdev, mode | FMODE_EXCL);
return ERR_PTR(error);
}
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index e1b8493..5a290f2 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1120,16 +1120,13 @@ int lmLogOpen(struct super_block *sb)
* file systems to log may have n-to-1 relationship;
*/
- bdev = open_by_devnum(sbi->logdev, FMODE_READ|FMODE_WRITE);
+ bdev = open_by_devnum(sbi->logdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ log);
if (IS_ERR(bdev)) {
rc = -PTR_ERR(bdev);
goto free;
}
- if ((rc = bd_claim(bdev, log))) {
- goto close;
- }
-
log->bdev = bdev;
memcpy(log->uuid, sbi->loguuid, sizeof(log->uuid));
@@ -1137,7 +1134,7 @@ int lmLogOpen(struct super_block *sb)
* initialize log:
*/
if ((rc = lmLogInit(log)))
- goto unclaim;
+ goto close;
list_add(&log->journal_list, &jfs_external_logs);
@@ -1163,11 +1160,8 @@ journal_found:
list_del(&log->journal_list);
lbmLogShutdown(log);
- unclaim:
- bd_release(bdev);
-
close: /* close external log device */
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
free: /* free log descriptor */
mutex_unlock(&jfs_log_mutex);
@@ -1512,8 +1506,7 @@ int lmLogClose(struct super_block *sb)
bdev = log->bdev;
rc = lmLogShutdown(log);
- bd_release(bdev);
- blkdev_put(bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
kfree(log);
diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index 92ca6fb..734b902 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -300,7 +300,7 @@ static int bdev_write_sb(struct super_block *sb, struct page *page)
static void bdev_put_device(struct logfs_super *s)
{
- close_bdev_exclusive(s->s_bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(s->s_bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
static int bdev_can_write_buf(struct super_block *sb, u64 ofs)
@@ -331,7 +331,7 @@ int logfs_get_sb_bdev(struct logfs_super *p, struct file_system_type *type,
if (MAJOR(bdev->bd_dev) == MTD_BLOCK_MAJOR) {
int mtdnr = MINOR(bdev->bd_dev);
- close_bdev_exclusive(bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
return logfs_get_sb_mtd(p, mtdnr);
}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index f804d41..756a679 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1233,7 +1233,7 @@ nilfs_mount(struct file_system_type *fs_type, int flags,
}
if (!s_new)
- close_bdev_exclusive(sd.bdev, mode);
+ blkdev_put(sd.bdev, mode | FMODE_EXCL);
return root_dentry;
@@ -1242,7 +1242,7 @@ nilfs_mount(struct file_system_type *fs_type, int flags,
failed:
if (!s_new)
- close_bdev_exclusive(sd.bdev, mode);
+ blkdev_put(sd.bdev, mode | FMODE_EXCL);
return ERR_PTR(err);
}
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 52c7557..d0a2721 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1674,7 +1674,7 @@ static ssize_t o2hb_region_dev_write(struct o2hb_region *reg,
goto out;
reg->hr_bdev = I_BDEV(filp->f_mapping->host);
- ret = blkdev_get(reg->hr_bdev, FMODE_WRITE | FMODE_READ);
+ ret = blkdev_get(reg->hr_bdev, FMODE_WRITE | FMODE_READ, NULL);
if (ret) {
reg->hr_bdev = NULL;
goto out;
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 0a8b0ad..2e6501d 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -549,7 +549,7 @@ void register_disk(struct gendisk *disk)
goto exit;
bdev->bd_invalidated = 1;
- err = blkdev_get(bdev, FMODE_READ);
+ err = blkdev_get(bdev, FMODE_READ, NULL);
if (err < 0)
goto exit;
blkdev_put(bdev, FMODE_READ);
diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 076c8b1..b488136 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2552,8 +2552,6 @@ static int release_journal_dev(struct super_block *super,
result = 0;
if (journal->j_dev_bd != NULL) {
- if (journal->j_dev_bd->bd_dev != super->s_dev)
- bd_release(journal->j_dev_bd);
result = blkdev_put(journal->j_dev_bd, journal->j_dev_mode);
journal->j_dev_bd = NULL;
}
@@ -2571,7 +2569,7 @@ static int journal_init_dev(struct super_block *super,
{
int result;
dev_t jdev;
- fmode_t blkdev_mode = FMODE_READ | FMODE_WRITE;
+ fmode_t blkdev_mode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
char b[BDEVNAME_SIZE];
result = 0;
@@ -2585,7 +2583,9 @@ static int journal_init_dev(struct super_block *super,
/* there is no "jdev" option and journal is on separate device */
if ((!jdev_name || !jdev_name[0])) {
- journal->j_dev_bd = open_by_devnum(jdev, blkdev_mode);
+ if (jdev == super->s_dev)
+ blkdev_mode &= ~FMODE_EXCL;
+ journal->j_dev_bd = open_by_devnum(jdev, blkdev_mode, journal);
journal->j_dev_mode = blkdev_mode;
if (IS_ERR(journal->j_dev_bd)) {
result = PTR_ERR(journal->j_dev_bd);
@@ -2594,15 +2594,8 @@ static int journal_init_dev(struct super_block *super,
"cannot init journal device '%s': %i",
__bdevname(jdev, b), result);
return result;
- } else if (jdev != super->s_dev) {
- result = bd_claim(journal->j_dev_bd, journal);
- if (result) {
- blkdev_put(journal->j_dev_bd, blkdev_mode);
- return result;
- }
-
+ } else if (jdev != super->s_dev)
set_blocksize(journal->j_dev_bd, super->s_blocksize);
- }
return 0;
}
diff --git a/fs/super.c b/fs/super.c
index ca69615..22374bf 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -801,13 +801,13 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
/*
* s_umount nests inside bd_mutex during
- * __invalidate_device(). close_bdev_exclusive()
- * acquires bd_mutex and can't be called under
- * s_umount. Drop s_umount temporarily. This is safe
- * as we're holding an active reference.
+ * __invalidate_device(). blkdev_put() acquires
+ * bd_mutex and can't be called under s_umount. Drop
+ * s_umount temporarily. This is safe as we're
+ * holding an active reference.
*/
up_write(&s->s_umount);
- close_bdev_exclusive(bdev, mode);
+ blkdev_put(bdev, mode | FMODE_EXCL);
down_write(&s->s_umount);
} else {
char b[BDEVNAME_SIZE];
@@ -831,7 +831,7 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
error_s:
error = PTR_ERR(s);
error_bdev:
- close_bdev_exclusive(bdev, mode);
+ blkdev_put(bdev, mode | FMODE_EXCL);
error:
return ERR_PTR(error);
}
@@ -862,7 +862,7 @@ void kill_block_super(struct super_block *sb)
bdev->bd_super = NULL;
generic_shutdown_super(sb);
sync_blockdev(bdev);
- close_bdev_exclusive(bdev, mode);
+ blkdev_put(bdev, mode | FMODE_EXCL);
}
EXPORT_SYMBOL(kill_block_super);
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index 9f3a78f..a1a6e5c 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -623,7 +623,7 @@ xfs_blkdev_put(
struct block_device *bdev)
{
if (bdev)
- close_bdev_exclusive(bdev, FMODE_READ|FMODE_WRITE);
+ blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
}
/*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b361848..64bd2f4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2007,7 +2007,8 @@ extern struct block_device *bdgrab(struct block_device *bdev);
extern void bd_set_size(struct block_device *, loff_t size);
extern void bd_forget(struct inode *inode);
extern void bdput(struct block_device *);
-extern struct block_device *open_by_devnum(dev_t, fmode_t);
+extern struct block_device *open_by_devnum(dev_t dev, fmode_t mode,
+ void *holder);
extern void invalidate_bdev(struct block_device *);
extern int sync_blockdev(struct block_device *bdev);
extern struct super_block *freeze_bdev(struct block_device *);
@@ -2038,22 +2039,16 @@ extern const struct file_operations def_fifo_fops;
extern int ioctl_by_bdev(struct block_device *, unsigned, unsigned long);
extern int blkdev_ioctl(struct block_device *, fmode_t, unsigned, unsigned long);
extern long compat_blkdev_ioctl(struct file *, unsigned, unsigned long);
-extern int blkdev_get(struct block_device *, fmode_t);
-extern int blkdev_put(struct block_device *, fmode_t);
-extern int bd_claim(struct block_device *, void *);
-extern void bd_release(struct block_device *);
+extern int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder);
+extern int blkdev_put(struct block_device *bdev, fmode_t mode);
#ifdef CONFIG_SYSFS
extern int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
-extern void bd_unlink_disk_holder(struct block_device *bdev);
#else
static inline int bd_link_disk_holder(struct block_device *bdev,
struct gendisk *disk)
{
return 0;
}
-static inline void bd_unlink_disk_holder(struct block_device *bdev)
-{
-}
#endif
#endif
@@ -2090,7 +2085,6 @@ extern const char *__bdevname(dev_t, char *buffer);
extern const char *bdevname(struct block_device *bdev, char *buffer);
extern struct block_device *lookup_bdev(const char *);
extern struct block_device *open_bdev_exclusive(const char *, fmode_t, void *);
-extern void close_bdev_exclusive(struct block_device *, fmode_t);
extern void blkdev_show(struct seq_file *,off_t);
#else
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index a0e4a86..513a77f 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -223,7 +223,7 @@ static int swsusp_swap_check(void)
return res;
root_swap = res;
- res = blkdev_get(hib_resume_bdev, FMODE_WRITE);
+ res = blkdev_get(hib_resume_bdev, FMODE_WRITE, NULL);
if (res)
return res;
@@ -907,7 +907,8 @@ int swsusp_check(void)
{
int error;
- hib_resume_bdev = open_by_devnum(swsusp_resume_device, FMODE_READ);
+ hib_resume_bdev = open_by_devnum(swsusp_resume_device,
+ FMODE_READ, NULL);
if (!IS_ERR(hib_resume_bdev)) {
set_blocksize(hib_resume_bdev, PAGE_SIZE);
clear_page(swsusp_header);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 67ddaaf..b6adcfb 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1677,7 +1677,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
if (S_ISBLK(inode->i_mode)) {
struct block_device *bdev = I_BDEV(inode);
set_blocksize(bdev, p->old_block_size);
- bd_release(bdev);
+ blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
} else {
mutex_lock(&inode->i_mutex);
inode->i_flags &= ~S_SWAPFILE;
@@ -1939,7 +1939,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
error = -EINVAL;
if (S_ISBLK(inode->i_mode)) {
bdev = I_BDEV(inode);
- error = bd_claim(bdev, sys_swapon);
+ error = blkdev_get(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL,
+ sys_swapon);
if (error < 0) {
bdev = NULL;
error = -EINVAL;
@@ -2136,7 +2137,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
bad_swap:
if (bdev) {
set_blocksize(bdev, p->old_block_size);
- bd_release(bdev);
+ blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
}
destroy_swap_extents(p);
swap_cgroup_swapoff(type);
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
@ 2010-11-03 15:06 ` Jan Kara
2010-11-09 10:18 ` Tejun Heo
2010-11-03 16:10 ` Christoph Hellwig
1 sibling, 1 reply; 27+ messages in thread
From: Jan Kara @ 2010-11-03 15:06 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
On Mon 01-11-10 17:15:28, Tejun Heo wrote:
> Over time, block layer has accumulated a set of APIs dealing with bdev
> open, close, claim and release.
>
> * blkdev_get/put() are the primary open and close functions.
>
> * bd_claim/release() deal with exclusive open.
>
> * open/close_bdev_exclusive() are combination of open and claim and
> the other way around, respectively.
>
> * bd_link/unlink_disk_holder() to create and remove holder/slave
> symlinks.
>
> * open_by_devnum() wraps bdget() + blkdev_get().
>
> The interface is a bit confusing and the decoupling of open and claim
> makes it impossible to properly guarantee exclusive access as
> in-kernel open + claim sequence can disturb the existing exclusive
> open even before the block layer knows the current open if for another
> exclusive access. Reorganize the interface such that,
...
The patch looks OK to me as far as ext3, ext4, and reiserfs are
concerned. One thing I wondered about when I looked at it - does someone
use the 'mode' argument of the blkdev_put() function (well, apart from the
exclusive flag)? Because I've looked at a few random disk ->release()
functions and none of them used it...
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-03 15:06 ` Jan Kara
@ 2010-11-09 10:18 ` Tejun Heo
0 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-09 10:18 UTC (permalink / raw)
To: Jan Kara
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
Hello,
On 11/03/2010 04:06 PM, Jan Kara wrote:
> On Mon 01-11-10 17:15:28, Tejun Heo wrote:
> The patch looks OK to me as far as ext3, ext4, and reiserfs are
> concerned. One thing I wondered about when I looked at it - does someone
> use the 'mode' argument of the blkdev_put() function (well, apart from the
> exclusive flag)? Because I've looked at a few random disk ->release()
> functions and none of them used it...
Yeah, I thought about removing @mode and make blkdev_put() take
@holder for exclusive releases. From what I can see, it doesn't seem
like any implementation cares but it's customary to have access to
@mode on ->release(), so I decided to keep it. It's rather flaky as
there's no mechanism to check or enforce it tho.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
2010-11-03 15:06 ` Jan Kara
@ 2010-11-03 16:10 ` Christoph Hellwig
2010-11-04 15:04 ` Mike Snitzer
2010-11-09 10:34 ` Tejun Heo
1 sibling, 2 replies; 27+ messages in thread
From: Christoph Hellwig @ 2010-11-03 16:10 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
On Mon, Nov 01, 2010 at 05:15:28PM +0100, Tejun Heo wrote:
> * blkdev_get() is extended to include exclusive access management.
> @holder argument is added and, if is @FMODE_EXCL specified, it will
> gain exclusive access atomically w.r.t. other exclusive accesses.
>
> * blkdev_put() is similarly extended. It now takes @mode argument and
> if @FMODE_EXCL is set, it releases an exclusive access. Also, when
> the last exclusive claim is released, the holder/slave symlinks are
> removed automatically.
Could we get rid of FMODE_EXCL and just make a non-NULL holder field
mean to open it exlusively (and pass a holder to the blkdev_put to
release it)?
> * bd_link_disk_holder() remains the same but bd_unlink_disk_holder()
> is no longer necessary and removed.
That's a rather asymetric interface. What about having
blkdev_get_stacked that require a gendisk as holder and set up the
links underneath?
> open_bdev_exclusive() and open_by_devnum() can use further cleanup -
> rename to blkdev_get_by_path() and blkdev_get_by_devt() and drop
> special features. Well, let's leave them for another day.
s/blkdev_get_by_devt/blkdev_get_by_dev/
And yes, that rename is a good idea and should go in ASAP after this
patch.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-03 16:10 ` Christoph Hellwig
@ 2010-11-04 15:04 ` Mike Snitzer
2010-11-09 10:34 ` Tejun Heo
1 sibling, 0 replies; 27+ messages in thread
From: Mike Snitzer @ 2010-11-04 15:04 UTC (permalink / raw)
To: device-mapper development
Cc: jack, leochen, heiko.carstens, adilger.kernel, konishi.ryusuke,
shaggy, drbd-dev, joel.becker, hch, aelder, mfasheh, joern,
reiserfs-devel, schwidefsky, viro, swhiteho, chris.mason, axboe,
tytso, sbranden, petero2, linux-kernel, Tejun Heo, akpm
On Wed, Nov 03 2010 at 12:10pm -0400,
Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 01, 2010 at 05:15:28PM +0100, Tejun Heo wrote:
> > * blkdev_get() is extended to include exclusive access management.
> > @holder argument is added and, if is @FMODE_EXCL specified, it will
> > gain exclusive access atomically w.r.t. other exclusive accesses.
> >
> > * blkdev_put() is similarly extended. It now takes @mode argument and
> > if @FMODE_EXCL is set, it releases an exclusive access. Also, when
> > the last exclusive claim is released, the holder/slave symlinks are
> > removed automatically.
>
> Could we get rid of FMODE_EXCL and just make a non-NULL holder field
> mean to open it exlusively (and pass a holder to the blkdev_put to
> release it)?
I agree that the need for a FMODE_EXCL flag is awkward. Christoph's
proposed change should clean things up nicely.
Other than that, for the DM bits:
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-03 16:10 ` Christoph Hellwig
2010-11-04 15:04 ` Mike Snitzer
@ 2010-11-09 10:34 ` Tejun Heo
2010-11-09 10:36 ` Christoph Hellwig
1 sibling, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-09 10:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, linux-kernel, petero2, schwidefsky, heiko.carstens, jack,
akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
Hello,
On 11/03/2010 05:10 PM, Christoph Hellwig wrote:
> On Mon, Nov 01, 2010 at 05:15:28PM +0100, Tejun Heo wrote:
>> * blkdev_get() is extended to include exclusive access management.
>> @holder argument is added and, if is @FMODE_EXCL specified, it will
>> gain exclusive access atomically w.r.t. other exclusive accesses.
>>
>> * blkdev_put() is similarly extended. It now takes @mode argument and
>> if @FMODE_EXCL is set, it releases an exclusive access. Also, when
>> the last exclusive claim is released, the holder/slave symlinks are
>> removed automatically.
>
> Could we get rid of FMODE_EXCL and just make a non-NULL holder field
> mean to open it exlusively (and pass a holder to the blkdev_put to
> release it)?
Yeah, I agree it's a bit awkward. I'd really like to force one way or
the other tho. ie. if non-NULL holder is gonna be required, I'll add
WARN_ON_ONCE(mode & FMODE_EXCL). There are several issues to
consider.
* As Jan suggested, @mode in blkdev_put() isn't too useful. I decided
to keep it and use FMODE_EXCL for exclusive releases as that it is
at least useful for something. If we're gonna add @holder to
blkdev_put(), it would make more sense to drop @mode. It's not like
there's a way to enforce restrictions according to open @mode during
device access if there are mixed r/w opens.
* Some users don't keep @holder easily accessible until blkdev_put()
is called, so the conversion will take a bit more effort. No big
deal in itself.
* What if @holder on blkdev_put() mismatches the current holder?
Probably WARN_ON_ONCE() is the only recourse. At that point, it's a
bit silly to have to keep @holder around till blkdev_put(). Holders
and opners counting already provide meaningful warning mechanism
against spurious or missing exclusive releases. Maybe we can have
blkdev_put() and blkdev_put_exclusive()? Or make it take boolean
@excl?
So, after the above points, I decided to keep @mode. It is a bit
awkward but the other way didn't seem too hip either. Any better
ideas?
>> * bd_link_disk_holder() remains the same but bd_unlink_disk_holder()
>> is no longer necessary and removed.
>
> That's a rather asymetric interface. What about having
> blkdev_get_stacked that require a gendisk as holder and set up the
> links underneath?
That will make the number of functions multiplied by two -
blkdev_get_by_path_stacked() and blkdev_get_by_dev_stacked(). The
symlinking for stacked drivers is an oddball feature which is and will
be only used by md and dm. So, I think it's better to keep it
separate and oddball.
>> open_bdev_exclusive() and open_by_devnum() can use further cleanup -
>> rename to blkdev_get_by_path() and blkdev_get_by_devt() and drop
>> special features. Well, let's leave them for another day.
>
> s/blkdev_get_by_devt/blkdev_get_by_dev/
>
> And yes, that rename is a good idea and should go in ASAP after this
> patch.
Alright, will do it.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] block: make blkdev_get/put() handle exclusive access
2010-11-09 10:34 ` Tejun Heo
@ 2010-11-09 10:36 ` Christoph Hellwig
0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2010-11-09 10:36 UTC (permalink / raw)
To: Tejun Heo
Cc: Christoph Hellwig, axboe, linux-kernel, petero2, schwidefsky,
heiko.carstens, jack, akpm, adilger.kernel, tytso, mfasheh,
joel.becker, aelder, dm-devel, drbd-dev, neilb, leochen, sbranden,
chris.mason, swhiteho, shaggy, joern, konishi.ryusuke,
reiserfs-devel, viro
On Tue, Nov 09, 2010 at 11:34:06AM +0100, Tejun Heo wrote:
> So, after the above points, I decided to keep @mode. It is a bit
> awkward but the other way didn't seem too hip either. Any better
> ideas?
Let's keep it for now and revisit it later.
> That will make the number of functions multiplied by two -
> blkdev_get_by_path_stacked() and blkdev_get_by_dev_stacked(). The
> symlinking for stacked drivers is an oddball feature which is and will
> be only used by md and dm. So, I think it's better to keep it
> separate and oddball.
Oh well. I still don't like it, but let keeps it simple to make
progress for now.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/5] block: reorganize claim/release implementation
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (3 preceding siblings ...)
2010-11-01 16:15 ` [PATCH 4/5] block: make blkdev_get/put() handle exclusive access Tejun Heo
@ 2010-11-01 16:15 ` Tejun Heo
2010-11-01 19:49 ` [PATCHSET] block: clean up bdev claim/release handling Neil Brown
` (6 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-01 16:15 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Cc: Tejun Heo
With claim/release rolled into blkdev_get/put(), there's no reason to
keep bd_abort/finish_claim(), __bd_claim() and bd_release() as
separate functions. It only makes the code difficult to follow.
Collapse them into blkdev_get/put(). This will ease future changes
around claim/release.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/block_dev.c | 127 +++++++++++++++++++++-----------------------------------
1 files changed, 48 insertions(+), 79 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index fc48912..269bfbb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -772,79 +772,6 @@ static struct block_device *bd_start_claiming(struct block_device *bdev,
}
}
-/* releases bdev_lock */
-static void __bd_abort_claiming(struct block_device *whole, void *holder)
-{
- BUG_ON(whole->bd_claiming != holder);
- whole->bd_claiming = NULL;
- wake_up_bit(&whole->bd_claiming, 0);
-
- spin_unlock(&bdev_lock);
- bdput(whole);
-}
-
-/**
- * bd_abort_claiming - abort claiming a block device
- * @whole: whole block device returned by bd_start_claiming()
- * @holder: holder trying to claim @bdev
- *
- * Abort a claiming block started by bd_start_claiming(). Note that
- * @whole is not the block device to be claimed but the whole device
- * returned by bd_start_claiming().
- *
- * CONTEXT:
- * Grabs and releases bdev_lock.
- */
-static void bd_abort_claiming(struct block_device *whole, void *holder)
-{
- spin_lock(&bdev_lock);
- __bd_abort_claiming(whole, holder); /* releases bdev_lock */
-}
-
-/* increment holders when we have a legitimate claim. requires bdev_lock */
-static void __bd_claim(struct block_device *bdev, struct block_device *whole,
- void *holder)
-{
- /* note that for a whole device bd_holders
- * will be incremented twice, and bd_holder will
- * be set to bd_may_claim before being set to holder
- */
- whole->bd_holders++;
- whole->bd_holder = bd_may_claim;
- bdev->bd_holders++;
- bdev->bd_holder = holder;
-}
-
-/**
- * bd_finish_claiming - finish claiming a block device
- * @bdev: block device of interest (passed to bd_start_claiming())
- * @whole: whole block device returned by bd_start_claiming()
- * @holder: holder trying to claim @bdev
- *
- * Finish a claiming block started by bd_start_claiming().
- *
- * CONTEXT:
- * Grabs and releases bdev_lock.
- */
-static void bd_finish_claiming(struct block_device *bdev,
- struct block_device *whole, void *holder)
-{
- spin_lock(&bdev_lock);
- BUG_ON(!bd_may_claim(bdev, whole, holder));
- __bd_claim(bdev, whole, holder);
- __bd_abort_claiming(whole, holder); /* not actually an abort */
-}
-
-static void bd_release(struct block_device *bdev)
-{
- spin_lock(&bdev_lock);
- if (!--bdev->bd_contains->bd_holders)
- bdev->bd_contains->bd_holder = NULL;
- if (!--bdev->bd_holders)
- bdev->bd_holder = NULL;
- spin_unlock(&bdev_lock);
-}
-
#ifdef CONFIG_SYSFS
static int add_symlink(struct kobject *from, struct kobject *to)
{
@@ -1223,10 +1150,30 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
res = __blkdev_get(bdev, mode, 0);
if (whole) {
- if (res == 0)
- bd_finish_claiming(bdev, whole, holder);
- else
- bd_abort_claiming(whole, holder);
+ /* finish claiming */
+ spin_lock(&bdev_lock);
+
+ if (res == 0) {
+ BUG_ON(!bd_may_claim(bdev, whole, holder));
+ /*
+ * Note that for a whole device bd_holders
+ * will be incremented twice, and bd_holder
+ * will be set to bd_may_claim before being
+ * set to holder
+ */
+ whole->bd_holders++;
+ whole->bd_holder = bd_may_claim;
+ bdev->bd_holders++;
+ bdev->bd_holder = holder;
+ }
+
+ /* tell others that we're done */
+ BUG_ON(whole->bd_claiming != holder);
+ whole->bd_claiming = NULL;
+ wake_up_bit(&whole->bd_claiming, 0);
+
+ spin_unlock(&bdev_lock);
+ bdput(whole);
}
return res;
@@ -1272,6 +1219,7 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_part_count--;
if (!--bdev->bd_openers) {
+ WARN_ON_ONCE(bdev->bd_holders);
sync_blockdev(bdev);
kill_bdev(bdev);
}
@@ -1303,10 +1251,31 @@ static int __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
int blkdev_put(struct block_device *bdev, fmode_t mode)
{
if (mode & FMODE_EXCL) {
+ bool bdev_free;
+
+ /*
+ * Release a claim on the device. The holder fields
+ * are protected with bdev_lock. bd_mutex is to
+ * synchronize disk_holder unlinking.
+ */
mutex_lock(&bdev->bd_mutex);
- bd_release(bdev);
- if (!bdev->bd_holders)
+ spin_lock(&bdev_lock);
+
+ WARN_ON_ONCE(--bdev->bd_holders < 0);
+ WARN_ON_ONCE(--bdev->bd_contains->bd_holders < 0);
+
+ /* bd_contains might point to self, check in a separate step */
+ if ((bdev_free = !bdev->bd_holders))
+ bdev->bd_holder = NULL;
+ if (!bdev->bd_contains->bd_holders)
+ bdev->bd_contains->bd_holder = NULL;
+
+ spin_unlock(&bdev_lock);
+
+ /* if this was the last claim, holder link should go too */
+ if (bdev_free)
bd_unlink_disk_holder(bdev);
+
mutex_unlock(&bdev->bd_mutex);
}
return __blkdev_put(bdev, mode, 0);
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCHSET] block: clean up bdev claim/release handling
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (4 preceding siblings ...)
2010-11-01 16:15 ` [PATCH 5/5] block: reorganize claim/release implementation Tejun Heo
@ 2010-11-01 19:49 ` Neil Brown
2010-11-03 15:53 ` [Drbd-dev] " Philipp Reisner
` (5 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Neil Brown @ 2010-11-01 19:49 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, leochen, sbranden, chris.mason, swhiteho,
shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
On Mon, 1 Nov 2010 17:15:24 +0100
Tejun Heo <tj@kernel.org> wrote:
> bdev exclusive access handling is somewhat clumsy. The primary
> interface is bd_claim() and bd_release() but there also are
> open_bdev_exclusive(), close_bdev_exclusive() and bd_claim_by_disk()
> and bd_release_from_disk().
>
> Gaining exclusive access as a separate step from open itself also has
> the inherent problem that the block layer doesn't know whether the
> current open attempt is for an exclusive access or not. The action of
> open itself may affect the existing exclusive access but there's no
> way to prevent it.
>
> This patchset simplifies the claim/release handling by folding it into
> blkdev_get/put() and making others simple wrappers around them. This
> simplifies code and removes strange corner cases like the
> non-atomicity of exclusive open described above.
He Tejun,
This looks like a really nice clean-up! Thanks.
Definitely
Acked-by: NeilBrown <neilb@suse.de>
for the md parts.
Thanks,
NeilBrown
>
> There are five patches in this series.
>
> 0001-mtd-fix-bdev-exclusive-open-bugs-in-block2mtd-add_de.patch
> 0002-btrfs-close_bdev_exclusive-should-use-the-same-flags.patch
> 0003-block-simplify-holder-symlink-handling.patch
> 0004-block-make-blkdev_get-put-handle-exclusive-access.patch
> 0005-block-reorganize-claim-release-implementation.patch
>
> 0001-0002 are fixes regarding exclusive bdev opens. 0003 simplifies
> holder/slave symlink handling. 0004 folds bdev claim/release into
> blkdev_get/put(). 0005 reorganizes implementation a bit.
>
> Cleaning up claim/release handling enables implementation of in-kernel
> disk event polling which be posted soon. Although this patchset
> touches various files including drivers and filesystems, the changes
> there are mostly trivial and I think it would be best to route these
> through the block tree.
>
> This patchset is on top of 2.6.37-rc1 and available in the following
> git tree.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git
> cleanup-bd_claim
>
> This patchset reduces about 400 lines of code.
>
> Thanks.
>
> block/ioctl.c | 5
> drivers/block/drbd/drbd_int.h | 2
> drivers/block/drbd/drbd_main.c | 7
> drivers/block/drbd/drbd_nl.c | 103 ++-----
> drivers/block/pktcdvd.c | 22 -
> drivers/char/raw.c | 14
> drivers/md/dm-table.c | 20 -
> drivers/md/md.c | 16 -
> drivers/mtd/devices/block2mtd.c | 10
> drivers/s390/block/dasd_genhd.c | 2
> fs/block_dev.c | 576
> +++++++++-------------------------------
> fs/btrfs/volumes.c | 14
> fs/ext3/super.c | 12
> fs/ext4/super.c | 12
> fs/gfs2/ops_fstype.c | 4
> fs/jfs/jfs_logmgr.c | 17 -
> fs/logfs/dev_bdev.c | 4
> fs/nilfs2/super.c | 4
> fs/ocfs2/cluster/heartbeat.c | 2
> fs/partitions/check.c | 2
> fs/reiserfs/journal.c | 17 -
> fs/super.c | 14
> fs/xfs/linux-2.6/xfs_super.c | 2
> include/linux/fs.h | 22 -
> kernel/power/swap.c | 5
> mm/swapfile.c | 7 26 files changed, 260
> insertions(+), 655 deletions(-)
>
> --
> tejun
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Drbd-dev] [PATCHSET] block: clean up bdev claim/release handling
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (5 preceding siblings ...)
2010-11-01 19:49 ` [PATCHSET] block: clean up bdev claim/release handling Neil Brown
@ 2010-11-03 15:53 ` Philipp Reisner
[not found] ` <1288628129-12811-4-git-send-email-tj@kernel.org>
` (4 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Philipp Reisner @ 2010-11-03 15:53 UTC (permalink / raw)
To: drbd-dev
Cc: Tejun Heo, axboe, hch, linux-kernel, petero2, schwidefsky,
heiko.carstens, jack, akpm, adilger.kernel, tytso, mfasheh,
joel.becker, aelder, dm-devel, neilb, leochen, sbranden,
chris.mason, swhiteho, shaggy, joern, konishi.ryusuke,
reiserfs-devel, viro
Am Montag, 1. November 2010, um 17:15:24 schrieb Tejun Heo:
> bdev exclusive access handling is somewhat clumsy. The primary
> interface is bd_claim() and bd_release() but there also are
> open_bdev_exclusive(), close_bdev_exclusive() and bd_claim_by_disk()
> and bd_release_from_disk().
[...]
> drivers/block/drbd/drbd_int.h | 2
> drivers/block/drbd/drbd_main.c | 7
> drivers/block/drbd/drbd_nl.c | 103 ++-----
[...]
Hi Tejun,
Acked-by: Philipp Reisner <philipp.reisner@linbit.com>
...for the DRBD part of the patch.
Best,
Phil
^ permalink raw reply [flat|nested] 27+ messages in thread[parent not found: <1288628129-12811-4-git-send-email-tj@kernel.org>]
* Re: [PATCH 3/5] block: simplify holder symlink handling
[not found] ` <1288628129-12811-4-git-send-email-tj@kernel.org>
@ 2010-11-04 15:06 ` Mike Snitzer
0 siblings, 0 replies; 27+ messages in thread
From: Mike Snitzer @ 2010-11-04 15:06 UTC (permalink / raw)
To: device-mapper development
Cc: jack, leochen, heiko.carstens, adilger.kernel, konishi.ryusuke,
shaggy, drbd-dev, joel.becker, hch, aelder, mfasheh, joern,
reiserfs-devel, Tejun Heo, viro, swhiteho, chris.mason, axboe,
tytso, sbranden, petero2, linux-kernel, schwidefsky, akpm
On Mon, Nov 01 2010 at 12:15pm -0400,
Tejun Heo <tj@kernel.org> wrote:
> Code to manage symlinks in /sys/block/*/{holders|slaves} are overly
> complex with multiple holder considerations, redundant extra
> references to all involved kobjects, unused generic kobject holder
> support and unnecessary mixup with bd_claim/release functionalities.
>
> Strip it down to what's necessary (single gendisk holder) and make it
> use a separate interface. This is a step for cleaning up
> bd_claim/release. This patch makes dm-table slightly more complex but
> it will be simplified again with further changes.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Neil Brown <neilb@suse.de>
> Cc: dm-devel@redhat.com
Acked-by: Mike Snitzer <snitzer@redhat.com>
> ---
> drivers/md/dm-table.c | 23 +++-
> drivers/md/md.c | 4 +-
> fs/block_dev.c | 322 +++++++------------------------------------------
> include/linux/fs.h | 16 ++-
> 4 files changed, 74 insertions(+), 291 deletions(-)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 90267f8..2c876ff 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -328,12 +328,22 @@ static int open_dev(struct dm_dev_internal *d, dev_t dev,
> bdev = open_by_devnum(dev, d->dm_dev.mode);
> if (IS_ERR(bdev))
> return PTR_ERR(bdev);
> - r = bd_claim_by_disk(bdev, _claim_ptr, dm_disk(md));
> - if (r)
> +
> + r = bd_claim(bdev, _claim_ptr);
> + if (r) {
> blkdev_put(bdev, d->dm_dev.mode);
> - else
> - d->dm_dev.bdev = bdev;
> - return r;
> + return r;
> + }
> +
> + r = bd_link_disk_holder(bdev, dm_disk(md));
> + if (r) {
> + bd_release(bdev);
> + blkdev_put(bdev, d->dm_dev.mode);
> + return r;
> + }
> +
> + d->dm_dev.bdev = bdev;
> + return 0;
> }
>
> /*
> @@ -344,7 +354,8 @@ static void close_dev(struct dm_dev_internal *d, struct mapped_device *md)
> if (!d->dm_dev.bdev)
> return;
>
> - bd_release_from_disk(d->dm_dev.bdev, dm_disk(md));
> + bd_unlink_disk_holder(d->dm_dev.bdev);
> + bd_release(d->dm_dev.bdev);
> blkdev_put(d->dm_dev.bdev, d->dm_dev.mode);
> d->dm_dev.bdev = NULL;
> }
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/5] block: check bdev_read_only() from blkdev_get()
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (7 preceding siblings ...)
[not found] ` <1288628129-12811-4-git-send-email-tj@kernel.org>
@ 2010-11-11 17:10 ` Tejun Heo
2010-11-11 17:11 ` [PATCH 7/5] block: clean up blkdev_get() wrappers and their users Tejun Heo
` (2 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-11 17:10 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
bdev read-only status can be queried using bdev_read_only() and may
change while the device is being opened. Enforce it by checking it
from blkdev_get() after open succeeds.
This makes bdev_read_only() check in open_bdev_exclusive() and
fsg_lun_open() unnecessary. Drop them.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: linux-usb@vger.kernel.org
---
drivers/usb/gadget/storage_common.c | 7 ++-----
fs/block_dev.c | 11 ++++++-----
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 3b513ba..b015561 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -543,7 +543,7 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
ro = curlun->initially_ro;
if (!ro) {
filp = filp_open(filename, O_RDWR | O_LARGEFILE, 0);
- if (-EROFS == PTR_ERR(filp))
+ if (PTR_ERR(filp) == -EROFS || PTR_ERR(filp) == -EACCES)
ro = 1;
}
if (ro)
@@ -558,10 +558,7 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename)
if (filp->f_path.dentry)
inode = filp->f_path.dentry->d_inode;
- if (inode && S_ISBLK(inode->i_mode)) {
- if (bdev_read_only(inode->i_bdev))
- ro = 1;
- } else if (!inode || !S_ISREG(inode->i_mode)) {
+ if (!inode || (!S_ISREG(inode->i_mode) && !S_ISBLK(inode->i_mode))) {
LINFO(curlun, "invalid file type: %s\n", filename);
goto out;
}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 269bfbb..606a525 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1149,6 +1149,12 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
res = __blkdev_get(bdev, mode, 0);
+ /* __blkdev_get() may alter read only status, check it afterwards */
+ if (!res && (mode & FMODE_WRITE) && bdev_read_only(bdev)) {
+ __blkdev_put(bdev, mode, 0);
+ res = -EACCES;
+ }
+
if (whole) {
/* finish claiming */
spin_lock(&bdev_lock);
@@ -1453,11 +1459,6 @@ struct block_device *open_bdev_exclusive(const char *path, fmode_t mode, void *h
if (error)
return ERR_PTR(error);
- if ((mode & FMODE_WRITE) && bdev_read_only(bdev)) {
- blkdev_put(bdev, mode);
- return ERR_PTR(-EACCES);
- }
-
return bdev;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 7/5] block: clean up blkdev_get() wrappers and their users
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (8 preceding siblings ...)
2010-11-11 17:10 ` [PATCH 6/5] block: check bdev_read_only() from blkdev_get() Tejun Heo
@ 2010-11-11 17:11 ` Tejun Heo
2010-11-11 19:19 ` Steven Whitehouse
2010-11-11 17:14 ` [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
[not found] ` <1288628129-12811-2-git-send-email-tj@kernel.org>
11 siblings, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-11 17:11 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
After recent blkdev_get() modifications, open_by_devnum() and
open_bdev_exclusive() are simple wrappers around blkdev_get().
Replace them with blkdev_get_by_dev() and blkdev_get_by_path().
blkdev_get_by_dev() is identical to open_by_devnum().
blkdev_get_by_path() is slightly different in that it doesn't
automatically add %FMODE_EXCL to @mode.
All users are converted. Most conversions are mechanical and don't
introduce any behavior difference. There are several exceptions.
* btrfs now sets FMODE_EXCL in btrfs_device->mode, so there's no
reason to OR it explicitly on blkdev_put().
* gfs2, nilfs2 and the generic mount_bdev() now set FMODE_EXCL in
sb->s_mode.
* With the above changes, sb->s_mode now always should contain
FMODE_EXCL. WARN_ON_ONCE() added to kill_block_super() to detect
errors.
The new blkdev_get_*() functions are with proper docbook comments.
While at it, add function description to blkdev_get() too.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Neil Brown <neilb@suse.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Joern Engel <joern@lazybastard.org>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Jan Kara <jack@suse.cz>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: KONISHI Ryusuke <konishi.ryusuke@lab.ntt.co.jp>
Cc: reiserfs-devel@vger.kernel.org
Cc: xfs-masters@oss.sgi.com
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
---
drivers/block/drbd/drbd_nl.c | 12 ++--
drivers/md/dm-table.c | 2 +-
drivers/md/md.c | 4 +-
drivers/mtd/devices/block2mtd.c | 5 +-
fs/block_dev.c | 139 ++++++++++++++++++++++++++-------------
fs/btrfs/volumes.c | 24 ++++---
fs/btrfs/volumes.h | 2 +-
fs/ext3/super.c | 2 +-
fs/ext4/super.c | 2 +-
fs/gfs2/ops_fstype.c | 8 +-
fs/jfs/jfs_logmgr.c | 4 +-
fs/logfs/dev_bdev.c | 3 +-
fs/nilfs2/super.c | 8 +-
fs/reiserfs/journal.c | 6 +-
fs/super.c | 9 ++-
fs/xfs/linux-2.6/xfs_super.c | 3 +-
include/linux/fs.h | 7 +-
kernel/power/swap.c | 4 +-
18 files changed, 150 insertions(+), 94 deletions(-)
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index fd03460..650e43b 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -902,8 +902,8 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
}
}
- bdev = open_bdev_exclusive(nbc->dc.backing_dev,
- FMODE_READ | FMODE_WRITE, mdev);
+ bdev = blkdev_get_by_path(nbc->dc.backing_dev,
+ FMODE_READ | FMODE_WRITE | FMODE_EXCL, mdev);
if (IS_ERR(bdev)) {
dev_err(DEV, "open(\"%s\") failed with %ld\n", nbc->dc.backing_dev,
PTR_ERR(bdev));
@@ -920,10 +920,10 @@ static int drbd_nl_disk_conf(struct drbd_conf *mdev, struct drbd_nl_cfg_req *nlp
* should check it for you already; but if you don't, or
* someone fooled it, we need to double check here)
*/
- bdev = open_bdev_exclusive(nbc->dc.meta_dev,
- FMODE_READ | FMODE_WRITE,
- (nbc->dc.meta_dev_idx < 0) ?
- (void *)mdev : (void *)drbd_m_holder);
+ bdev = blkdev_get_by_path(nbc->dc.meta_dev,
+ FMODE_READ | FMODE_WRITE | FMODE_EXCL,
+ (nbc->dc.meta_dev_idx < 0) ?
+ (void *)mdev : (void *)drbd_m_holder);
if (IS_ERR(bdev)) {
dev_err(DEV, "open(\"%s\") failed with %ld\n", nbc->dc.meta_dev,
PTR_ERR(bdev));
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 9e88ca0..67150c3 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -325,7 +325,7 @@ static int open_dev(struct dm_dev_internal *d, dev_t dev,
BUG_ON(d->dm_dev.bdev);
- bdev = open_by_devnum(dev, d->dm_dev.mode | FMODE_EXCL, _claim_ptr);
+ bdev = blkdev_get_by_dev(dev, d->dm_dev.mode | FMODE_EXCL, _claim_ptr);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 6af951f..5aaa6bf 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1934,8 +1934,8 @@ static int lock_rdev(mdk_rdev_t *rdev, dev_t dev, int shared)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
- shared ? (mdk_rdev_t *)lock_rdev : rdev);
+ bdev = blkdev_get_by_dev(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ shared ? (mdk_rdev_t *)lock_rdev : rdev);
if (IS_ERR(bdev)) {
printk(KERN_ERR "md: could not open %s.\n",
__bdevname(dev, b));
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 84a4045..029cede 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -246,7 +246,8 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
return NULL;
/* Get a handle on the device */
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, dev);
+ bdev = blkdev_get_by_path(devname,
+ FMODE_READ|FMODE_WRITE|FMODE_EXCL, dev);
#ifndef MODULE
if (IS_ERR(bdev)) {
@@ -255,7 +256,7 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
dev_t devt = name_to_dev_t(devname);
if (devt)
- bdev = open_by_devnum(devt,
+ bdev = blkdev_get_by_dev(devt,
FMODE_READ|FMODE_WRITE|FMODE_EXCL, dev);
}
#endif
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 606a525..c1c1b8c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -854,24 +854,6 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev)
{ }
#endif
-/*
- * Tries to open block device by device number. Use it ONLY if you
- * really do not have anything better - i.e. when you are behind a
- * truly sucky interface and all you are given is a device number. _Never_
- * to be used for internal purposes. If you ever need it - reconsider
- * your API.
- */
-struct block_device *open_by_devnum(dev_t dev, fmode_t mode, void *holder)
-{
- struct block_device *bdev = bdget(dev);
- int err = -ENOMEM;
- if (bdev)
- err = blkdev_get(bdev, mode, holder);
- return err ? ERR_PTR(err) : bdev;
-}
-
-EXPORT_SYMBOL(open_by_devnum);
-
/**
* flush_disk - invalidates all buffer-cache entries on a disk
*
@@ -1132,6 +1114,25 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
return ret;
}
+/**
+ * blkdev_get - open a block device
+ * @bdev: block_device to open
+ * @mode: FMODE_* mask
+ * @holder: exclusive holder identifier
+ *
+ * Open @bdev with @mode. If @mode includes %FMODE_EXCL, @bdev is
+ * open with exclusive access. Specifying %FMODE_EXCL with %NULL
+ * @holder is invalid. Exclusive opens may nest for the same @holder.
+ *
+ * On success, the reference count of @bdev is unchanged. On failure,
+ * @bdev is put.
+ *
+ * CONTEXT:
+ * Might sleep.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
{
struct block_device *whole = NULL;
@@ -1186,6 +1187,80 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
}
EXPORT_SYMBOL(blkdev_get);
+/**
+ * blkdev_get_by_path - open a block device by name
+ * @path: path to the block device to open
+ * @mode: FMODE_* mask
+ * @holder: exclusive holder identifier
+ *
+ * Open the blockdevice described by the device file at @path. @mode
+ * and @holder are identical to blkdev_get().
+ *
+ * On success, the returned block_device has reference count of one.
+ *
+ * CONTEXT:
+ * Might sleep.
+ *
+ * RETURNS:
+ * Pointer to block_device on success, ERR_PTR(-errno) on failure.
+ */
+struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
+ void *holder)
+{
+ struct block_device *bdev;
+ int err;
+
+ bdev = lookup_bdev(path);
+ if (IS_ERR(bdev))
+ return bdev;
+
+ err = blkdev_get(bdev, mode, holder);
+ if (err)
+ return ERR_PTR(err);
+
+ return bdev;
+}
+EXPORT_SYMBOL(blkdev_get_by_path);
+
+/**
+ * blkdev_get_by_dev - open a block device by device number
+ * @dev: device number of block device to open
+ * @mode: FMODE_* mask
+ * @holder: exclusive holder identifier
+ *
+ * Open the blockdevice described by device number @dev. @mode and
+ * @holder are identical to blkdev_get().
+ *
+ * Use it ONLY if you really do not have anything better - i.e. when
+ * you are behind a truly sucky interface and all you are given is a
+ * device number. _Never_ to be used for internal purposes. If you
+ * ever need it - reconsider your API.
+ *
+ * On success, the returned block_device has reference count of one.
+ *
+ * CONTEXT:
+ * Might sleep.
+ *
+ * RETURNS:
+ * Pointer to block_device on success, ERR_PTR(-errno) on failure.
+ */
+struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
+{
+ struct block_device *bdev;
+ int err;
+
+ bdev = bdget(dev);
+ if (!bdev)
+ return ERR_PTR(-ENOMEM);
+
+ err = blkdev_get(bdev, mode, holder);
+ if (err)
+ return ERR_PTR(err);
+
+ return bdev;
+}
+EXPORT_SYMBOL(blkdev_get_by_dev);
+
static int blkdev_open(struct inode * inode, struct file * filp)
{
struct block_device *bdev;
@@ -1436,34 +1511,6 @@ fail:
}
EXPORT_SYMBOL(lookup_bdev);
-/**
- * open_bdev_exclusive - open a block device by name and set it up for use
- *
- * @path: special file representing the block device
- * @mode: FMODE_... combination to pass be used
- * @holder: owner for exclusion
- *
- * Open the blockdevice described by the special file at @path, claim it
- * for the @holder.
- */
-struct block_device *open_bdev_exclusive(const char *path, fmode_t mode, void *holder)
-{
- struct block_device *bdev;
- int error;
-
- bdev = lookup_bdev(path);
- if (IS_ERR(bdev))
- return bdev;
-
- error = blkdev_get(bdev, mode | FMODE_EXCL, holder);
- if (error)
- return ERR_PTR(error);
-
- return bdev;
-}
-
-EXPORT_SYMBOL(open_bdev_exclusive);
-
int __invalidate_device(struct block_device *bdev)
{
struct super_block *sb = get_super(bdev);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f1b729d..95324e9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -489,7 +489,7 @@ again:
continue;
if (device->bdev) {
- blkdev_put(device->bdev, device->mode | FMODE_EXCL);
+ blkdev_put(device->bdev, device->mode);
device->bdev = NULL;
fs_devices->open_devices--;
}
@@ -523,7 +523,7 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
list_for_each_entry(device, &fs_devices->devices, dev_list) {
if (device->bdev) {
- blkdev_put(device->bdev, device->mode | FMODE_EXCL);
+ blkdev_put(device->bdev, device->mode);
fs_devices->open_devices--;
}
if (device->writeable) {
@@ -580,13 +580,15 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
int seeding = 1;
int ret = 0;
+ flags |= FMODE_EXCL;
+
list_for_each_entry(device, head, dev_list) {
if (device->bdev)
continue;
if (!device->name)
continue;
- bdev = open_bdev_exclusive(device->name, flags, holder);
+ bdev = blkdev_get_by_path(device->name, flags, holder);
if (IS_ERR(bdev)) {
printk(KERN_INFO "open %s failed\n", device->name);
goto error;
@@ -638,7 +640,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
error_brelse:
brelse(bh);
error_close:
- blkdev_put(bdev, flags | FMODE_EXCL);
+ blkdev_put(bdev, flags);
error:
continue;
}
@@ -684,7 +686,8 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
mutex_lock(&uuid_mutex);
- bdev = open_bdev_exclusive(path, flags, holder);
+ flags |= FMODE_EXCL;
+ bdev = blkdev_get_by_path(path, flags, holder);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
@@ -716,7 +719,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
brelse(bh);
error_close:
- blkdev_put(bdev, flags | FMODE_EXCL);
+ blkdev_put(bdev, flags);
error:
mutex_unlock(&uuid_mutex);
return ret;
@@ -1179,8 +1182,8 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
goto out;
}
} else {
- bdev = open_bdev_exclusive(device_path, FMODE_READ,
- root->fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, FMODE_READ | FMODE_EXCL,
+ root->fs_info->bdev_holder);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
goto out;
@@ -1244,7 +1247,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
root->fs_info->fs_devices->latest_bdev = next_device->bdev;
if (device->bdev) {
- blkdev_put(device->bdev, device->mode | FMODE_EXCL);
+ blkdev_put(device->bdev, device->mode);
device->bdev = NULL;
device->fs_devices->open_devices--;
}
@@ -1439,7 +1442,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
if ((sb->s_flags & MS_RDONLY) && !root->fs_info->fs_devices->seeding)
return -EINVAL;
- bdev = open_bdev_exclusive(device_path, 0, root->fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, FMODE_EXCL,
+ root->fs_info->bdev_holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 2b638b6..856e757 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -49,7 +49,7 @@ struct btrfs_device {
struct block_device *bdev;
- /* the mode sent to open_bdev_exclusive */
+ /* the mode sent to blkdev_get */
fmode_t mode;
char *name;
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 23e7513..123720b 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -347,7 +347,7 @@ static struct block_device *ext3_blkdev_get(dev_t dev, struct super_block *sb)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
+ bdev = blkdev_get_by_dev(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
if (IS_ERR(bdev))
goto fail;
return bdev;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 5dd0b3e..bd63e69 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -647,7 +647,7 @@ static struct block_device *ext4_blkdev_get(dev_t dev, struct super_block *sb)
struct block_device *bdev;
char b[BDEVNAME_SIZE];
- bdev = open_by_devnum(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
+ bdev = blkdev_get_by_dev(dev, FMODE_READ|FMODE_WRITE|FMODE_EXCL, sb);
if (IS_ERR(bdev))
goto fail;
return bdev;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c1f0763..bc56ccf 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1268,7 +1268,7 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
{
struct block_device *bdev;
struct super_block *s;
- fmode_t mode = FMODE_READ;
+ fmode_t mode = FMODE_READ | FMODE_EXCL;
int error;
struct gfs2_args args;
struct gfs2_sbd *sdp;
@@ -1276,7 +1276,7 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
if (!(flags & MS_RDONLY))
mode |= FMODE_WRITE;
- bdev = open_bdev_exclusive(dev_name, mode, fs_type);
+ bdev = blkdev_get_by_path(dev_name, mode, fs_type);
if (IS_ERR(bdev))
return ERR_CAST(bdev);
@@ -1298,7 +1298,7 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
goto error_bdev;
if (s->s_root)
- blkdev_put(bdev, mode | FMODE_EXCL);
+ blkdev_put(bdev, mode);
memset(&args, 0, sizeof(args));
args.ar_quota = GFS2_QUOTA_DEFAULT;
@@ -1342,7 +1342,7 @@ error_super:
deactivate_locked_super(s);
return ERR_PTR(error);
error_bdev:
- blkdev_put(bdev, mode | FMODE_EXCL);
+ blkdev_put(bdev, mode);
return ERR_PTR(error);
}
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 5a290f2..278e3fb 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1120,8 +1120,8 @@ int lmLogOpen(struct super_block *sb)
* file systems to log may have n-to-1 relationship;
*/
- bdev = open_by_devnum(sbi->logdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
- log);
+ bdev = blkdev_get_by_dev(sbi->logdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ log);
if (IS_ERR(bdev)) {
rc = -PTR_ERR(bdev);
goto free;
diff --git a/fs/logfs/dev_bdev.c b/fs/logfs/dev_bdev.c
index 734b902..723bc5b 100644
--- a/fs/logfs/dev_bdev.c
+++ b/fs/logfs/dev_bdev.c
@@ -325,7 +325,8 @@ int logfs_get_sb_bdev(struct logfs_super *p, struct file_system_type *type,
{
struct block_device *bdev;
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, type);
+ bdev = blkdev_get_by_path(devname, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ type);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 756a679..0030640 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1147,14 +1147,14 @@ nilfs_mount(struct file_system_type *fs_type, int flags,
{
struct nilfs_super_data sd;
struct super_block *s;
- fmode_t mode = FMODE_READ;
+ fmode_t mode = FMODE_READ | FMODE_EXCL;
struct dentry *root_dentry;
int err, s_new = false;
if (!(flags & MS_RDONLY))
mode |= FMODE_WRITE;
- sd.bdev = open_bdev_exclusive(dev_name, mode, fs_type);
+ sd.bdev = blkdev_get_by_path(dev_name, mode, fs_type);
if (IS_ERR(sd.bdev))
return ERR_CAST(sd.bdev);
@@ -1233,7 +1233,7 @@ nilfs_mount(struct file_system_type *fs_type, int flags,
}
if (!s_new)
- blkdev_put(sd.bdev, mode | FMODE_EXCL);
+ blkdev_put(sd.bdev, mode);
return root_dentry;
@@ -1242,7 +1242,7 @@ nilfs_mount(struct file_system_type *fs_type, int flags,
failed:
if (!s_new)
- blkdev_put(sd.bdev, mode | FMODE_EXCL);
+ blkdev_put(sd.bdev, mode);
return ERR_PTR(err);
}
diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index b488136..e2fce51 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2585,7 +2585,8 @@ static int journal_init_dev(struct super_block *super,
if ((!jdev_name || !jdev_name[0])) {
if (jdev == super->s_dev)
blkdev_mode &= ~FMODE_EXCL;
- journal->j_dev_bd = open_by_devnum(jdev, blkdev_mode, journal);
+ journal->j_dev_bd = blkdev_get_by_dev(jdev, blkdev_mode,
+ journal);
journal->j_dev_mode = blkdev_mode;
if (IS_ERR(journal->j_dev_bd)) {
result = PTR_ERR(journal->j_dev_bd);
@@ -2601,8 +2602,7 @@ static int journal_init_dev(struct super_block *super,
}
journal->j_dev_mode = blkdev_mode;
- journal->j_dev_bd = open_bdev_exclusive(jdev_name,
- blkdev_mode, journal);
+ journal->j_dev_bd = blkdev_get_by_path(jdev_name, blkdev_mode, journal);
if (IS_ERR(journal->j_dev_bd)) {
result = PTR_ERR(journal->j_dev_bd);
journal->j_dev_bd = NULL;
diff --git a/fs/super.c b/fs/super.c
index 22374bf..5d9a449 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -766,13 +766,13 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
{
struct block_device *bdev;
struct super_block *s;
- fmode_t mode = FMODE_READ;
+ fmode_t mode = FMODE_READ | FMODE_EXCL;
int error = 0;
if (!(flags & MS_RDONLY))
mode |= FMODE_WRITE;
- bdev = open_bdev_exclusive(dev_name, mode, fs_type);
+ bdev = blkdev_get_by_path(dev_name, mode, fs_type);
if (IS_ERR(bdev))
return ERR_CAST(bdev);
@@ -807,7 +807,7 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
* holding an active reference.
*/
up_write(&s->s_umount);
- blkdev_put(bdev, mode | FMODE_EXCL);
+ blkdev_put(bdev, mode);
down_write(&s->s_umount);
} else {
char b[BDEVNAME_SIZE];
@@ -831,7 +831,7 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
error_s:
error = PTR_ERR(s);
error_bdev:
- blkdev_put(bdev, mode | FMODE_EXCL);
+ blkdev_put(bdev, mode);
error:
return ERR_PTR(error);
}
@@ -862,6 +862,7 @@ void kill_block_super(struct super_block *sb)
bdev->bd_super = NULL;
generic_shutdown_super(sb);
sync_blockdev(bdev);
+ WARN_ON_ONCE(!(mode & FMODE_EXCL));
blkdev_put(bdev, mode | FMODE_EXCL);
}
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index a1a6e5c..9209cd1 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -609,7 +609,8 @@ xfs_blkdev_get(
{
int error = 0;
- *bdevp = open_bdev_exclusive(name, FMODE_READ|FMODE_WRITE, mp);
+ *bdevp = blkdev_get_by_path(name, FMODE_READ|FMODE_WRITE|FMODE_EXCL,
+ mp);
if (IS_ERR(*bdevp)) {
error = PTR_ERR(*bdevp);
printk("XFS: Invalid device [%s], error=%d\n", name, error);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1a033e8..f485015 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2006,8 +2006,6 @@ extern struct block_device *bdgrab(struct block_device *bdev);
extern void bd_set_size(struct block_device *, loff_t size);
extern void bd_forget(struct inode *inode);
extern void bdput(struct block_device *);
-extern struct block_device *open_by_devnum(dev_t dev, fmode_t mode,
- void *holder);
extern void invalidate_bdev(struct block_device *);
extern int sync_blockdev(struct block_device *bdev);
extern struct super_block *freeze_bdev(struct block_device *);
@@ -2039,6 +2037,10 @@ extern int ioctl_by_bdev(struct block_device *, unsigned, unsigned long);
extern int blkdev_ioctl(struct block_device *, fmode_t, unsigned, unsigned long);
extern long compat_blkdev_ioctl(struct file *, unsigned, unsigned long);
extern int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder);
+extern struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
+ void *holder);
+extern struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode,
+ void *holder);
extern int blkdev_put(struct block_device *bdev, fmode_t mode);
#ifdef CONFIG_SYSFS
extern int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
@@ -2083,7 +2085,6 @@ static inline void unregister_chrdev(unsigned int major, const char *name)
extern const char *__bdevname(dev_t, char *buffer);
extern const char *bdevname(struct block_device *bdev, char *buffer);
extern struct block_device *lookup_bdev(const char *);
-extern struct block_device *open_bdev_exclusive(const char *, fmode_t, void *);
extern void blkdev_show(struct seq_file *,off_t);
#else
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 513a77f..b019609 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -907,8 +907,8 @@ int swsusp_check(void)
{
int error;
- hib_resume_bdev = open_by_devnum(swsusp_resume_device,
- FMODE_READ, NULL);
+ hib_resume_bdev = blkdev_get_by_dev(swsusp_resume_device,
+ FMODE_READ, NULL);
if (!IS_ERR(hib_resume_bdev)) {
set_blocksize(hib_resume_bdev, PAGE_SIZE);
clear_page(swsusp_header);
--
1.7.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 7/5] block: clean up blkdev_get() wrappers and their users
2010-11-11 17:11 ` [PATCH 7/5] block: clean up blkdev_get() wrappers and their users Tejun Heo
@ 2010-11-11 19:19 ` Steven Whitehouse
0 siblings, 0 replies; 27+ messages in thread
From: Steven Whitehouse @ 2010-11-11 19:19 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason, shaggy,
joern, konishi.ryusuke, reiserfs-devel, viro, philipp.reisner,
snitzer
Hi,
On Thu, 2010-11-11 at 18:11 +0100, Tejun Heo wrote:
> After recent blkdev_get() modifications, open_by_devnum() and
> open_bdev_exclusive() are simple wrappers around blkdev_get().
> Replace them with blkdev_get_by_dev() and blkdev_get_by_path().
>
> blkdev_get_by_dev() is identical to open_by_devnum().
> blkdev_get_by_path() is slightly different in that it doesn't
> automatically add %FMODE_EXCL to @mode.
>
> All users are converted. Most conversions are mechanical and don't
> introduce any behavior difference. There are several exceptions.
>
> * btrfs now sets FMODE_EXCL in btrfs_device->mode, so there's no
> reason to OR it explicitly on blkdev_put().
>
> * gfs2, nilfs2 and the generic mount_bdev() now set FMODE_EXCL in
> sb->s_mode.
>
> * With the above changes, sb->s_mode now always should contain
> FMODE_EXCL. WARN_ON_ONCE() added to kill_block_super() to detect
> errors.
>
> The new blkdev_get_*() functions are with proper docbook comments.
> While at it, add function description to blkdev_get() too.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Philipp Reisner <philipp.reisner@linbit.com>
> Cc: Neil Brown <neilb@suse.de>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Joern Engel <joern@lazybastard.org>
> Cc: Chris Mason <chris.mason@oracle.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: KONISHI Ryusuke <konishi.ryusuke@lab.ntt.co.jp>
> Cc: reiserfs-devel@vger.kernel.org
> Cc: xfs-masters@oss.sgi.com
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Steve.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCHSET] block: clean up bdev claim/release handling
2010-11-01 16:15 [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
` (9 preceding siblings ...)
2010-11-11 17:11 ` [PATCH 7/5] block: clean up blkdev_get() wrappers and their users Tejun Heo
@ 2010-11-11 17:14 ` Tejun Heo
2010-11-27 16:01 ` Tejun Heo
[not found] ` <1288628129-12811-2-git-send-email-tj@kernel.org>
11 siblings, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-11 17:14 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
Hello,
I just appended two patches to the series. These two are the
blkdev_get_by_*() cleanup patches suggested in the fourth patch which
Christoph liked.
I updated the git tree accordingly and added acked-by's.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cleanup-bd_claim
Jens, if you want the whole series resent, please let me know.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCHSET] block: clean up bdev claim/release handling
2010-11-11 17:14 ` [PATCHSET] block: clean up bdev claim/release handling Tejun Heo
@ 2010-11-27 16:01 ` Tejun Heo
2010-11-27 18:47 ` Jens Axboe
0 siblings, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-27 16:01 UTC (permalink / raw)
To: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilge
On 11/11/2010 06:14 PM, Tejun Heo wrote:
> Hello,
>
> I just appended two patches to the series. These two are the
> blkdev_get_by_*() cleanup patches suggested in the fourth patch which
> Christoph liked.
>
> I updated the git tree accordingly and added acked-by's.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cleanup-bd_claim
>
> Jens, if you want the whole series resent, please let me know.
Ping.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCHSET] block: clean up bdev claim/release handling
2010-11-27 16:01 ` Tejun Heo
@ 2010-11-27 18:47 ` Jens Axboe
0 siblings, 0 replies; 27+ messages in thread
From: Jens Axboe @ 2010-11-27 18:47 UTC (permalink / raw)
To: Tejun Heo
Cc: hch, linux-kernel, petero2, schwidefsky, heiko.carstens, jack,
akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro
On 2010-11-27 17:01, Tejun Heo wrote:
> On 11/11/2010 06:14 PM, Tejun Heo wrote:
>> Hello,
>>
>> I just appended two patches to the series. These two are the
>> blkdev_get_by_*() cleanup patches suggested in the fourth patch which
>> Christoph liked.
>>
>> I updated the git tree accordingly and added acked-by's.
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cleanup-bd_claim
>>
>> Jens, if you want the whole series resent, please let me know.
>
> Ping.
You don't have to resend it, I'll pull it in.
--
Jens Axboe
^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1288628129-12811-2-git-send-email-tj@kernel.org>]
* Re: [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
[not found] ` <1288628129-12811-2-git-send-email-tj@kernel.org>
@ 2010-11-13 10:38 ` Artem Bityutskiy
2010-11-13 10:42 ` Tejun Heo
2010-11-13 10:59 ` [PATCH UPDATED " Tejun Heo
1 sibling, 1 reply; 27+ messages in thread
From: Artem Bityutskiy @ 2010-11-13 10:38 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
Hi,
On Mon, 2010-11-01 at 17:15 +0100, Tejun Heo wrote:
> + if (!IS_ERR(bdev)) {
> + int ret;
> + ret = bd_claim(bdev, dev);
> + if (ret) {
> + blkdev_put(bdev,
> + FMODE_READ | FMODE_WRITE);
Would be a bit cleaner to define ea temporary variable:
fmode_t mode = FMODE_READ | FMODE_WRITE;
Would you to re-send with this little change please?
And ideally, 2 independent patches would be nicer because you fix 2
independent issues.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
2010-11-13 10:38 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Artem Bityutskiy
@ 2010-11-13 10:42 ` Tejun Heo
2010-11-13 11:10 ` Artem Bityutskiy
0 siblings, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-13 10:42 UTC (permalink / raw)
To: dedekind1
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
Hello,
On 11/13/2010 11:38 AM, Artem Bityutskiy wrote:
> On Mon, 2010-11-01 at 17:15 +0100, Tejun Heo wrote:
>> + if (!IS_ERR(bdev)) {
>> + int ret;
>> + ret = bd_claim(bdev, dev);
>> + if (ret) {
>> + blkdev_put(bdev,
>> + FMODE_READ | FMODE_WRITE);
>
> Would be a bit cleaner to define ea temporary variable:
>
> fmode_t mode = FMODE_READ | FMODE_WRITE;
>
> Would you to re-send with this little change please?
Yeap, sure.
> And ideally, 2 independent patches would be nicer because you fix 2
> independent issues.
Hmmm... not really. The patch is small enough and splitting it won't
really buy as any better bisectability. Splitting patches into
logical changes is a good thing but it's no religious dogma. Let's
apply it to the point it actually helps.
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
2010-11-13 10:42 ` Tejun Heo
@ 2010-11-13 11:10 ` Artem Bityutskiy
0 siblings, 0 replies; 27+ messages in thread
From: Artem Bityutskiy @ 2010-11-13 11:10 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
On Sat, 2010-11-13 at 11:42 +0100, Tejun Heo wrote:
> Hmmm... not really. The patch is small enough and splitting it won't
> really buy as any better bisectability. Splitting patches into
> logical changes is a good thing but it's no religious dogma. Let's
> apply it to the point it actually helps.
Well, it is more about if your second fix is buggy, we could revert it
without reverting the first fix. But anyway, this is not a big deal, so
let's forget about this indeed.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH UPDATED 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
[not found] ` <1288628129-12811-2-git-send-email-tj@kernel.org>
2010-11-13 10:38 ` [PATCH 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device() Artem Bityutskiy
@ 2010-11-13 10:59 ` Tejun Heo
2010-11-13 11:14 ` Artem Bityutskiy
1 sibling, 1 reply; 27+ messages in thread
From: Tejun Heo @ 2010-11-13 10:59 UTC (permalink / raw)
To: axboe, dedekind1
Cc: hch, linux-kernel, petero2, schwidefsky, heiko.carstens, jack,
akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
There are two bdev exclusive open bugs.
* open_bdev_exclusive() must not be called with NULL holder. Use dev
as the holder.
* open_by_devnum() doesn't open the bdev exclusively but
block2mtd_free_device() always assumes it. Explicitly claim the
bdev.
The latter is rather clumsy but will be simplified with future
blkdev_get/put() cleanups.
- Updated to use local variable @mode to cache FMODE_* masks as
suggested by Artem Bityutskiy.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <dedekind1@gmail.com>
---
This change will cause two conflicts in the series. I'm not reposting
them now as they're mostly trivial. Git tree is fully updated.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cleanup-bd_claim
Jens, please let me know when/if you want the full series reposted.
Thanks.
drivers/mtd/devices/block2mtd.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
Index: work/drivers/mtd/devices/block2mtd.c
===================================================================
--- work.orig/drivers/mtd/devices/block2mtd.c
+++ work/drivers/mtd/devices/block2mtd.c
@@ -234,6 +234,7 @@ static void block2mtd_free_device(struct
/* FIXME: ensure that mtd->size % erase_size == 0 */
static struct block2mtd_dev *add_device(char *devname, int erase_size)
{
+ const fmode_t mode = FMODE_READ | FMODE_WRITE;
struct block_device *bdev;
struct block2mtd_dev *dev;
char *name;
@@ -246,7 +247,7 @@ static struct block2mtd_dev *add_device(
return NULL;
/* Get a handle on the device */
- bdev = open_bdev_exclusive(devname, FMODE_READ|FMODE_WRITE, NULL);
+ bdev = open_bdev_exclusive(devname, mode, dev);
#ifndef MODULE
if (IS_ERR(bdev)) {
@@ -255,7 +256,15 @@ static struct block2mtd_dev *add_device(
dev_t devt = name_to_dev_t(devname);
if (devt) {
- bdev = open_by_devnum(devt, FMODE_WRITE | FMODE_READ);
+ bdev = open_by_devnum(devt, mode);
+ if (!IS_ERR(bdev)) {
+ int ret;
+ ret = bd_claim(bdev, dev);
+ if (ret) {
+ blkdev_put(bdev, mode);
+ bdev = ERR_PTR(ret);
+ }
+ }
}
}
#endif
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH UPDATED 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
2010-11-13 10:59 ` [PATCH UPDATED " Tejun Heo
@ 2010-11-13 11:14 ` Artem Bityutskiy
2010-11-13 11:18 ` Tejun Heo
0 siblings, 1 reply; 27+ messages in thread
From: Artem Bityutskiy @ 2010-11-13 11:14 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
On Sat, 2010-11-13 at 11:59 +0100, Tejun Heo wrote:
> There are two bdev exclusive open bugs.
>
> * open_bdev_exclusive() must not be called with NULL holder. Use dev
> as the holder.
>
> * open_by_devnum() doesn't open the bdev exclusively but
> block2mtd_free_device() always assumes it. Explicitly claim the
> bdev.
>
> The latter is rather clumsy but will be simplified with future
> blkdev_get/put() cleanups.
>
> - Updated to use local variable @mode to cache FMODE_* masks as
> suggested by Artem Bityutskiy.
Do you want this patch to go in via Jens' tree with the rest of the
series, or as an independent patch via the mtd tree?
Anyway,
Acked-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Thanks!
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH UPDATED 1/5] mtd: fix bdev exclusive open bugs in block2mtd::add_device()
2010-11-13 11:14 ` Artem Bityutskiy
@ 2010-11-13 11:18 ` Tejun Heo
0 siblings, 0 replies; 27+ messages in thread
From: Tejun Heo @ 2010-11-13 11:18 UTC (permalink / raw)
To: dedekind1
Cc: axboe, hch, linux-kernel, petero2, schwidefsky, heiko.carstens,
jack, akpm, adilger.kernel, tytso, mfasheh, joel.becker, aelder,
dm-devel, drbd-dev, neilb, leochen, sbranden, chris.mason,
swhiteho, shaggy, joern, konishi.ryusuke, reiserfs-devel, viro,
linux-mtd
Hello,
On 11/13/2010 12:14 PM, Artem Bityutskiy wrote:
> On Sat, 2010-11-13 at 11:59 +0100, Tejun Heo wrote:
>> There are two bdev exclusive open bugs.
>>
>> * open_bdev_exclusive() must not be called with NULL holder. Use dev
>> as the holder.
>>
>> * open_by_devnum() doesn't open the bdev exclusively but
>> block2mtd_free_device() always assumes it. Explicitly claim the
>> bdev.
>>
>> The latter is rather clumsy but will be simplified with future
>> blkdev_get/put() cleanups.
>>
>> - Updated to use local variable @mode to cache FMODE_* masks as
>> suggested by Artem Bityutskiy.
>
> Do you want this patch to go in via Jens' tree with the rest of the
> series, or as an independent patch via the mtd tree?
It probably would be easier if this is routed through Jens' tree as
it's necessary for later patches in the series, but this being a fix,
this should be merged during this cycle rather than the next one.
Jens, how do you want to proceed?
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 27+ messages in thread