Skip to content

Commit

Permalink
block: use the holder as indication for exclusive opens
Browse files Browse the repository at this point in the history
The current interface for exclusive opens is rather confusing as it
requires both the FMODE_EXCL flag and a holder.  Remove the need to pass
FMODE_EXCL and just key off the exclusive open off a non-NULL holder.

For blkdev_put this requires adding the holder argument, which provides
better debug checking that only the holder actually releases the hold,
but at the same time allows removing the now superfluous mode argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christian Brauner <brauner@kernel.org>
Acked-by: David Sterba <dsterba@suse.com>		[btrfs]
Acked-by: Jack Wang <jinpu.wang@ionos.com>		[rnbd]
Link: https://lore.kernel.org/r/20230608110258.189493-16-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
Christoph Hellwig authored and axboe committed Jun 12, 2023
1 parent 2ef7892 commit 2736e8e
Show file tree
Hide file tree
Showing 37 changed files with 183 additions and 192 deletions.
37 changes: 21 additions & 16 deletions block/bdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ void bd_abort_claiming(struct block_device *bdev, void *holder)
}
EXPORT_SYMBOL(bd_abort_claiming);

static void bd_end_claim(struct block_device *bdev)
static void bd_end_claim(struct block_device *bdev, void *holder)
{
struct block_device *whole = bdev_whole(bdev);
bool unblock = false;
Expand All @@ -614,6 +614,7 @@ static void bd_end_claim(struct block_device *bdev)
* bdev_lock. open_mutex is used to synchronize disk_holder unlinking.
*/
mutex_lock(&bdev_lock);
WARN_ON_ONCE(bdev->bd_holder != holder);
WARN_ON_ONCE(--bdev->bd_holders < 0);
WARN_ON_ONCE(--whole->bd_holders < 0);
if (!bdev->bd_holders) {
Expand Down Expand Up @@ -750,10 +751,9 @@ void blkdev_put_no_open(struct block_device *bdev)
* @holder: exclusive holder identifier
* @hops: holder operations
*
* Open the block device described by device number @dev. If @mode includes
* %FMODE_EXCL, the block device is opened with exclusive access. Specifying
* %FMODE_EXCL with a %NULL @holder is invalid. Exclusive opens may nest for
* the same @holder.
* Open the block device described by device number @dev. If @holder is not
* %NULL, the block device is opened with exclusive access. Exclusive opens may
* nest for the same @holder.
*
* Use this interface 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
Expand Down Expand Up @@ -785,10 +785,16 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder,
return ERR_PTR(-ENXIO);
disk = bdev->bd_disk;

if (mode & FMODE_EXCL) {
if (holder) {
mode |= FMODE_EXCL;
ret = bd_prepare_to_claim(bdev, holder, hops);
if (ret)
goto put_blkdev;
} else {
if (WARN_ON_ONCE(mode & FMODE_EXCL)) {
ret = -EIO;
goto put_blkdev;
}
}

disk_block_events(disk);
Expand All @@ -805,7 +811,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder,
ret = blkdev_get_whole(bdev, mode);
if (ret)
goto put_module;
if (mode & FMODE_EXCL) {
if (holder) {
bd_finish_claiming(bdev, holder, hops);

/*
Expand All @@ -829,7 +835,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder,
put_module:
module_put(disk->fops->owner);
abort_claiming:
if (mode & FMODE_EXCL)
if (holder)
bd_abort_claiming(bdev, holder);
mutex_unlock(&disk->open_mutex);
disk_unblock_events(disk);
Expand All @@ -845,10 +851,9 @@ EXPORT_SYMBOL(blkdev_get_by_dev);
* @mode: FMODE_* mask
* @holder: exclusive holder identifier
*
* Open the block device described by the device file at @path. If @mode
* includes %FMODE_EXCL, the block device is opened with exclusive access.
* Specifying %FMODE_EXCL with a %NULL @holder is invalid. Exclusive opens may
* nest for the same @holder.
* Open the block device described by the device file at @path. If @holder is
* not %NULL, the block device is opened with exclusive access. Exclusive opens
* may nest for the same @holder.
*
* CONTEXT:
* Might sleep.
Expand All @@ -869,15 +874,15 @@ struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,

bdev = blkdev_get_by_dev(dev, mode, holder, hops);
if (!IS_ERR(bdev) && (mode & FMODE_WRITE) && bdev_read_only(bdev)) {
blkdev_put(bdev, mode);
blkdev_put(bdev, holder);
return ERR_PTR(-EACCES);
}

return bdev;
}
EXPORT_SYMBOL(blkdev_get_by_path);

void blkdev_put(struct block_device *bdev, fmode_t mode)
void blkdev_put(struct block_device *bdev, void *holder)
{
struct gendisk *disk = bdev->bd_disk;

Expand All @@ -892,8 +897,8 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
sync_blockdev(bdev);

mutex_lock(&disk->open_mutex);
if (mode & FMODE_EXCL)
bd_end_claim(bdev);
if (holder)
bd_end_claim(bdev, holder);

/*
* Trigger event checking and tell drivers to flush MEDIA_CHANGE
Expand Down
6 changes: 4 additions & 2 deletions block/fops.c
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,9 @@ static int blkdev_open(struct inode *inode, struct file *filp)
if ((filp->f_flags & O_ACCMODE) == 3)
filp->f_mode |= FMODE_WRITE_IOCTL;

bdev = blkdev_get_by_dev(inode->i_rdev, filp->f_mode, filp, NULL);
bdev = blkdev_get_by_dev(inode->i_rdev, filp->f_mode,
(filp->f_mode & FMODE_EXCL) ? filp : NULL,
NULL);
if (IS_ERR(bdev))
return PTR_ERR(bdev);

Expand All @@ -504,7 +506,7 @@ static int blkdev_release(struct inode *inode, struct file *filp)
{
struct block_device *bdev = filp->private_data;

blkdev_put(bdev, filp->f_mode);
blkdev_put(bdev, (filp->f_mode & FMODE_EXCL) ? filp : NULL);
return 0;
}

Expand Down
5 changes: 2 additions & 3 deletions block/genhd.c
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,11 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
}

set_bit(GD_NEED_PART_SCAN, &disk->state);
bdev = blkdev_get_by_dev(disk_devt(disk), mode & ~FMODE_EXCL, NULL,
NULL);
bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL, NULL);
if (IS_ERR(bdev))
ret = PTR_ERR(bdev);
else
blkdev_put(bdev, mode & ~FMODE_EXCL);
blkdev_put(bdev, NULL);

/*
* If blkdev_get_by_dev() failed early, GD_NEED_PART_SCAN is still set,
Expand Down
5 changes: 2 additions & 3 deletions block/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,10 @@ static int blkdev_bszset(struct block_device *bdev, fmode_t mode,
if (mode & FMODE_EXCL)
return set_blocksize(bdev, n);

if (IS_ERR(blkdev_get_by_dev(bdev->bd_dev, mode | FMODE_EXCL, &bdev,
NULL)))
if (IS_ERR(blkdev_get_by_dev(bdev->bd_dev, mode, &bdev, NULL)))
return -EBUSY;
ret = set_blocksize(bdev, n);
blkdev_put(bdev, mode | FMODE_EXCL);
blkdev_put(bdev, &bdev);

return ret;
}
Expand Down
23 changes: 14 additions & 9 deletions drivers/block/drbd/drbd_nl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1640,8 +1640,7 @@ static struct block_device *open_backing_dev(struct drbd_device *device,
struct block_device *bdev;
int err = 0;

bdev = blkdev_get_by_path(bdev_path,
FMODE_READ | FMODE_WRITE | FMODE_EXCL,
bdev = blkdev_get_by_path(bdev_path, FMODE_READ | FMODE_WRITE,
claim_ptr, NULL);
if (IS_ERR(bdev)) {
drbd_err(device, "open(\"%s\") failed with %ld\n",
Expand All @@ -1654,7 +1653,7 @@ static struct block_device *open_backing_dev(struct drbd_device *device,

err = bd_link_disk_holder(bdev, device->vdisk);
if (err) {
blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(bdev, claim_ptr);
drbd_err(device, "bd_link_disk_holder(\"%s\", ...) failed with %d\n",
bdev_path, err);
bdev = ERR_PTR(err);
Expand Down Expand Up @@ -1696,22 +1695,25 @@ static int open_backing_devices(struct drbd_device *device,
}

static void close_backing_dev(struct drbd_device *device, struct block_device *bdev,
bool do_bd_unlink)
void *claim_ptr, bool do_bd_unlink)
{
if (!bdev)
return;
if (do_bd_unlink)
bd_unlink_disk_holder(bdev, device->vdisk);
blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(bdev, claim_ptr);
}

void drbd_backing_dev_free(struct drbd_device *device, struct drbd_backing_dev *ldev)
{
if (ldev == NULL)
return;

close_backing_dev(device, ldev->md_bdev, ldev->md_bdev != ldev->backing_bdev);
close_backing_dev(device, ldev->backing_bdev, true);
close_backing_dev(device, ldev->md_bdev,
ldev->md.meta_dev_idx < 0 ?
(void *)device : (void *)drbd_m_holder,
ldev->md_bdev != ldev->backing_bdev);
close_backing_dev(device, ldev->backing_bdev, device, true);

kfree(ldev->disk_conf);
kfree(ldev);
Expand Down Expand Up @@ -2127,8 +2129,11 @@ int drbd_adm_attach(struct sk_buff *skb, struct genl_info *info)
fail:
conn_reconfig_done(connection);
if (nbc) {
close_backing_dev(device, nbc->md_bdev, nbc->md_bdev != nbc->backing_bdev);
close_backing_dev(device, nbc->backing_bdev, true);
close_backing_dev(device, nbc->md_bdev,
nbc->disk_conf->meta_dev_idx < 0 ?
(void *)device : (void *)drbd_m_holder,
nbc->md_bdev != nbc->backing_bdev);
close_backing_dev(device, nbc->backing_bdev, device, true);
kfree(nbc);
}
kfree(new_disk_conf);
Expand Down
13 changes: 6 additions & 7 deletions drivers/block/pktcdvd.c
Original file line number Diff line number Diff line change
Expand Up @@ -2167,8 +2167,7 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
* to read/write from/to it. It is already opened in O_NONBLOCK mode
* so open should not fail.
*/
bdev = blkdev_get_by_dev(pd->bdev->bd_dev, FMODE_READ | FMODE_EXCL, pd,
NULL);
bdev = blkdev_get_by_dev(pd->bdev->bd_dev, FMODE_READ, pd, NULL);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
goto out;
Expand Down Expand Up @@ -2215,7 +2214,7 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
return 0;

out_putdev:
blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
blkdev_put(bdev, pd);
out:
return ret;
}
Expand All @@ -2234,7 +2233,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);
blkdev_put(pd->bdev, FMODE_READ | FMODE_EXCL);
blkdev_put(pd->bdev, pd);

pkt_shrink_pktlist(pd);
}
Expand Down Expand Up @@ -2520,7 +2519,7 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
return PTR_ERR(bdev);
sdev = scsi_device_from_queue(bdev->bd_disk->queue);
if (!sdev) {
blkdev_put(bdev, FMODE_READ | FMODE_NDELAY);
blkdev_put(bdev, NULL);
return -EINVAL;
}
put_device(&sdev->sdev_gendev);
Expand All @@ -2545,7 +2544,7 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
return 0;

out_mem:
blkdev_put(bdev, FMODE_READ | FMODE_NDELAY);
blkdev_put(bdev, NULL);
/* This is safe: open() is still holding a reference. */
module_put(THIS_MODULE);
return -ENOMEM;
Expand Down Expand Up @@ -2751,7 +2750,7 @@ static int pkt_remove_dev(dev_t pkt_dev)
pkt_debugfs_dev_remove(pd);
pkt_sysfs_dev_remove(pd);

blkdev_put(pd->bdev, FMODE_READ | FMODE_NDELAY);
blkdev_put(pd->bdev, NULL);

remove_proc_entry(pd->disk->disk_name, pkt_proc);
dev_notice(ddev, "writer unmapped\n");
Expand Down
4 changes: 2 additions & 2 deletions drivers/block/rnbd/rnbd-srv.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void rnbd_destroy_sess_dev(struct rnbd_srv_sess_dev *sess_dev, bool keep_id)
rnbd_put_sess_dev(sess_dev);
wait_for_completion(&dc); /* wait for inflights to drop to zero */

blkdev_put(sess_dev->bdev, sess_dev->open_flags);
blkdev_put(sess_dev->bdev, NULL);
mutex_lock(&sess_dev->dev->lock);
list_del(&sess_dev->dev_list);
if (sess_dev->open_flags & FMODE_WRITE)
Expand Down Expand Up @@ -791,7 +791,7 @@ static int process_msg_open(struct rnbd_srv_session *srv_sess,
}
rnbd_put_srv_dev(srv_dev);
blkdev_put:
blkdev_put(bdev, open_flags);
blkdev_put(bdev, NULL);
free_path:
kfree(full_path);
reject:
Expand Down
2 changes: 1 addition & 1 deletion drivers/block/xen-blkback/xenbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ static void xenvbd_sysfs_delif(struct xenbus_device *dev)
static void xen_vbd_free(struct xen_vbd *vbd)
{
if (vbd->bdev)
blkdev_put(vbd->bdev, vbd->readonly ? FMODE_READ : FMODE_WRITE);
blkdev_put(vbd->bdev, NULL);
vbd->bdev = NULL;
}

Expand Down
8 changes: 4 additions & 4 deletions drivers/block/zram/zram_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ static void reset_bdev(struct zram *zram)
return;

bdev = zram->bdev;
blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
blkdev_put(bdev, zram);
/* hope filp_close flush all of IO */
filp_close(zram->backing_dev, NULL);
zram->backing_dev = NULL;
Expand Down Expand Up @@ -507,8 +507,8 @@ static ssize_t backing_dev_store(struct device *dev,
goto out;
}

bdev = blkdev_get_by_dev(inode->i_rdev,
FMODE_READ | FMODE_WRITE | FMODE_EXCL, zram, NULL);
bdev = blkdev_get_by_dev(inode->i_rdev, FMODE_READ | FMODE_WRITE, zram,
NULL);
if (IS_ERR(bdev)) {
err = PTR_ERR(bdev);
bdev = NULL;
Expand Down Expand Up @@ -539,7 +539,7 @@ static ssize_t backing_dev_store(struct device *dev,
kvfree(bitmap);

if (bdev)
blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(bdev, zram);

if (backing_dev)
filp_close(backing_dev, NULL);
Expand Down
15 changes: 7 additions & 8 deletions drivers/md/bcache/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,7 @@ static void cached_dev_free(struct closure *cl)
put_page(virt_to_page(dc->sb_disk));

if (!IS_ERR_OR_NULL(dc->bdev))
blkdev_put(dc->bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
blkdev_put(dc->bdev, bcache_kobj);

wake_up(&unregister_wait);

Expand Down Expand Up @@ -2218,7 +2218,7 @@ void bch_cache_release(struct kobject *kobj)
put_page(virt_to_page(ca->sb_disk));

if (!IS_ERR_OR_NULL(ca->bdev))
blkdev_put(ca->bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
blkdev_put(ca->bdev, bcache_kobj);

kfree(ca);
module_put(THIS_MODULE);
Expand Down Expand Up @@ -2359,7 +2359,7 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
* call blkdev_put() to bdev in bch_cache_release(). So we
* explicitly call blkdev_put() here.
*/
blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
blkdev_put(bdev, bcache_kobj);
if (ret == -ENOMEM)
err = "cache_alloc(): -ENOMEM";
else if (ret == -EPERM)
Expand Down Expand Up @@ -2461,7 +2461,7 @@ static void register_bdev_worker(struct work_struct *work)
if (!dc) {
fail = true;
put_page(virt_to_page(args->sb_disk));
blkdev_put(args->bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(args->bdev, bcache_kobj);
goto out;
}

Expand Down Expand Up @@ -2491,7 +2491,7 @@ static void register_cache_worker(struct work_struct *work)
if (!ca) {
fail = true;
put_page(virt_to_page(args->sb_disk));
blkdev_put(args->bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(args->bdev, bcache_kobj);
goto out;
}

Expand Down Expand Up @@ -2558,8 +2558,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,

ret = -EINVAL;
err = "failed to open device";
bdev = blkdev_get_by_path(strim(path),
FMODE_READ|FMODE_WRITE|FMODE_EXCL,
bdev = blkdev_get_by_path(strim(path), FMODE_READ | FMODE_WRITE,
bcache_kobj, NULL);
if (IS_ERR(bdev)) {
if (bdev == ERR_PTR(-EBUSY)) {
Expand Down Expand Up @@ -2648,7 +2647,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
out_put_sb_page:
put_page(virt_to_page(sb_disk));
out_blkdev_put:
blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
blkdev_put(bdev, register_bcache);
out_free_sb:
kfree(sb);
out_free_path:
Expand Down
Loading

0 comments on commit 2736e8e

Please sign in to comment.