Skip to content

Commit

Permalink
block: fix locking for struct block_device size updates
Browse files Browse the repository at this point in the history
Two different callers use two different mutexes for updating the
block device size, which obviously doesn't help to actually protect
against concurrent updates from the different callers.  In addition
one of the locks, bd_mutex is rather prone to deadlocks with other
parts of the block stack that use it for high level synchronization.

Switch to using a new spinlock protecting just the size updates, as
that is all we need, and make sure everyone does the update through
the proper helper.

This fixes a bug reported with the nvme revalidating disks during a
hot removal operation, which can currently deadlock on bd_mutex.

Reported-by: Xianting Tian <xianting_tian@126.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
Christoph Hellwig authored and Jens Axboe committed Sep 1, 2020
1 parent 611bee5 commit c2b4bb8
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 36 deletions.
4 changes: 2 additions & 2 deletions block/partitions/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,8 @@ int bdev_resize_partition(struct block_device *bdev, int partno,
if (partition_overlaps(bdev->bd_disk, start, length, partno))
goto out_unlock;

part_nr_sects_write(part, (sector_t)length);
i_size_write(bdevp->bd_inode, length << SECTOR_SHIFT);
part_nr_sects_write(part, length);
bd_set_nr_sectors(bdevp, length);

ret = 0;
out_unlock:
Expand Down
4 changes: 1 addition & 3 deletions drivers/block/aoe/aoecmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -900,9 +900,7 @@ aoecmd_sleepwork(struct work_struct *work)
ssize = get_capacity(d->gd);
bd = bdget_disk(d->gd, 0);
if (bd) {
inode_lock(bd->bd_inode);
i_size_write(bd->bd_inode, (loff_t)ssize<<9);
inode_unlock(bd->bd_inode);
bd_set_nr_sectors(bd, ssize);
bdput(bd);
}
spin_lock_irq(&d->lock);
Expand Down
15 changes: 2 additions & 13 deletions drivers/md/dm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2097,18 +2097,6 @@ static void event_callback(void *context)
dm_issue_global_event();
}

/*
* Protected by md->suspend_lock obtained by dm_swap_table().
*/
static void __set_size(struct mapped_device *md, sector_t size)
{
lockdep_assert_held(&md->suspend_lock);

set_capacity(md->disk, size);

i_size_write(md->bdev->bd_inode, (loff_t)size << SECTOR_SHIFT);
}

/*
* Returns old map, which caller must destroy.
*/
Expand All @@ -2131,7 +2119,8 @@ static struct dm_table *__bind(struct mapped_device *md, struct dm_table *t,
if (size != dm_get_size(md))
memset(&md->geometry, 0, sizeof(md->geometry));

__set_size(md, size);
set_capacity(md->disk, size);
bd_set_nr_sectors(md->bdev, size);

dm_table_event_callback(t, event_callback, md);

Expand Down
9 changes: 2 additions & 7 deletions drivers/s390/block/dasd_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ dasd_ioctl_enable(struct block_device *bdev)

dasd_enable_device(base);
/* Formatting the dasd device can change the capacity. */
mutex_lock(&bdev->bd_mutex);
i_size_write(bdev->bd_inode,
(loff_t)get_capacity(base->block->gdp) << 9);
mutex_unlock(&bdev->bd_mutex);
bd_set_nr_sectors(bdev, get_capacity(base->block->gdp));
dasd_put_device(base);
return 0;
}
Expand Down Expand Up @@ -91,9 +88,7 @@ dasd_ioctl_disable(struct block_device *bdev)
* Set i_size to zero, since read, write, etc. check against this
* value.
*/
mutex_lock(&bdev->bd_mutex);
i_size_write(bdev->bd_inode, 0);
mutex_unlock(&bdev->bd_mutex);
bd_set_nr_sectors(bdev, 0);
dasd_put_device(base);
return 0;
}
Expand Down
25 changes: 14 additions & 11 deletions fs/block_dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ struct block_device *bdget(dev_t dev)
bdev = &BDEV_I(inode)->bdev;

if (inode->i_state & I_NEW) {
spin_lock_init(&bdev->bd_size_lock);
bdev->bd_contains = NULL;
bdev->bd_super = NULL;
bdev->bd_inode = inode;
Expand Down Expand Up @@ -1290,6 +1291,7 @@ static void check_disk_size_change(struct gendisk *disk,
{
loff_t disk_size, bdev_size;

spin_lock(&bdev->bd_size_lock);
disk_size = (loff_t)get_capacity(disk) << 9;
bdev_size = i_size_read(bdev->bd_inode);
if (disk_size != bdev_size) {
Expand All @@ -1299,11 +1301,15 @@ static void check_disk_size_change(struct gendisk *disk,
disk->disk_name, bdev_size, disk_size);
}
i_size_write(bdev->bd_inode, disk_size);
if (bdev_size > disk_size && __invalidate_device(bdev, false))
}
bdev->bd_invalidated = 0;
spin_unlock(&bdev->bd_size_lock);

if (bdev_size > disk_size) {
if (__invalidate_device(bdev, false))
pr_warn("VFS: busy inodes on resized disk %s\n",
disk->disk_name);
}
bdev->bd_invalidated = 0;
}

/**
Expand All @@ -1328,13 +1334,10 @@ int revalidate_disk(struct gendisk *disk)
if (!(disk->flags & GENHD_FL_HIDDEN)) {
struct block_device *bdev = bdget_disk(disk, 0);

if (!bdev)
return ret;

mutex_lock(&bdev->bd_mutex);
check_disk_size_change(disk, bdev, ret == 0);
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
if (bdev) {
check_disk_size_change(disk, bdev, ret == 0);
bdput(bdev);
}
}
return ret;
}
Expand Down Expand Up @@ -1373,9 +1376,9 @@ EXPORT_SYMBOL(check_disk_change);

void bd_set_nr_sectors(struct block_device *bdev, sector_t sectors)
{
inode_lock(bdev->bd_inode);
spin_lock(&bdev->bd_size_lock);
i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
inode_unlock(bdev->bd_inode);
spin_unlock(&bdev->bd_size_lock);
}
EXPORT_SYMBOL(bd_set_nr_sectors);

Expand Down
1 change: 1 addition & 0 deletions include/linux/blk_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ struct block_device {
/* number of times partitions within this device have been opened. */
unsigned bd_part_count;
int bd_invalidated;
spinlock_t bd_size_lock; /* for bd_inode->i_size updates */
struct gendisk * bd_disk;
struct backing_dev_info *bd_bdi;

Expand Down

0 comments on commit c2b4bb8

Please sign in to comment.