diff mbox series

[13/34] btrfs: remove now unused checksumming helpers

Message ID 20230121065031.1139353-14-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/34] block: export bio_split_rw | expand

Commit Message

Christoph Hellwig Jan. 21, 2023, 6:50 a.m. UTC
Remove the unused btrfs_verify_data_csum helper, and fold
btrfs_check_data_csum into its only caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/btrfs_inode.h |   5 --
 fs/btrfs/inode.c       | 127 ++++++-----------------------------------
 2 files changed, 17 insertions(+), 115 deletions(-)

Comments

Johannes Thumshirn Jan. 23, 2023, 4:49 p.m. UTC | #1
On 21.01.23 07:51, Christoph Hellwig wrote:
> -	if (btrfs_check_data_csum(inode, bbio, bio_offset, bv->bv_page,
> -				  bv->bv_offset) < 0)
> -		return false;
> +	csum_expected = btrfs_csum_ptr(fs_info, bbio->csum, bio_offset);
> +	if (btrfs_check_sector_csum(fs_info, bv->bv_page, bv->bv_offset, csum,
> +				    csum_expected))
> +		goto zeroit;
>  	return true;

We could even go as far as that:

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c63160cf135d..9f384551b722 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3449,13 +3449,6 @@ int btrfs_check_sector_csum(struct btrfs_fs_info *fs_info, struct page *page,
        return 0;
 }
 
