Skip to content

Commit

Permalink
block: make blkdev_get/put() handle exclusive access
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Acked-by: Neil Brown <[email protected]>
Acked-by: Ryusuke Konishi <[email protected]>
Acked-by: Mike Snitzer <[email protected]>
Acked-by: Philipp Reisner <[email protected]>
Cc: Peter Osterlund <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: "Theodore Ts'o" <[email protected]>
Cc: Mark Fasheh <[email protected]>
Cc: Joel Becker <[email protected]>
Cc: Alex Elder <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Leo Chen <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Chris Mason <[email protected]>
Cc: Steven Whitehouse <[email protected]>
Cc: Dave Kleikamp <[email protected]>
Cc: Joern Engel <[email protected]>
Cc: [email protected]
Cc: Alexander Viro <[email protected]>
  • Loading branch information
@htejun
htejun committed Nov 13, 2010
1 parent e09b457 commit e525fd8
Show file tree
Hide file tree
Showing 26 changed files with 162 additions and 318 deletions.
5 changes: 3 additions & 2 deletions block/ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions drivers/block/drbd/drbd_int.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
7 changes: 2 additions & 5 deletions drivers/block/drbd/drbd_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
103 changes: 36 additions & 67 deletions drivers/block/drbd/drbd_nl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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 */
Expand All @@ -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) {
Expand All @@ -999,15 +972,15 @@ 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
* (we may currently be R_PRIMARY with no local disk...) */
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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
22 changes: 8 additions & 14 deletions drivers/block/pktcdvd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand All @@ -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;
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;

Expand Down
14 changes: 4 additions & 10 deletions drivers/char/raw.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
15 changes: 3 additions & 12 deletions drivers/md/dm-table.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
14 changes: 3 additions & 11 deletions drivers/md/md.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Loading

0 comments on commit e525fd8

Please sign in to comment.