Re: [PATCH 2/8] btrfs: introduce a pure data checksum checking helper

Christoph Hellwig
2022-05-24T07:25:07Z

On Mon, May 23, 2022 at 08:38:13AM +0800, Qu Wenruo wrote:
>
>
> On 2022/5/22 19:47, Christoph Hellwig wrote:
>> From: Qu Wenruo <wqu@suse.com>
>>
>> Although we have several data csum verification code, we never have a
>> function really just to verify checksum for one sector.
>>
>> Function check_data_csum() do extra work for error reporting, thus it
>> requires a lot of extra things like file offset, bio_offset etc.
>>
>> Function btrfs_verify_data_csum() is even worse, it will utizlie page
>> checked flag, which means it can not be utilized for direct IO pages.
>>
>> Here we introduce a new helper, btrfs_check_sector_csum(), which really
>> only accept a sector in page, and expected checksum pointer.
>>
>> We use this function to implement check_data_csum(), and export it for
>> incoming patch.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> [hch: keep passing the csum array as an arguments, as the callers want
>> to print it, rename per request]
>
> Mind to constify the @csum_expected parameter?

This would be the incremental diff, if Dave cares deeply he can fold
it in:

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d982ea62c521b..f01ce82af8ca9 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3262,7 +3262,7 @@ u64 btrfs_file_extent_end(const struct btrfs_path *path);
void btrfs_submit_data_bio(struct inode *inode, struct bio *bio,
int mirror_num, enum btrfs_compression_type compress_type);
int btrfs_check_sector_csum(struct btrfs_fs_info *fs_info, struct page *page,
- u32 pgoff, u8 *csum, u8 *csum_expected);
+ u32 pgoff, u8 *csum, u8 * const csum_expected);
unsigned int btrfs_verify_data_csum(struct btrfs_bio *bbio,
u32 bio_offset, struct page *page,
u64 start, u64 end);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index bfc0b0035b03c..c344ed0e057ac 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3330,7 +3330,7 @@ void btrfs_writepage_endio_finish_ordered(struct btrfs_inode *inode,
* depend on the type of I/O.
*/
int btrfs_check_sector_csum(struct btrfs_fs_info *fs_info, struct page *page,
- u32 pgoff, u8 *csum, u8 *csum_expected)
+ u32 pgoff, u8 *csum, u8 * const csum_expected)
{
SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
char *kaddr;