| Message ID | 20260226075448.2229655-1-dlemoal@kernel.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | block: remove bdev_nonrot() | expand |
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
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, > 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(). Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
On 2/26/26 5:27 AM, Damien Le Moal wrote: > > 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. It's a bit of pointless churn, but I kind of suspected this was coming when we added the bdev_rot() helper and now had both of them. So I guess we may as well finish it, as we're half-way there anyway.
On Thu, 26 Feb 2026 16:54:48 +0900, Damien Le Moal wrote: > 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(). > > Applied, thanks! [1/1] block: remove bdev_nonrot() commit: d47f7c173262bbeb09645ec72bf91755eed6b1b3 Best regards,
On 2/26/26 08:54, Damien Le Moal wrote: > 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(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
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);
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(-)