[PATCH] block: remove bdev_nonrot()

[PATCH] block: remove bdev_nonrot()

[Damien Le Moal]

bdev_nonrot() is simply the negative return value of bdev_rot().
So replace all call sites of bdev_nonrot() with calls to bdev_rot()
and remove bdev_nonrot().

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/md/raid1.c                  | 2 +-
 drivers/md/raid10.c                 | 2 +-
 drivers/md/raid5.c                  | 2 +-
 drivers/target/target_core_file.c   | 2 +-
 drivers/target/target_core_iblock.c | 2 +-
 fs/btrfs/volumes.c                  | 4 ++--
 fs/ext4/mballoc-test.c              | 2 +-
 fs/ext4/mballoc.c                   | 2 +-
 include/linux/blkdev.h              | 5 -----
 mm/swapfile.c                       | 2 +-
 10 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 181400e147c0..cda6af0712b9 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1878,7 +1878,7 @@ static bool raid1_add_conf(struct r1conf *conf, struct md_rdev *rdev, int disk,
 	if (info->rdev)
 		return false;
 
-	if (bdev_nonrot(rdev->bdev)) {
+	if (!bdev_rot(rdev->bdev)) {
 		set_bit(Nonrot, &rdev->flags);
 		WRITE_ONCE(conf->nonrot_disks, conf->nonrot_disks + 1);
 	}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0653b5d8545a..cfbd345805ca 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -806,7 +806,7 @@ static struct md_rdev *read_balance(struct r10conf *conf,
 		if (!do_balance)
 			break;
 
-		nonrot = bdev_nonrot(rdev->bdev);
+		nonrot = !bdev_rot(rdev->bdev);
 		has_nonrot_disk |= nonrot;
 		pending = atomic_read(&rdev->nr_pending);
 		if (min_pending > pending && nonrot) {
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index a8e8d431071b..ba9d6d05b089 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7541,7 +7541,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	rdev_for_each(rdev, mddev) {
 		if (test_bit(Journal, &rdev->flags))
 			continue;
-		if (bdev_nonrot(rdev->bdev)) {
+		if (!bdev_rot(rdev->bdev)) {
 			conf->batch_bio_dispatch = false;
 			break;
 		}
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 3ae1f7137d9d..d6e3e5214652 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -173,7 +173,7 @@ static int fd_configure_device(struct se_device *dev)
 		 */
 		dev->dev_attrib.max_write_same_len = 0xFFFF;
 
-		if (bdev_nonrot(bdev))
+		if (!bdev_rot(bdev))
 			dev->dev_attrib.is_nonrot = 1;
 	} else {
 		if (!(fd_dev->fbd_flags & FBDF_HAS_SIZE)) {
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 3c92f94497b4..1087d1d17c36 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -148,7 +148,7 @@ static int iblock_configure_device(struct se_device *dev)
 	else
 		dev->dev_attrib.max_write_same_len = 0xFFFF;
 
-	if (bdev_nonrot(bd))
+	if (!bdev_rot(bd))
 		dev->dev_attrib.is_nonrot = 1;
 
 	target_configure_write_atomic_from_bdev(&dev->dev_attrib, bd);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6fb0c4cd50ff..c6e49eb74f3a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -694,7 +694,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
 			set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	}
 
-	if (!bdev_nonrot(file_bdev(bdev_file)))
+	if (bdev_rot(file_bdev(bdev_file)))
 		fs_devices->rotating = true;
 
 	if (bdev_max_discard_sectors(file_bdev(bdev_file)))
@@ -2919,7 +2919,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 	atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
 
-	if (!bdev_nonrot(device->bdev))
+	if (bdev_rot(device->bdev))
 		fs_devices->rotating = true;
 
 	orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
index 9fbdf6a09489..b9f22e3a8d5c 100644
--- a/fs/ext4/mballoc-test.c
+++ b/fs/ext4/mballoc-test.c
@@ -72,7 +72,7 @@ static int mbt_mb_init(struct super_block *sb)
 	ext4_fsblk_t block;
 	int ret;
 
-	/* needed by ext4_mb_init->bdev_nonrot(sb->s_bdev) */
+	/* needed by ext4_mb_init->bdev_rot(sb->s_bdev) */
 	sb->s_bdev = kzalloc_obj(*sb->s_bdev);
 	if (sb->s_bdev == NULL)
 		return -ENOMEM;
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 20e9fdaf4301..8a4dfe19878c 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3836,7 +3836,7 @@ int ext4_mb_init(struct super_block *sb)
 		spin_lock_init(&lg->lg_prealloc_lock);
 	}
 
-	if (bdev_nonrot(sb->s_bdev))
+	if (!bdev_rot(sb->s_bdev))
 		sbi->s_mb_max_linear_groups = 0;
 	else
 		sbi->s_mb_max_linear_groups = MB_DEFAULT_LINEAR_LIMIT;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d463b9b5a0a5..e439d6fa8484 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,11 +1467,6 @@ static inline bool bdev_rot(struct block_device *bdev)
 	return blk_queue_rot(bdev_get_queue(bdev));
 }
 
-static inline bool bdev_nonrot(struct block_device *bdev)
-{
-	return !bdev_rot(bdev);
-}
-
 static inline bool bdev_synchronous(struct block_device *bdev)
 {
 	return bdev->bd_disk->queue->limits.features & BLK_FEAT_SYNCHRONOUS;
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 94af29d1de88..60e21414624b 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3460,7 +3460,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	if (si->bdev && bdev_synchronous(si->bdev))
 		si->flags |= SWP_SYNCHRONOUS_IO;
 
-	if (si->bdev && bdev_nonrot(si->bdev)) {
+	if (si->bdev && !bdev_rot(si->bdev)) {
 		si->flags |= SWP_SOLIDSTATE;
 	} else {
 		atomic_inc(&nr_rotate_swap);
-- 
2.53.0

Re: [PATCH] block: remove bdev_nonrot()

[Paul Menzel]

Dear Damien,


Thank you for your patch.


Am 26.02.26 um 08:54 schrieb Damien Le Moal:
> bdev_nonrot() is simply the negative return value of bdev_rot().
> So replace all call sites of bdev_nonrot() with calls to bdev_rot()
> and remove bdev_nonrot().

Is the generated code different now?

Is it worth the change, as it looks quite subjective if you prefer the 
one or the other way?

> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/md/raid1.c                  | 2 +-
>   drivers/md/raid10.c                 | 2 +-
>   drivers/md/raid5.c                  | 2 +-
>   drivers/target/target_core_file.c   | 2 +-
>   drivers/target/target_core_iblock.c | 2 +-
>   fs/btrfs/volumes.c                  | 4 ++--
>   fs/ext4/mballoc-test.c              | 2 +-
>   fs/ext4/mballoc.c                   | 2 +-
>   include/linux/blkdev.h              | 5 -----
>   mm/swapfile.c                       | 2 +-
>   10 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 181400e147c0..cda6af0712b9 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1878,7 +1878,7 @@ static bool raid1_add_conf(struct r1conf *conf, struct md_rdev *rdev, int disk,
>   	if (info->rdev)
>   		return false;
>   
> -	if (bdev_nonrot(rdev->bdev)) {
> +	if (!bdev_rot(rdev->bdev)) {
>   		set_bit(Nonrot, &rdev->flags);
>   		WRITE_ONCE(conf->nonrot_disks, conf->nonrot_disks + 1);
>   	}
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0653b5d8545a..cfbd345805ca 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -806,7 +806,7 @@ static struct md_rdev *read_balance(struct r10conf *conf,
>   		if (!do_balance)
>   			break;
>   
> -		nonrot = bdev_nonrot(rdev->bdev);
> +		nonrot = !bdev_rot(rdev->bdev);
>   		has_nonrot_disk |= nonrot;
>   		pending = atomic_read(&rdev->nr_pending);
>   		if (min_pending > pending && nonrot) {
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index a8e8d431071b..ba9d6d05b089 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7541,7 +7541,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>   	rdev_for_each(rdev, mddev) {
>   		if (test_bit(Journal, &rdev->flags))
>   			continue;
> -		if (bdev_nonrot(rdev->bdev)) {
> +		if (!bdev_rot(rdev->bdev)) {
>   			conf->batch_bio_dispatch = false;
>   			break;
>   		}
> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index 3ae1f7137d9d..d6e3e5214652 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -173,7 +173,7 @@ static int fd_configure_device(struct se_device *dev)
>   		 */
>   		dev->dev_attrib.max_write_same_len = 0xFFFF;
>   
> -		if (bdev_nonrot(bdev))
> +		if (!bdev_rot(bdev))
>   			dev->dev_attrib.is_nonrot = 1;
>   	} else {
>   		if (!(fd_dev->fbd_flags & FBDF_HAS_SIZE)) {
> diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
> index 3c92f94497b4..1087d1d17c36 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
> @@ -148,7 +148,7 @@ static int iblock_configure_device(struct se_device *dev)
>   	else
>   		dev->dev_attrib.max_write_same_len = 0xFFFF;
>   
> -	if (bdev_nonrot(bd))
> +	if (!bdev_rot(bd))
>   		dev->dev_attrib.is_nonrot = 1;
>   
>   	target_configure_write_atomic_from_bdev(&dev->dev_attrib, bd);
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6fb0c4cd50ff..c6e49eb74f3a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -694,7 +694,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>   			set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>   	}
>   
> -	if (!bdev_nonrot(file_bdev(bdev_file)))
> +	if (bdev_rot(file_bdev(bdev_file)))
>   		fs_devices->rotating = true;
>   
>   	if (bdev_max_discard_sectors(file_bdev(bdev_file)))
> @@ -2919,7 +2919,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   
>   	atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
>   
> -	if (!bdev_nonrot(device->bdev))
> +	if (bdev_rot(device->bdev))
>   		fs_devices->rotating = true;
>   
>   	orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
> diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
> index 9fbdf6a09489..b9f22e3a8d5c 100644
> --- a/fs/ext4/mballoc-test.c
> +++ b/fs/ext4/mballoc-test.c
> @@ -72,7 +72,7 @@ static int mbt_mb_init(struct super_block *sb)
>   	ext4_fsblk_t block;
>   	int ret;
>   
> -	/* needed by ext4_mb_init->bdev_nonrot(sb->s_bdev) */
> +	/* needed by ext4_mb_init->bdev_rot(sb->s_bdev) */
>   	sb->s_bdev = kzalloc_obj(*sb->s_bdev);
>   	if (sb->s_bdev == NULL)
>   		return -ENOMEM;
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 20e9fdaf4301..8a4dfe19878c 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3836,7 +3836,7 @@ int ext4_mb_init(struct super_block *sb)
>   		spin_lock_init(&lg->lg_prealloc_lock);
>   	}
>   
> -	if (bdev_nonrot(sb->s_bdev))
> +	if (!bdev_rot(sb->s_bdev))
>   		sbi->s_mb_max_linear_groups = 0;
>   	else
>   		sbi->s_mb_max_linear_groups = MB_DEFAULT_LINEAR_LIMIT;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index d463b9b5a0a5..e439d6fa8484 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1467,11 +1467,6 @@ static inline bool bdev_rot(struct block_device *bdev)
>   	return blk_queue_rot(bdev_get_queue(bdev));
>   }
>   
> -static inline bool bdev_nonrot(struct block_device *bdev)
> -{
> -	return !bdev_rot(bdev);
> -}
> -
>   static inline bool bdev_synchronous(struct block_device *bdev)
>   {
>   	return bdev->bd_disk->queue->limits.features & BLK_FEAT_SYNCHRONOUS;
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 94af29d1de88..60e21414624b 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3460,7 +3460,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>   	if (si->bdev && bdev_synchronous(si->bdev))
>   		si->flags |= SWP_SYNCHRONOUS_IO;
>   
> -	if (si->bdev && bdev_nonrot(si->bdev)) {
> +	if (si->bdev && !bdev_rot(si->bdev)) {
>   		si->flags |= SWP_SOLIDSTATE;
>   	} else {
>   		atomic_inc(&nr_rotate_swap);

My point above aside, the diff looks good.

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [PATCH] block: remove bdev_nonrot()

[Damien Le Moal]

On 2/26/26 19:04, Paul Menzel wrote:
> Dear Damien,
> 
> 
> Thank you for your patch.
> 
> 
> Am 26.02.26 um 08:54 schrieb Damien Le Moal:
>> bdev_nonrot() is simply the negative return value of bdev_rot().
>> So replace all call sites of bdev_nonrot() with calls to bdev_rot()
>> and remove bdev_nonrot().
> 
> Is the generated code different now?

I did not check but I doubt there is any difference at all.

This is more about having a single helper function for bdevs that is consistent
with the helper for request queues (blk_queue_rot()) which directly reflects the
setting of a block device BLK_FEAT_ROTATIONAL queue feature. This way is also in
my opinion simpler as you do not get your brain overheating when seeing things
like "!bdev_nonrot()" :)
  > Is it worth the change, as it looks quite subjective if you prefer the
> one or the other way?

I think it is a nice cleanup, but I will let Jens and other maintainers decide
on the worth of this patch.

> My point above aside, the diff looks good.
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>

Thanks.

-- 
Damien Le Moal
Western Digital Research