Re: [PATCH 05/13] block: turn bdev->bd_openers into an atomic_t

[Jan Kara <jack@suse.cz>]

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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR