| Message ID | 20220324075119.1556334-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] nbd: use the correct block_device in nbd_ioctl | expand |
On Thu 24-03-22 08:51:11, Christoph Hellwig wrote: > All manipulation of bd_openers is under disk->open_mutex and will remain > so for the foreseeable future. But at least one place reads it without > the lock (blkdev_get) and there are more to be added. So make sure the > compiler does not do turn the increments and decrements into non-atomic > sequences by using an atomic_t. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> BTW I suspect that drivers/block/aoe/ could now use bd_openers instead of its driver specific mirror of it (->nopen). But that's certainly a separate cleanup for some other time. Honza > --- > block/bdev.c | 16 ++++++++-------- > block/partitions/core.c | 2 +- > include/linux/blk_types.h | 2 +- > include/linux/blkdev.h | 2 +- > 4 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index 13de871fa8169..7bf88e591aaf3 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -673,17 +673,17 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode) > } > } > > - if (!bdev->bd_openers) > + if (!atomic_read(&bdev->bd_openers)) > set_init_blocksize(bdev); > if (test_bit(GD_NEED_PART_SCAN, &disk->state)) > bdev_disk_changed(disk, false); > - bdev->bd_openers++; > + atomic_inc(&bdev->bd_openers); > return 0; > } > > static void blkdev_put_whole(struct block_device *bdev, fmode_t mode) > { > - if (!--bdev->bd_openers) > + if (atomic_dec_and_test(&bdev->bd_openers)) > blkdev_flush_mapping(bdev); > if (bdev->bd_disk->fops->release) > bdev->bd_disk->fops->release(bdev->bd_disk, mode); > @@ -694,7 +694,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) > struct gendisk *disk = part->bd_disk; > int ret; > > - if (part->bd_openers) > + if (atomic_read(&part->bd_openers)) > goto done; > > ret = blkdev_get_whole(bdev_whole(part), mode); > @@ -708,7 +708,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) > disk->open_partitions++; > set_init_blocksize(part); > done: > - part->bd_openers++; > + atomic_inc(&part->bd_openers); > return 0; > > out_blkdev_put: > @@ -720,7 +720,7 @@ static void blkdev_put_part(struct block_device *part, fmode_t mode) > { > struct block_device *whole = bdev_whole(part); > > - if (--part->bd_openers) > + if (!atomic_dec_and_test(&part->bd_openers)) > return; > blkdev_flush_mapping(part); > whole->bd_disk->open_partitions--; > @@ -899,7 +899,7 @@ void blkdev_put(struct block_device *bdev, fmode_t mode) > * of the world and we want to avoid long (could be several minute) > * syncs while holding the mutex. > */ > - if (bdev->bd_openers == 1) > + if (atomic_read(&bdev->bd_openers) == 1) > sync_blockdev(bdev); > > mutex_lock(&disk->open_mutex); > @@ -1044,7 +1044,7 @@ void sync_bdevs(bool wait) > bdev = I_BDEV(inode); > > mutex_lock(&bdev->bd_disk->open_mutex); > - if (!bdev->bd_openers) { > + if (!atomic_read(&bdev->bd_openers)) { > ; /* skip */ > } else if (wait) { > /* > diff --git a/block/partitions/core.c b/block/partitions/core.c > index 2ef8dfa1e5c85..373ed748dcf26 100644 > --- a/block/partitions/core.c > +++ b/block/partitions/core.c > @@ -486,7 +486,7 @@ int bdev_del_partition(struct gendisk *disk, int partno) > goto out_unlock; > > ret = -EBUSY; > - if (part->bd_openers) > + if (atomic_read(&part->bd_openers)) > goto out_unlock; > > delete_partition(part); > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 0c3563b45fe90..b1ced43ed0d3f 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -44,7 +44,7 @@ struct block_device { > unsigned long bd_stamp; > bool bd_read_only; /* read-only policy */ > dev_t bd_dev; > - int bd_openers; > + atomic_t bd_openers; > struct inode * bd_inode; /* will die */ > struct super_block * bd_super; > void * bd_claiming; > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 9824ebc9b4d31..6b7c5af1d01df 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -188,7 +188,7 @@ static inline bool disk_live(struct gendisk *disk) > */ > static inline unsigned int disk_openers(struct gendisk *disk) > { > - return disk->part0->bd_openers; > + return atomic_read(&disk->part0->bd_openers); > } > > /* > -- > 2.30.2 >
On Thu, Mar 24, 2022 at 02:31:36PM +0100, Jan Kara wrote: > BTW I suspect that drivers/block/aoe/ could now use bd_openers instead of > its driver specific mirror of it (->nopen). But that's certainly a separate > cleanup for some other time. Yes. There actually are a lot of places that should drop their internal number of openers counters.
diff --git a/block/bdev.c b/block/bdev.c index 13de871fa8169..7bf88e591aaf3 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -673,17 +673,17 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode) } } - if (!bdev->bd_openers) + if (!atomic_read(&bdev->bd_openers)) set_init_blocksize(bdev); if (test_bit(GD_NEED_PART_SCAN, &disk->state)) bdev_disk_changed(disk, false); - bdev->bd_openers++; + atomic_inc(&bdev->bd_openers); return 0; } static void blkdev_put_whole(struct block_device *bdev, fmode_t mode) { - if (!--bdev->bd_openers) + if (atomic_dec_and_test(&bdev->bd_openers)) blkdev_flush_mapping(bdev); if (bdev->bd_disk->fops->release) bdev->bd_disk->fops->release(bdev->bd_disk, mode); @@ -694,7 +694,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) struct gendisk *disk = part->bd_disk; int ret; - if (part->bd_openers) + if (atomic_read(&part->bd_openers)) goto done; ret = blkdev_get_whole(bdev_whole(part), mode); @@ -708,7 +708,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) disk->open_partitions++; set_init_blocksize(part); done: - part->bd_openers++; + atomic_inc(&part->bd_openers); return 0; out_blkdev_put: @@ -720,7 +720,7 @@ static void blkdev_put_part(struct block_device *part, fmode_t mode) { struct block_device *whole = bdev_whole(part); - if (--part->bd_openers) + if (!atomic_dec_and_test(&part->bd_openers)) return; blkdev_flush_mapping(part); whole->bd_disk->open_partitions--; @@ -899,7 +899,7 @@ void blkdev_put(struct block_device *bdev, fmode_t mode) * of the world and we want to avoid long (could be several minute) * syncs while holding the mutex. */ - if (bdev->bd_openers == 1) + if (atomic_read(&bdev->bd_openers) == 1) sync_blockdev(bdev); mutex_lock(&disk->open_mutex); @@ -1044,7 +1044,7 @@ void sync_bdevs(bool wait) bdev = I_BDEV(inode); mutex_lock(&bdev->bd_disk->open_mutex); - if (!bdev->bd_openers) { + if (!atomic_read(&bdev->bd_openers)) { ; /* skip */ } else if (wait) { /* diff --git a/block/partitions/core.c b/block/partitions/core.c index 2ef8dfa1e5c85..373ed748dcf26 100644 --- a/block/partitions/core.c +++ b/block/partitions/core.c @@ -486,7 +486,7 @@ int bdev_del_partition(struct gendisk *disk, int partno) goto out_unlock; ret = -EBUSY; - if (part->bd_openers) + if (atomic_read(&part->bd_openers)) goto out_unlock; delete_partition(part); diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 0c3563b45fe90..b1ced43ed0d3f 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -44,7 +44,7 @@ struct block_device { unsigned long bd_stamp; bool bd_read_only; /* read-only policy */ dev_t bd_dev; - int bd_openers; + atomic_t bd_openers; struct inode * bd_inode; /* will die */ struct super_block * bd_super; void * bd_claiming; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 9824ebc9b4d31..6b7c5af1d01df 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -188,7 +188,7 @@ static inline bool disk_live(struct gendisk *disk) */ static inline unsigned int disk_openers(struct gendisk *disk) { - return disk->part0->bd_openers; + return atomic_read(&disk->part0->bd_openers); } /*
All manipulation of bd_openers is under disk->open_mutex and will remain so for the foreseeable future. But at least one place reads it without the lock (blkdev_get) and there are more to be added. So make sure the compiler does not do turn the increments and decrements into non-atomic sequences by using an atomic_t. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bdev.c | 16 ++++++++-------- block/partitions/core.c | 2 +- include/linux/blk_types.h | 2 +- include/linux/blkdev.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-)