[2/2] btrfs: cleanup btrfs_lookup_bio_sums

Commit Message

Christoph Hellwig Feb. 21, 2023, 8:56 p.m. UTC

Introduce a bio_offset variable for the current offset into the bio
instead of recalculating it over and over, remove the now superflous
search_len and reduce variable scope as applicable as well as switch
count to and unsigned type for the unsigned values it holds.

Signed-off-by: Christoph Hellwig <[e-mail]>
---
 fs/btrfs/file-item.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

Comments

Johannes Thumshirn Feb. 22, 2023, 11:57 a.m. UTC | #1

On 21.02.23 21:57, Christoph Hellwig wrote:
> +	while (bio_offset < orig_len) {
> +		u64 cur_disk_bytenr = orig_disk_bytenr + bio_offset;
> +		u64 search_len = orig_len - bio_offset;
> +		u8 *csum_dst = bbio->csum +
> +			(bio_offset >> fs_info->sectorsize_bits) * csum_size;
> +		u32 count = 0;
>  
>  		count = search_csum_tree(fs_info, path, cur_disk_bytenr,
>  					 search_len, csum_dst);

That wont work, as a) search_csum_tree() returns an 'int' and b)
count is checked to be < 0 (aka error) the line below this diff:


                count = search_csum_tree(fs_info, path, cur_disk_bytenr,                                                                                                                                                                                                                                                                                                   
                                         search_len, csum_dst);                
                if (count < 0) {                                               
                        ret = errno_to_blk_status(count);                      
                        if (bbio->csum != bbio->csum_inline)                   
                                kfree(bbio->csum);                             
                        bbio->csum = NULL;                                     
                        break;                                                 
                }           


So count has to stay 'int'.

Thanks,
	Johannes

Christoph Hellwig Feb. 22, 2023, 1:52 p.m. UTC | #2

On Wed, Feb 22, 2023 at 11:57:12AM +0000, Johannes Thumshirn wrote:
> 
> That wont work, as a) search_csum_tree() returns an 'int' and b)
> count is checked to be < 0 (aka error) the line below this diff:

Indeed.  I'll resend.

Patch

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 9df9b91dbc6463..efc914b2e17e08 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -350,10 +350,9 @@  blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 	const u32 csum_size = fs_info->csum_size;
 	u32 orig_len = bio->bi_iter.bi_size;
 	u64 orig_disk_bytenr = bio->bi_iter.bi_sector << SECTOR_SHIFT;
-	u64 cur_disk_bytenr;
 	const unsigned int nblocks = orig_len >> fs_info->sectorsize_bits;
-	int count = 0;
 	blk_status_t ret = BLK_STS_OK;
+	u32 bio_offset = 0;
 
 	if ((inode->flags & BTRFS_INODE_NODATASUM) ||
 	    test_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state))
@@ -404,25 +403,12 @@  blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 		path->skip_locking = 1;
 	}
 
-	for (cur_disk_bytenr = orig_disk_bytenr;
-	     cur_disk_bytenr < orig_disk_bytenr + orig_len;
-	     cur_disk_bytenr += (count * sectorsize)) {
-		u64 search_len = orig_disk_bytenr + orig_len - cur_disk_bytenr;
-		unsigned int sector_offset;
-		u8 *csum_dst;
-
-		/*
-		 * Although both cur_disk_bytenr and orig_disk_bytenr is u64,
-		 * we're calculating the offset to the bio start.
-		 *
-		 * Bio size is limited to UINT_MAX, thus unsigned int is large
-		 * enough to contain the raw result, not to mention the right
-		 * shifted result.
-		 */
-		ASSERT(cur_disk_bytenr - orig_disk_bytenr < UINT_MAX);
-		sector_offset = (cur_disk_bytenr - orig_disk_bytenr) >>
-				fs_info->sectorsize_bits;
-		csum_dst = bbio->csum + sector_offset * csum_size;
+	while (bio_offset < orig_len) {
+		u64 cur_disk_bytenr = orig_disk_bytenr + bio_offset;
+		u64 search_len = orig_len - bio_offset;
+		u8 *csum_dst = bbio->csum +
+			(bio_offset >> fs_info->sectorsize_bits) * csum_size;
+		u32 count = 0;
 
 		count = search_csum_tree(fs_info, path, cur_disk_bytenr,
 					 search_len, csum_dst);
@@ -451,7 +437,7 @@  blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 			if (inode->root->root_key.objectid ==
 			    BTRFS_DATA_RELOC_TREE_OBJECTID) {
 				u64 file_offset = bbio->file_offset +
-					cur_disk_bytenr - orig_disk_bytenr;
+					bio_offset;
 
 				set_extent_bits(&inode->io_tree, file_offset,
 						file_offset + sectorsize - 1,
@@ -462,6 +448,7 @@  blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 				cur_disk_bytenr, cur_disk_bytenr + sectorsize);
 			}
 		}
+		bio_offset += count * sectorsize;
 	}
 
 	btrfs_free_path(path);