diff mbox series

[02/12] btrfs: remove a bogus submit_one_bio call in read_extent_buffer_subpage

Message ID 20230216163437.2370948-3-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/12] btrfs: remove the force_bio_submit to submit_extent_page | expand

Commit Message

Christoph Hellwig Feb. 16, 2023, 4:34 p.m. UTC
The first call to submit_one_bio call in read_extent_buffer_subpage is
for a btrfs_bio_ctrl structure that has just been initialized and thus
can't have a non-NULL bio, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Johannes Thumshirn Feb. 20, 2023, 9:45 a.m. UTC | #1
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Qu Wenruo Feb. 21, 2023, 11:14 a.m. UTC | #2
On 2023/2/17 00:34, Christoph Hellwig wrote:
> The first call to submit_one_bio call in read_extent_buffer_subpage is
> for a btrfs_bio_ctrl structure that has just been initialized and thus
> can't have a non-NULL bio, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This new submit_one_bio() is mostly caused by the previous patch.

Can we just fold this one into the previous patch?

Thanks,
Qu
> ---
>   fs/btrfs/extent_io.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index b53486ed8804af..e9639128962c99 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4442,7 +4442,6 @@ static int read_extent_buffer_subpage(struct extent_buffer *eb, int wait,
>   	btrfs_subpage_clear_error(fs_info, page, eb->start, eb->len);
>   
>   	btrfs_subpage_start_reader(fs_info, page, eb->start, eb->len);
> -	submit_one_bio(&bio_ctrl);
>   	ret = submit_extent_page(REQ_OP_READ, NULL, &bio_ctrl,
>   				 eb->start, page, eb->len,
>   				 eb->start - page_offset(page), 0);
Christoph Hellwig Feb. 21, 2023, 2:31 p.m. UTC | #3
On Tue, Feb 21, 2023 at 07:14:09PM +0800, Qu Wenruo wrote:
>
>
> On 2023/2/17 00:34, Christoph Hellwig wrote:
>> The first call to submit_one_bio call in read_extent_buffer_subpage is
>> for a btrfs_bio_ctrl structure that has just been initialized and thus
>> can't have a non-NULL bio, so remove it.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> This new submit_one_bio() is mostly caused by the previous patch.
>
> Can we just fold this one into the previous patch?

I don't think mixing a change in behavior (even if it is a no-op
for the I/O pattern) into a pure refactoring is a good idea.  I've
been arguing about doing this patch first before patch 1 as I've
been expecting this argument, but the current order seems more
obvious to review.  I could switch it around if strongly preferred.
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b53486ed8804af..e9639128962c99 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4442,7 +4442,6 @@  static int read_extent_buffer_subpage(struct extent_buffer *eb, int wait,
 	btrfs_subpage_clear_error(fs_info, page, eb->start, eb->len);
 
 	btrfs_subpage_start_reader(fs_info, page, eb->start, eb->len);
-	submit_one_bio(&bio_ctrl);
 	ret = submit_extent_page(REQ_OP_READ, NULL, &bio_ctrl,
 				 eb->start, page, eb->len,
 				 eb->start - page_offset(page), 0);