-static u8 *btrfs_csum_ptr(const struct btrfs_fs_info *fs_info, u8 *csums, u64 offset)
-{
-       u64 offset_in_sectors = offset >> fs_info->sectorsize_bits;
-
-       return csums + offset_in_sectors * fs_info->csum_size;
-}
-
 /*
  * btrfs_data_csum_ok - verify the checksum of single data sector
  * @bbio:      btrfs_io_bio which contains the csum
@@ -3493,7 +3486,8 @@ bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
                return true;
        }
 
-       csum_expected = btrfs_csum_ptr(fs_info, bbio->csum, bio_offset);
+       csum_expected = bbio->csum + (bio_offset >> fs_info->sectorsize_bits) *
+                               fs_info->csum_size;
        if (btrfs_check_sector_csum(fs_info, bv->bv_page, bv->bv_offset, csum,
                                    csum_expected))
                goto zeroit;

Anyways that's splitting hairs,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Christoph Hellwig Jan. 23, 2023, 4:53 p.m. UTC | #2
On Mon, Jan 23, 2023 at 04:49:30PM +0000, Johannes Thumshirn wrote:
> We could even go as far as that:

That looks nice.  Care to send an incremental patch once the series is
in?
Johannes Thumshirn Jan. 24, 2023, 8:33 a.m. UTC | #3
On 23.01.23 17:53, Christoph Hellwig wrote:
> On Mon, Jan 23, 2023 at 04:49:30PM +0000, Johannes Thumshirn wrote:
>> We could even go as far as that:
> 
> That looks nice.  Care to send an incremental patch once the series is
> in?
> 

Sure can do, but it's untested as of now. Although I just folded 
btrfs_csum_ptr() into it's only caller.
diff mbox series

Patch

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 3faabcef9898fb..99430d0ebbb5fd 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -421,13 +421,8 @@  blk_status_t btrfs_submit_bio_start_direct_io(struct btrfs_inode *inode,
 					      u64 dio_file_offset);
 int btrfs_check_sector_csum(struct btrfs_fs_info *fs_info, struct page *page,
 			    u32 pgoff, u8 *csum, const u8 * const csum_expected);
-int btrfs_check_data_csum(struct btrfs_inode *inode, struct btrfs_bio *bbio,
-			  u32 bio_offset, struct page *page, u32 pgoff);
 bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
 			u32 bio_offset, struct bio_vec *bv);
-unsigned int btrfs_verify_data_csum(struct btrfs_bio *bbio,
-				    u32 bio_offset, struct page *page,
-				    u64 start, u64 end);
 noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
 			      u64 *orig_start, u64 *orig_block_len,
 			      u64 *ram_bytes, bool nowait, bool strict);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d122bf9a72aaff..c63160cf135d98 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3456,45 +3456,6 @@  static u8 *btrfs_csum_ptr(const struct btrfs_fs_info *fs_info, u8 *csums, u64 of
 	return csums + offset_in_sectors * fs_info->csum_size;
 }
 
-/*
- * check_data_csum - verify checksum of one sector of uncompressed data
- * @inode:	inode
- * @bbio:	btrfs_bio which contains the csum
- * @bio_offset:	offset to the beginning of the bio (in bytes)
- * @page:	page where is the data to be verified
- * @pgoff:	offset inside the page
- *
- * The length of such check is always one sector size.
- *
- * When csum mismatch is detected, we will also report the error and fill the
- * corrupted range with zero. (Thus it needs the extra parameters)
- */
-int btrfs_check_data_csum(struct btrfs_inode *inode, struct btrfs_bio *bbio,
-			  u32 bio_offset, struct page *page, u32 pgoff)
-{
-	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	u32 len = fs_info->sectorsize;
-	u8 *csum_expected;
-	u8 csum[BTRFS_CSUM_SIZE];
-
-	ASSERT(pgoff + len <= PAGE_SIZE);
-
-	csum_expected = btrfs_csum_ptr(fs_info, bbio->csum, bio_offset);
-
-	if (btrfs_check_sector_csum(fs_info, page, pgoff, csum, csum_expected))
-		goto zeroit;
-	return 0;
-
-zeroit:
-	btrfs_print_data_csum_error(inode, bbio->file_offset + bio_offset,
-				    csum, csum_expected, bbio->mirror_num);
-	if (bbio->device)
-		btrfs_dev_stat_inc_and_print(bbio->device,
-					     BTRFS_DEV_STAT_CORRUPTION_ERRS);
-	memzero_page(page, pgoff, len);
-	return -EIO;
-}
-
 /*
  * btrfs_data_csum_ok - verify the checksum of single data sector
  * @bbio:	btrfs_io_bio which contains the csum
@@ -3512,8 +3473,13 @@  bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
 			u32 bio_offset, struct bio_vec *bv)
 {
 	struct btrfs_inode *inode = bbio->inode;
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	u64 file_offset = bbio->file_offset + bio_offset;
 	u64 end = file_offset + bv->bv_len - 1;
+	u8 *csum_expected;
+	u8 csum[BTRFS_CSUM_SIZE];
+
+	ASSERT(bv->bv_len == fs_info->sectorsize);
 
 	if (!bbio->csum)
 		return true;
@@ -3527,78 +3493,19 @@  bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
 		return true;
 	}
 
-	if (btrfs_check_data_csum(inode, bbio, bio_offset, bv->bv_page,
-				  bv->bv_offset) < 0)
-		return false;
+	csum_expected = btrfs_csum_ptr(fs_info, bbio->csum, bio_offset);
+	if (btrfs_check_sector_csum(fs_info, bv->bv_page, bv->bv_offset, csum,
+				    csum_expected))
+		goto zeroit;
 	return true;
-}
-
-
-/*
- * When reads are done, we need to check csums to verify the data is correct.
- * if there's a match, we allow the bio to finish.  If not, the code in
- * extent_io.c will try to find good copies for us.
- *
- * @bio_offset:	offset to the beginning of the bio (in bytes)
- * @start:	file offset of the range start
- * @end:	file offset of the range end (inclusive)
- *
- * Return a bitmap where bit set means a csum mismatch, and bit not set means
- * csum match.
- */
-unsigned int btrfs_verify_data_csum(struct btrfs_bio *bbio,
-				    u32 bio_offset, struct page *page,
-				    u64 start, u64 end)
-{
-	struct btrfs_inode *inode = BTRFS_I(page->mapping->host);
-	struct btrfs_root *root = inode->root;
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct extent_io_tree *io_tree = &inode->io_tree;
-	const u32 sectorsize = root->fs_info->sectorsize;
-	u32 pg_off;
-	unsigned int result = 0;
-
-	/*
-	 * This only happens for NODATASUM or compressed read.
-	 * Normally this should be covered by above check for compressed read
-	 * or the next check for NODATASUM.  Just do a quicker exit here.
-	 */
-	if (bbio->csum == NULL)
-		return 0;
-
-	if (inode->flags & BTRFS_INODE_NODATASUM)
-		return 0;
-
-	if (unlikely(test_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state)))
-		return 0;
-
-	ASSERT(page_offset(page) <= start &&
-	       end <= page_offset(page) + PAGE_SIZE - 1);
-	for (pg_off = offset_in_page(start);
-	     pg_off < offset_in_page(end);
-	     pg_off += sectorsize, bio_offset += sectorsize) {
-		u64 file_offset = pg_off + page_offset(page);
-		int ret;
-
-		if (btrfs_is_data_reloc_root(root) &&
-		    test_range_bit(io_tree, file_offset,
-				   file_offset + sectorsize - 1,
-				   EXTENT_NODATASUM, 1, NULL)) {
-			/* Skip the range without csum for data reloc inode */
-			clear_extent_bits(io_tree, file_offset,
-					  file_offset + sectorsize - 1,
-					  EXTENT_NODATASUM);
-			continue;
-		}
-		ret = btrfs_check_data_csum(inode, bbio, bio_offset, page, pg_off);
-		if (ret < 0) {
-			const int nr_bit = (pg_off - offset_in_page(start)) >>
-				     root->fs_info->sectorsize_bits;
-
-			result |= (1U << nr_bit);
-		}
-	}
-	return result;
+zeroit:
+	btrfs_print_data_csum_error(inode, file_offset, csum, csum_expected,
+				    bbio->mirror_num);
+	if (dev)
+		btrfs_dev_stat_inc_and_print(dev,
+					     BTRFS_DEV_STAT_CORRUPTION_ERRS);
+	memzero_bvec(bv);
+	return false;
 }
 
 /*