* [PATCH] btrfs: cleanup finding rotating device @ 2022-01-13 4:44 Anand Jain 2022-01-13 7:38 ` Nikolay Borisov 2022-01-13 7:48 ` [PATCH v2] " Anand Jain 0 siblings, 2 replies; 7+ messages in thread From: Anand Jain @ 2022-01-13 4:44 UTC (permalink / raw) To: linux-btrfs The pointer to struct request_queue is used only to get device type rotating or the non-rotating. So use it directly. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 7e92bf130137..68ea15e8e3cb 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -609,7 +609,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, struct btrfs_device *device, fmode_t flags, void *holder) { - struct request_queue *q; struct block_device *bdev; struct btrfs_super_block *disk_super; u64 devid; @@ -651,8 +650,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); } - q = bdev_get_queue(bdev); - if (!blk_queue_nonrot(q)) + if (!blk_queue_nonrot(bdev_get_queue(bdev))) fs_devices->rotating = true; device->bdev = bdev; @@ -2600,7 +2598,6 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path) { struct btrfs_root *root = fs_info->dev_root; - struct request_queue *q; struct btrfs_trans_handle *trans; struct btrfs_device *device; struct block_device *bdev; @@ -2676,7 +2673,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path goto error_free_zone; } - q = bdev_get_queue(bdev); set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); device->generation = trans->transid; device->io_width = fs_info->sectorsize; @@ -2724,7 +2720,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 (!blk_queue_nonrot(q)) + if (!blk_queue_nonrot(bdev_get_queue(bdev))) fs_devices->rotating = true; orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy); -- 2.33.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: cleanup finding rotating device 2022-01-13 4:44 [PATCH] btrfs: cleanup finding rotating device Anand Jain @ 2022-01-13 7:38 ` Nikolay Borisov 2022-01-13 7:48 ` [PATCH v2] " Anand Jain 1 sibling, 0 replies; 7+ messages in thread From: Nikolay Borisov @ 2022-01-13 7:38 UTC (permalink / raw) To: Anand Jain, linux-btrfs On 13.01.22 г. 6:44, Anand Jain wrote: > The pointer to struct request_queue is used only to get device type > rotating or the non-rotating. So use it directly. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/volumes.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 7e92bf130137..68ea15e8e3cb 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -609,7 +609,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, > struct btrfs_device *device, fmode_t flags, > void *holder) > { > - struct request_queue *q; > struct block_device *bdev; > struct btrfs_super_block *disk_super; > u64 devid; > @@ -651,8 +650,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, > set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); > } > > - q = bdev_get_queue(bdev); > - if (!blk_queue_nonrot(q)) > + if (!blk_queue_nonrot(bdev_get_queue(bdev))) > fs_devices->rotating = true; Eliminate the altogether: fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); > > device->bdev = bdev; > @@ -2600,7 +2598,6 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) > int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path) > { > struct btrfs_root *root = fs_info->dev_root; > - struct request_queue *q; > struct btrfs_trans_handle *trans; > struct btrfs_device *device; > struct block_device *bdev; > @@ -2676,7 +2673,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > goto error_free_zone; > } > > - q = bdev_get_queue(bdev); > set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); > device->generation = trans->transid; > device->io_width = fs_info->sectorsize; > @@ -2724,7 +2720,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 (!blk_queue_nonrot(q)) > + if (!blk_queue_nonrot(bdev_get_queue(bdev))) > fs_devices->rotating = true; Ditto > > orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy); > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] btrfs: cleanup finding rotating device 2022-01-13 4:44 [PATCH] btrfs: cleanup finding rotating device Anand Jain 2022-01-13 7:38 ` Nikolay Borisov @ 2022-01-13 7:48 ` Anand Jain 2022-01-13 7:50 ` Nikolay Borisov 2022-01-18 15:43 ` David Sterba 1 sibling, 2 replies; 7+ messages in thread From: Anand Jain @ 2022-01-13 7:48 UTC (permalink / raw) To: linux-btrfs; +Cc: nborisov The pointer to struct request_queue is used only to get device type rotating or the non-rotating. So use it directly. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v2: Eliminate the if statement. fs/btrfs/volumes.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 70b085dff500..34d08c4f7b6c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -601,7 +601,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, struct btrfs_device *device, fmode_t flags, void *holder) { - struct request_queue *q; struct block_device *bdev; struct btrfs_super_block *disk_super; u64 devid; @@ -643,9 +642,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); } - q = bdev_get_queue(bdev); - if (!blk_queue_nonrot(q)) - fs_devices->rotating = true; + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); device->bdev = bdev; clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state); @@ -2590,7 +2587,6 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path) { struct btrfs_root *root = fs_info->dev_root; - struct request_queue *q; struct btrfs_trans_handle *trans; struct btrfs_device *device; struct block_device *bdev; @@ -2666,7 +2662,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path goto error_free_zone; } - q = bdev_get_queue(bdev); set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); device->generation = trans->transid; device->io_width = fs_info->sectorsize; @@ -2714,8 +2709,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 (!blk_queue_nonrot(q)) - fs_devices->rotating = true; + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy); btrfs_set_super_total_bytes(fs_info->super_copy, -- 2.33.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] btrfs: cleanup finding rotating device 2022-01-13 7:48 ` [PATCH v2] " Anand Jain @ 2022-01-13 7:50 ` Nikolay Borisov 2022-01-18 15:43 ` David Sterba 1 sibling, 0 replies; 7+ messages in thread From: Nikolay Borisov @ 2022-01-13 7:50 UTC (permalink / raw) To: Anand Jain, linux-btrfs On 13.01.22 г. 9:48, Anand Jain wrote: > The pointer to struct request_queue is used only to get device type > rotating or the non-rotating. So use it directly. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] btrfs: cleanup finding rotating device 2022-01-13 7:48 ` [PATCH v2] " Anand Jain 2022-01-13 7:50 ` Nikolay Borisov @ 2022-01-18 15:43 ` David Sterba 2022-01-19 7:43 ` Anand Jain 1 sibling, 1 reply; 7+ messages in thread From: David Sterba @ 2022-01-18 15:43 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, nborisov On Thu, Jan 13, 2022 at 03:48:54PM +0800, Anand Jain wrote: > The pointer to struct request_queue is used only to get device type > rotating or the non-rotating. So use it directly. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > v2: Eliminate the if statement. > > fs/btrfs/volumes.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 70b085dff500..34d08c4f7b6c 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -601,7 +601,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, > struct btrfs_device *device, fmode_t flags, > void *holder) > { > - struct request_queue *q; > struct block_device *bdev; > struct btrfs_super_block *disk_super; > u64 devid; > @@ -643,9 +642,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, > set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); > } > > - q = bdev_get_queue(bdev); > - if (!blk_queue_nonrot(q)) > - fs_devices->rotating = true; > + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); This is an equivalent change, in the new code fs_devices->rotating will be always set according to the last device, while in the old code, any rotational device will set it to true and never back. > > device->bdev = bdev; > clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state); > @@ -2590,7 +2587,6 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) > int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path) > { > struct btrfs_root *root = fs_info->dev_root; > - struct request_queue *q; > struct btrfs_trans_handle *trans; > struct btrfs_device *device; > struct block_device *bdev; > @@ -2666,7 +2662,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > goto error_free_zone; > } > > - q = bdev_get_queue(bdev); > set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); > device->generation = trans->transid; > device->io_width = fs_info->sectorsize; > @@ -2714,8 +2709,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 (!blk_queue_nonrot(q)) > - fs_devices->rotating = true; > + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); Same here. > > orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy); > btrfs_set_super_total_bytes(fs_info->super_copy, > -- > 2.33.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] btrfs: cleanup finding rotating device 2022-01-18 15:43 ` David Sterba @ 2022-01-19 7:43 ` Anand Jain 2022-01-20 17:08 ` David Sterba 0 siblings, 1 reply; 7+ messages in thread From: Anand Jain @ 2022-01-19 7:43 UTC (permalink / raw) To: dsterba; +Cc: linux-btrfs, nborisov On 18/01/2022 23:43, David Sterba wrote: > On Thu, Jan 13, 2022 at 03:48:54PM +0800, Anand Jain wrote: >> The pointer to struct request_queue is used only to get device type >> rotating or the non-rotating. So use it directly. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> v2: Eliminate the if statement. >> >> fs/btrfs/volumes.c | 10 ++-------- >> 1 file changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 70b085dff500..34d08c4f7b6c 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -601,7 +601,6 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, >> struct btrfs_device *device, fmode_t flags, >> void *holder) >> { >> - struct request_queue *q; >> struct block_device *bdev; >> struct btrfs_super_block *disk_super; >> u64 devid; >> @@ -643,9 +642,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices, >> set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); >> } >> >> - q = bdev_get_queue(bdev); >> - if (!blk_queue_nonrot(q)) >> - fs_devices->rotating = true; >> + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); > > This is an equivalent change, in the new code fs_devices->rotating will > be always set according to the last device, while in the old code, any > rotational device will set it to true and never back. > Oh. The previous status gets overwritten based on the current device non-rot check. This is not good. My bad. However, V1 is ok could you pls consider that? Also, there is a bug in the original code - If we have at least one rotating device (HDD) we won't enable SSD benefit. However, if that HDD is deleted later on, we won't still enable the SSD status. This bug can be fixed easily on top of the patch '[PATCH 1/2] btrfs: keep device type in the struct btrfs_device' I will wait for some comments before fixing this. Thanks, Anand >> >> device->bdev = bdev; >> clear_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state); >> @@ -2590,7 +2587,6 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) >> int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path) >> { >> struct btrfs_root *root = fs_info->dev_root; >> - struct request_queue *q; >> struct btrfs_trans_handle *trans; >> struct btrfs_device *device; >> struct block_device *bdev; >> @@ -2666,7 +2662,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path >> goto error_free_zone; >> } >> >> - q = bdev_get_queue(bdev); >> set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); >> device->generation = trans->transid; >> device->io_width = fs_info->sectorsize; >> @@ -2714,8 +2709,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 (!blk_queue_nonrot(q)) >> - fs_devices->rotating = true; >> + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); > > Same here. > >> >> orig_super_total_bytes = btrfs_super_total_bytes(fs_info->super_copy); >> btrfs_set_super_total_bytes(fs_info->super_copy, >> -- >> 2.33.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] btrfs: cleanup finding rotating device 2022-01-19 7:43 ` Anand Jain @ 2022-01-20 17:08 ` David Sterba 0 siblings, 0 replies; 7+ messages in thread From: David Sterba @ 2022-01-20 17:08 UTC (permalink / raw) To: Anand Jain; +Cc: dsterba, linux-btrfs, nborisov On Wed, Jan 19, 2022 at 03:43:42PM +0800, Anand Jain wrote: > On 18/01/2022 23:43, David Sterba wrote: > > On Thu, Jan 13, 2022 at 03:48:54PM +0800, Anand Jain wrote: > >> - q = bdev_get_queue(bdev); > >> - if (!blk_queue_nonrot(q)) > >> - fs_devices->rotating = true; > >> + fs_devices->rotating = !blk_queue_nonrot(bdev_get_queue(bdev)); > > > > This is an equivalent change, in the new code fs_devices->rotating will > > be always set according to the last device, while in the old code, any > > rotational device will set it to true and never back. > > Oh. The previous status gets overwritten based on the current device > non-rot check. This is not good. My bad. However, V1 is ok could you > pls consider that? Yes, v1 applied to misc-next, thanks. > Also, there is a bug in the original code - If we have at least one > rotating device (HDD) we won't enable SSD benefit. However, if that > HDD is deleted later on, we won't still enable the SSD status. This > bug can be fixed easily on top of the patch > > '[PATCH 1/2] btrfs: keep device type in the struct btrfs_device' > > I will wait for some comments before fixing this. Right, I think that mixed HDD/SSD devices were not considered and keeping the status should be fixed. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-20 17:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-13 4:44 [PATCH] btrfs: cleanup finding rotating device Anand Jain 2022-01-13 7:38 ` Nikolay Borisov 2022-01-13 7:48 ` [PATCH v2] " Anand Jain 2022-01-13 7:50 ` Nikolay Borisov 2022-01-18 15:43 ` David Sterba 2022-01-19 7:43 ` Anand Jain 2022-01-20 17:08 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).