[PATCH 00/10] Sectorsize, csum_size lifted to fs_info

[PATCH 00/10] Sectorsize, csum_size lifted to fs_info

[David Sterba]

Clean up usage of multiplication or division by sectorsize by shifts,
checksums per leaf are calculated once and csum_size has a copy in
fs_info so we don't have to read it from raw superblocks.

David Sterba (10):
  btrfs: use precalculated sectorsize_bits from fs_info
  btrfs: replace div_u64 by shift in free_space_bitmap_size
  btrfs: replace s_blocksize_bits with fs_info::sectorsize_bits
  btrfs: store precalculated csum_size in fs_info
  btrfs: precalculate checksums per leaf once
  btrfs: use cached value of fs_info::csum_size everywhere
  btrfs: switch cached fs_info::csum_size from u16 to u32
  btrfs: remove unnecessary local variables for checksum size
  btrfs: check integrity: remove local copy of csum_size
  btrfs: scrub: remove local copy of csum_size from context

 fs/btrfs/btrfs_inode.h     |  3 +-
 fs/btrfs/check-integrity.c |  6 +---
 fs/btrfs/compression.c     |  9 ++----
 fs/btrfs/ctree.h           |  5 +++-
 fs/btrfs/disk-io.c         | 11 ++++---
 fs/btrfs/extent-tree.c     | 11 ++-----
 fs/btrfs/extent_io.c       |  4 +--
 fs/btrfs/file-item.c       | 60 ++++++++++++++++++--------------------
 fs/btrfs/file.c            |  3 +-
 fs/btrfs/free-space-tree.c | 30 +++++++++----------
 fs/btrfs/inode.c           | 13 ++++-----
 fs/btrfs/ordered-data.c    | 11 ++++---
 fs/btrfs/ordered-data.h    |  3 +-
 fs/btrfs/scrub.c           | 31 ++++++++++----------
 fs/btrfs/super.c           |  2 +-
 fs/btrfs/tree-checker.c    |  5 ++--
 16 files changed, 95 insertions(+), 112 deletions(-)

-- 
2.25.0

[PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[David Sterba]

We do a lot of calculations where we divide or multiply by sectorsize.
We also know and make sure that sectorsize is a power of two, so this
means all divisions can be turned to shifts and avoid eg. expensive
u64/u32 divisions.

The type is u32 as it's more register friendly on x86_64 compared to u8
and the resulting assembly is smaller (movzbl vs movl).

There's also superblock s_blocksize_bits but it's usually one more
pointer dereference farther than fs_info.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h           |  1 +
 fs/btrfs/disk-io.c         |  2 ++
 fs/btrfs/extent-tree.c     |  2 +-
 fs/btrfs/file-item.c       | 11 ++++++-----
 fs/btrfs/free-space-tree.c | 12 +++++++-----
 fs/btrfs/ordered-data.c    |  3 +--
 fs/btrfs/scrub.c           | 12 ++++++------
 fs/btrfs/tree-checker.c    |  3 ++-
 8 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8a83bce3225c..87c40cc5c42e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -931,6 +931,7 @@ struct btrfs_fs_info {
 	/* Cached block sizes */
 	u32 nodesize;
 	u32 sectorsize;
+	u32 sectorsize_bits;
 	u32 stripesize;
 
 	/* Block groups and devices containing active swapfiles. */
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 601a7ab2adb4..4e67c122389c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2812,6 +2812,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
 	/* Usable values until the real ones are cached from the superblock */
 	fs_info->nodesize = 4096;
 	fs_info->sectorsize = 4096;
+	fs_info->sectorsize_bits = ilog2(4096);
 	fs_info->stripesize = 4096;
 
 	spin_lock_init(&fs_info->swapfile_pins_lock);
@@ -3076,6 +3077,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	/* Cache block sizes */
 	fs_info->nodesize = nodesize;
 	fs_info->sectorsize = sectorsize;
+	fs_info->sectorsize_bits = ilog2(sectorsize);
 	fs_info->stripesize = stripesize;
 
 	/*
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5fd60b13f4f8..29ac97248942 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2145,7 +2145,7 @@ u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
 	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
 	num_csums_per_leaf = div64_u64(csum_size,
 			(u64)btrfs_super_csum_size(fs_info->super_copy));
-	num_csums = div64_u64(csum_bytes, fs_info->sectorsize);
+	num_csums = csum_bytes >> fs_info->sectorsize_bits;
 	num_csums += num_csums_per_leaf - 1;
 	num_csums = div64_u64(num_csums, num_csums_per_leaf);
 	return num_csums;
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 816f57d52fc9..d8cd467b4e0c 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -119,7 +119,7 @@ static inline u32 max_ordered_sum_bytes(struct btrfs_fs_info *fs_info,
 {
 	u32 ncsums = (PAGE_SIZE - sizeof(struct btrfs_ordered_sum)) / csum_size;
 
-	return ncsums * fs_info->sectorsize;
+	return ncsums << fs_info->sectorsize_bits;
 }
 
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
@@ -369,7 +369,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 		 * a single leaf so it will also fit inside a u32
 		 */
 		diff = disk_bytenr - item_start_offset;
-		diff = diff / fs_info->sectorsize;
+		diff = diff >> fs_info->sectorsize_bits;
 		diff = diff * csum_size;
 		count = min_t(int, nblocks, (item_last_offset - disk_bytenr) >>
 					    inode->i_sb->s_blocksize_bits);
@@ -465,7 +465,8 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 			start = key.offset;
 
 		size = btrfs_item_size_nr(leaf, path->slots[0]);
-		csum_end = key.offset + (size / csum_size) * fs_info->sectorsize;
+		csum_end = key.offset +
+			   ((size / csum_size) >> fs_info->sectorsize_bits);
 		if (csum_end <= start) {
 			path->slots[0]++;
 			continue;
@@ -606,7 +607,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
 
 			data = kmap_atomic(bvec.bv_page);
 			crypto_shash_digest(shash, data + bvec.bv_offset
-					    + (i * fs_info->sectorsize),
+					    + (i << fs_info->sectorsize_bits),
 					    fs_info->sectorsize,
 					    sums->sums + index);
 			kunmap_atomic(data);
@@ -1020,7 +1021,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 
 	index += ins_size;
 	ins_size /= csum_size;
-	total_bytes += ins_size * fs_info->sectorsize;
+	total_bytes += ins_size << fs_info->sectorsize_bits;
 
 	btrfs_mark_buffer_dirty(path->nodes[0]);
 	if (total_bytes < sums->len) {
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 6b9faf3b0e96..f09f62e245a0 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -416,16 +416,18 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(leaf);
 	btrfs_release_path(path);
 
-	nrbits = div_u64(block_group->length, block_group->fs_info->sectorsize);
+	nrbits = block_group->length >> block_group->fs_info->sectorsize_bits;
 	start_bit = find_next_bit_le(bitmap, nrbits, 0);
 
 	while (start_bit < nrbits) {
 		end_bit = find_next_zero_bit_le(bitmap, nrbits, start_bit);
 		ASSERT(start_bit < end_bit);
 
-		key.objectid = start + start_bit * block_group->fs_info->sectorsize;
+		key.objectid = start +
+			       (start_bit << block_group->fs_info->sectorsize_bits);
 		key.type = BTRFS_FREE_SPACE_EXTENT_KEY;
-		key.offset = (end_bit - start_bit) * block_group->fs_info->sectorsize;
+		key.offset = (end_bit - start_bit) <<
+					block_group->fs_info->sectorsize_bits;
 
 		ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
 		if (ret)
@@ -540,8 +542,8 @@ static void free_space_set_bits(struct btrfs_block_group *block_group,
 		end = found_end;
 
 	ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
-	first = div_u64(*start - found_start, fs_info->sectorsize);
-	last = div_u64(end - found_start, fs_info->sectorsize);
+	first = (*start - found_start) >> fs_info->sectorsize_bits;
+	last = (end - found_start) >> fs_info->sectorsize_bits;
 	if (bit)
 		extent_buffer_bitmap_set(leaf, ptr, first, last - first);
 	else
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 87bac9ecdf4c..7b62dcc6cd98 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -868,7 +868,6 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
-	u32 sectorsize = btrfs_inode_sectorsize(inode);
 	const u8 blocksize_bits = inode->vfs_inode.i_sb->s_blocksize_bits;
 	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int index = 0;
@@ -890,7 +889,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 			index += (int)num_sectors * csum_size;
 			if (index == len)
 				goto out;
-			disk_bytenr += num_sectors * sectorsize;
+			disk_bytenr += num_sectors << fs_info->sectorsize_bits;
 		}
 	}
 out:
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 54a4f34d4c1c..7babf670c8c2 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2300,7 +2300,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
 	u64 offset;
 	u64 nsectors64;
 	u32 nsectors;
-	int sectorsize = sparity->sctx->fs_info->sectorsize;
+	u32 sectorsize_bits = sparity->sctx->fs_info->sectorsize_bits;
 
 	if (len >= sparity->stripe_len) {
 		bitmap_set(bitmap, 0, sparity->nsectors);
@@ -2309,8 +2309,8 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
 
 	start -= sparity->logic_start;
 	start = div64_u64_rem(start, sparity->stripe_len, &offset);
-	offset = div_u64(offset, sectorsize);
-	nsectors64 = div_u64(len, sectorsize);
+	offset = offset >> sectorsize_bits;
+	nsectors64 = len >> sectorsize_bits;
 
 	ASSERT(nsectors64 < UINT_MAX);
 	nsectors = (u32)nsectors64;
@@ -2386,10 +2386,10 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
 	if (!sum)
 		return 0;
 
-	index = div_u64(logical - sum->bytenr, sctx->fs_info->sectorsize);
+	index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
 	ASSERT(index < UINT_MAX);
 
-	num_sectors = sum->len / sctx->fs_info->sectorsize;
+	num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
 	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
 	if (index == num_sectors - 1) {
 		list_del(&sum->list);
@@ -2776,7 +2776,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 	int extent_mirror_num;
 	int stop_loop = 0;
 
-	nsectors = div_u64(map->stripe_len, fs_info->sectorsize);
+	nsectors = map->stripe_len >> fs_info->sectorsize_bits;
 	bitmap_len = scrub_calc_parity_bitmap_len(nsectors);
 	sparity = kzalloc(sizeof(struct scrub_parity) + 2 * bitmap_len,
 			  GFP_NOFS);
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 8784b74f5232..c0e19917e59b 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -361,7 +361,8 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
 		u32 prev_item_size;
 
 		prev_item_size = btrfs_item_size_nr(leaf, slot - 1);
-		prev_csum_end = (prev_item_size / csumsize) * sectorsize;
+		prev_csum_end = (prev_item_size / csumsize);
+		prev_csum_end <<= fs_info->sectorsize_bits;
 		prev_csum_end += prev_key->offset;
 		if (prev_csum_end > key->offset) {
 			generic_err(leaf, slot - 1,
-- 
2.25.0

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 10511 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> We do a lot of calculations where we divide or multiply by sectorsize.
> We also know and make sure that sectorsize is a power of two, so this
> means all divisions can be turned to shifts and avoid eg. expensive
> u64/u32 divisions.
> 
> The type is u32 as it's more register friendly on x86_64 compared to u8
> and the resulting assembly is smaller (movzbl vs movl).
> 
> There's also superblock s_blocksize_bits but it's usually one more
> pointer dereference farther than fs_info.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h           |  1 +
>  fs/btrfs/disk-io.c         |  2 ++
>  fs/btrfs/extent-tree.c     |  2 +-
>  fs/btrfs/file-item.c       | 11 ++++++-----
>  fs/btrfs/free-space-tree.c | 12 +++++++-----
>  fs/btrfs/ordered-data.c    |  3 +--
>  fs/btrfs/scrub.c           | 12 ++++++------
>  fs/btrfs/tree-checker.c    |  3 ++-
>  8 files changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 8a83bce3225c..87c40cc5c42e 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -931,6 +931,7 @@ struct btrfs_fs_info {
>  	/* Cached block sizes */
>  	u32 nodesize;
>  	u32 sectorsize;
> +	u32 sectorsize_bits;

For the bit shift, it can alwasy be contained in one u8.
Since one u32 is only to be at most 32 bits, it can be easily contained
in u8 whose max value is 255.

This should allow us to pack several u8 together to reduce some memory
usage.

Despite this, the series is pretty good.

Thanks,
Qu
>  	u32 stripesize;
>  
>  	/* Block groups and devices containing active swapfiles. */
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 601a7ab2adb4..4e67c122389c 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2812,6 +2812,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
>  	/* Usable values until the real ones are cached from the superblock */
>  	fs_info->nodesize = 4096;
>  	fs_info->sectorsize = 4096;
> +	fs_info->sectorsize_bits = ilog2(4096);
>  	fs_info->stripesize = 4096;
>  
>  	spin_lock_init(&fs_info->swapfile_pins_lock);
> @@ -3076,6 +3077,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  	/* Cache block sizes */
>  	fs_info->nodesize = nodesize;
>  	fs_info->sectorsize = sectorsize;
> +	fs_info->sectorsize_bits = ilog2(sectorsize);
>  	fs_info->stripesize = stripesize;
>  
>  	/*
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5fd60b13f4f8..29ac97248942 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2145,7 +2145,7 @@ u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
>  	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
>  	num_csums_per_leaf = div64_u64(csum_size,
>  			(u64)btrfs_super_csum_size(fs_info->super_copy));
> -	num_csums = div64_u64(csum_bytes, fs_info->sectorsize);
> +	num_csums = csum_bytes >> fs_info->sectorsize_bits;
>  	num_csums += num_csums_per_leaf - 1;
>  	num_csums = div64_u64(num_csums, num_csums_per_leaf);
>  	return num_csums;
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 816f57d52fc9..d8cd467b4e0c 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -119,7 +119,7 @@ static inline u32 max_ordered_sum_bytes(struct btrfs_fs_info *fs_info,
>  {
>  	u32 ncsums = (PAGE_SIZE - sizeof(struct btrfs_ordered_sum)) / csum_size;
>  
> -	return ncsums * fs_info->sectorsize;
> +	return ncsums << fs_info->sectorsize_bits;
>  }
>  
>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
> @@ -369,7 +369,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>  		 * a single leaf so it will also fit inside a u32
>  		 */
>  		diff = disk_bytenr - item_start_offset;
> -		diff = diff / fs_info->sectorsize;
> +		diff = diff >> fs_info->sectorsize_bits;
>  		diff = diff * csum_size;
>  		count = min_t(int, nblocks, (item_last_offset - disk_bytenr) >>
>  					    inode->i_sb->s_blocksize_bits);
> @@ -465,7 +465,8 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  			start = key.offset;
>  
>  		size = btrfs_item_size_nr(leaf, path->slots[0]);
> -		csum_end = key.offset + (size / csum_size) * fs_info->sectorsize;
> +		csum_end = key.offset +
> +			   ((size / csum_size) >> fs_info->sectorsize_bits);
>  		if (csum_end <= start) {
>  			path->slots[0]++;
>  			continue;
> @@ -606,7 +607,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
>  
>  			data = kmap_atomic(bvec.bv_page);
>  			crypto_shash_digest(shash, data + bvec.bv_offset
> -					    + (i * fs_info->sectorsize),
> +					    + (i << fs_info->sectorsize_bits),
>  					    fs_info->sectorsize,
>  					    sums->sums + index);
>  			kunmap_atomic(data);
> @@ -1020,7 +1021,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  
>  	index += ins_size;
>  	ins_size /= csum_size;
> -	total_bytes += ins_size * fs_info->sectorsize;
> +	total_bytes += ins_size << fs_info->sectorsize_bits;
>  
>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>  	if (total_bytes < sums->len) {
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index 6b9faf3b0e96..f09f62e245a0 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -416,16 +416,18 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
>  	btrfs_mark_buffer_dirty(leaf);
>  	btrfs_release_path(path);
>  
> -	nrbits = div_u64(block_group->length, block_group->fs_info->sectorsize);
> +	nrbits = block_group->length >> block_group->fs_info->sectorsize_bits;
>  	start_bit = find_next_bit_le(bitmap, nrbits, 0);
>  
>  	while (start_bit < nrbits) {
>  		end_bit = find_next_zero_bit_le(bitmap, nrbits, start_bit);
>  		ASSERT(start_bit < end_bit);
>  
> -		key.objectid = start + start_bit * block_group->fs_info->sectorsize;
> +		key.objectid = start +
> +			       (start_bit << block_group->fs_info->sectorsize_bits);
>  		key.type = BTRFS_FREE_SPACE_EXTENT_KEY;
> -		key.offset = (end_bit - start_bit) * block_group->fs_info->sectorsize;
> +		key.offset = (end_bit - start_bit) <<
> +					block_group->fs_info->sectorsize_bits;
>  
>  		ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
>  		if (ret)
> @@ -540,8 +542,8 @@ static void free_space_set_bits(struct btrfs_block_group *block_group,
>  		end = found_end;
>  
>  	ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
> -	first = div_u64(*start - found_start, fs_info->sectorsize);
> -	last = div_u64(end - found_start, fs_info->sectorsize);
> +	first = (*start - found_start) >> fs_info->sectorsize_bits;
> +	last = (end - found_start) >> fs_info->sectorsize_bits;
>  	if (bit)
>  		extent_buffer_bitmap_set(leaf, ptr, first, last - first);
>  	else
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 87bac9ecdf4c..7b62dcc6cd98 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -868,7 +868,6 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>  	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
>  	unsigned long num_sectors;
>  	unsigned long i;
> -	u32 sectorsize = btrfs_inode_sectorsize(inode);
>  	const u8 blocksize_bits = inode->vfs_inode.i_sb->s_blocksize_bits;
>  	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  	int index = 0;
> @@ -890,7 +889,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>  			index += (int)num_sectors * csum_size;
>  			if (index == len)
>  				goto out;
> -			disk_bytenr += num_sectors * sectorsize;
> +			disk_bytenr += num_sectors << fs_info->sectorsize_bits;
>  		}
>  	}
>  out:
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 54a4f34d4c1c..7babf670c8c2 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2300,7 +2300,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
>  	u64 offset;
>  	u64 nsectors64;
>  	u32 nsectors;
> -	int sectorsize = sparity->sctx->fs_info->sectorsize;
> +	u32 sectorsize_bits = sparity->sctx->fs_info->sectorsize_bits;
>  
>  	if (len >= sparity->stripe_len) {
>  		bitmap_set(bitmap, 0, sparity->nsectors);
> @@ -2309,8 +2309,8 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
>  
>  	start -= sparity->logic_start;
>  	start = div64_u64_rem(start, sparity->stripe_len, &offset);
> -	offset = div_u64(offset, sectorsize);
> -	nsectors64 = div_u64(len, sectorsize);
> +	offset = offset >> sectorsize_bits;
> +	nsectors64 = len >> sectorsize_bits;
>  
>  	ASSERT(nsectors64 < UINT_MAX);
>  	nsectors = (u32)nsectors64;
> @@ -2386,10 +2386,10 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
>  	if (!sum)
>  		return 0;
>  
> -	index = div_u64(logical - sum->bytenr, sctx->fs_info->sectorsize);
> +	index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
>  	ASSERT(index < UINT_MAX);
>  
> -	num_sectors = sum->len / sctx->fs_info->sectorsize;
> +	num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
>  	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
>  	if (index == num_sectors - 1) {
>  		list_del(&sum->list);
> @@ -2776,7 +2776,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>  	int extent_mirror_num;
>  	int stop_loop = 0;
>  
> -	nsectors = div_u64(map->stripe_len, fs_info->sectorsize);
> +	nsectors = map->stripe_len >> fs_info->sectorsize_bits;
>  	bitmap_len = scrub_calc_parity_bitmap_len(nsectors);
>  	sparity = kzalloc(sizeof(struct scrub_parity) + 2 * bitmap_len,
>  			  GFP_NOFS);
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 8784b74f5232..c0e19917e59b 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -361,7 +361,8 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
>  		u32 prev_item_size;
>  
>  		prev_item_size = btrfs_item_size_nr(leaf, slot - 1);
> -		prev_csum_end = (prev_item_size / csumsize) * sectorsize;
> +		prev_csum_end = (prev_item_size / csumsize);
> +		prev_csum_end <<= fs_info->sectorsize_bits;
>  		prev_csum_end += prev_key->offset;
>  		if (prev_csum_end > key->offset) {
>  			generic_err(leaf, slot - 1,
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 11049 bytes --]



On 2020/11/2 下午10:05, Qu Wenruo wrote:
> 
> 
> On 2020/10/29 下午10:27, David Sterba wrote:
>> We do a lot of calculations where we divide or multiply by sectorsize.
>> We also know and make sure that sectorsize is a power of two, so this
>> means all divisions can be turned to shifts and avoid eg. expensive
>> u64/u32 divisions.
>>
>> The type is u32 as it's more register friendly on x86_64 compared to u8
>> and the resulting assembly is smaller (movzbl vs movl).
>>
>> There's also superblock s_blocksize_bits but it's usually one more
>> pointer dereference farther than fs_info.
>>
>> Signed-off-by: David Sterba <dsterba@suse.com>
>> ---
>>  fs/btrfs/ctree.h           |  1 +
>>  fs/btrfs/disk-io.c         |  2 ++
>>  fs/btrfs/extent-tree.c     |  2 +-
>>  fs/btrfs/file-item.c       | 11 ++++++-----
>>  fs/btrfs/free-space-tree.c | 12 +++++++-----
>>  fs/btrfs/ordered-data.c    |  3 +--
>>  fs/btrfs/scrub.c           | 12 ++++++------
>>  fs/btrfs/tree-checker.c    |  3 ++-
>>  8 files changed, 26 insertions(+), 20 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 8a83bce3225c..87c40cc5c42e 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -931,6 +931,7 @@ struct btrfs_fs_info {
>>  	/* Cached block sizes */
>>  	u32 nodesize;
>>  	u32 sectorsize;
>> +	u32 sectorsize_bits;
> 
> For the bit shift, it can alwasy be contained in one u8.
> Since one u32 is only to be at most 32 bits, it can be easily contained
> in u8 whose max value is 255.
> 
> This should allow us to pack several u8 together to reduce some memory
> usage.
> 
> Despite this, the series is pretty good.
> 
> Thanks,
> Qu
>>  	u32 stripesize;
>>  
>>  	/* Block groups and devices containing active swapfiles. */
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 601a7ab2adb4..4e67c122389c 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2812,6 +2812,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
>>  	/* Usable values until the real ones are cached from the superblock */
>>  	fs_info->nodesize = 4096;
>>  	fs_info->sectorsize = 4096;
>> +	fs_info->sectorsize_bits = ilog2(4096);

This may sounds like a nitpicking, but what about "ffs(4096) - 1"?
IMHO it should be a little more faster than ilog2, especially when we
have ensure all sector size is power of 2 already.

>>  	fs_info->stripesize = 4096;
>>  
>>  	spin_lock_init(&fs_info->swapfile_pins_lock);
>> @@ -3076,6 +3077,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>>  	/* Cache block sizes */
>>  	fs_info->nodesize = nodesize;
>>  	fs_info->sectorsize = sectorsize;
>> +	fs_info->sectorsize_bits = ilog2(sectorsize);

Same here.

Thanks,
Qu
>>  	fs_info->stripesize = stripesize;
>>  
>>  	/*
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 5fd60b13f4f8..29ac97248942 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -2145,7 +2145,7 @@ u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
>>  	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
>>  	num_csums_per_leaf = div64_u64(csum_size,
>>  			(u64)btrfs_super_csum_size(fs_info->super_copy));
>> -	num_csums = div64_u64(csum_bytes, fs_info->sectorsize);
>> +	num_csums = csum_bytes >> fs_info->sectorsize_bits;
>>  	num_csums += num_csums_per_leaf - 1;
>>  	num_csums = div64_u64(num_csums, num_csums_per_leaf);
>>  	return num_csums;
>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>> index 816f57d52fc9..d8cd467b4e0c 100644
>> --- a/fs/btrfs/file-item.c
>> +++ b/fs/btrfs/file-item.c
>> @@ -119,7 +119,7 @@ static inline u32 max_ordered_sum_bytes(struct btrfs_fs_info *fs_info,
>>  {
>>  	u32 ncsums = (PAGE_SIZE - sizeof(struct btrfs_ordered_sum)) / csum_size;
>>  
>> -	return ncsums * fs_info->sectorsize;
>> +	return ncsums << fs_info->sectorsize_bits;
>>  }
>>  
>>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
>> @@ -369,7 +369,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>>  		 * a single leaf so it will also fit inside a u32
>>  		 */
>>  		diff = disk_bytenr - item_start_offset;
>> -		diff = diff / fs_info->sectorsize;
>> +		diff = diff >> fs_info->sectorsize_bits;
>>  		diff = diff * csum_size;
>>  		count = min_t(int, nblocks, (item_last_offset - disk_bytenr) >>
>>  					    inode->i_sb->s_blocksize_bits);
>> @@ -465,7 +465,8 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>>  			start = key.offset;
>>  
>>  		size = btrfs_item_size_nr(leaf, path->slots[0]);
>> -		csum_end = key.offset + (size / csum_size) * fs_info->sectorsize;
>> +		csum_end = key.offset +
>> +			   ((size / csum_size) >> fs_info->sectorsize_bits);
>>  		if (csum_end <= start) {
>>  			path->slots[0]++;
>>  			continue;
>> @@ -606,7 +607,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
>>  
>>  			data = kmap_atomic(bvec.bv_page);
>>  			crypto_shash_digest(shash, data + bvec.bv_offset
>> -					    + (i * fs_info->sectorsize),
>> +					    + (i << fs_info->sectorsize_bits),
>>  					    fs_info->sectorsize,
>>  					    sums->sums + index);
>>  			kunmap_atomic(data);
>> @@ -1020,7 +1021,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>>  
>>  	index += ins_size;
>>  	ins_size /= csum_size;
>> -	total_bytes += ins_size * fs_info->sectorsize;
>> +	total_bytes += ins_size << fs_info->sectorsize_bits;
>>  
>>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>>  	if (total_bytes < sums->len) {
>> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
>> index 6b9faf3b0e96..f09f62e245a0 100644
>> --- a/fs/btrfs/free-space-tree.c
>> +++ b/fs/btrfs/free-space-tree.c
>> @@ -416,16 +416,18 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
>>  	btrfs_mark_buffer_dirty(leaf);
>>  	btrfs_release_path(path);
>>  
>> -	nrbits = div_u64(block_group->length, block_group->fs_info->sectorsize);
>> +	nrbits = block_group->length >> block_group->fs_info->sectorsize_bits;
>>  	start_bit = find_next_bit_le(bitmap, nrbits, 0);
>>  
>>  	while (start_bit < nrbits) {
>>  		end_bit = find_next_zero_bit_le(bitmap, nrbits, start_bit);
>>  		ASSERT(start_bit < end_bit);
>>  
>> -		key.objectid = start + start_bit * block_group->fs_info->sectorsize;
>> +		key.objectid = start +
>> +			       (start_bit << block_group->fs_info->sectorsize_bits);
>>  		key.type = BTRFS_FREE_SPACE_EXTENT_KEY;
>> -		key.offset = (end_bit - start_bit) * block_group->fs_info->sectorsize;
>> +		key.offset = (end_bit - start_bit) <<
>> +					block_group->fs_info->sectorsize_bits;
>>  
>>  		ret = btrfs_insert_empty_item(trans, root, path, &key, 0);
>>  		if (ret)
>> @@ -540,8 +542,8 @@ static void free_space_set_bits(struct btrfs_block_group *block_group,
>>  		end = found_end;
>>  
>>  	ptr = btrfs_item_ptr_offset(leaf, path->slots[0]);
>> -	first = div_u64(*start - found_start, fs_info->sectorsize);
>> -	last = div_u64(end - found_start, fs_info->sectorsize);
>> +	first = (*start - found_start) >> fs_info->sectorsize_bits;
>> +	last = (end - found_start) >> fs_info->sectorsize_bits;
>>  	if (bit)
>>  		extent_buffer_bitmap_set(leaf, ptr, first, last - first);
>>  	else
>> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
>> index 87bac9ecdf4c..7b62dcc6cd98 100644
>> --- a/fs/btrfs/ordered-data.c
>> +++ b/fs/btrfs/ordered-data.c
>> @@ -868,7 +868,6 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>>  	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
>>  	unsigned long num_sectors;
>>  	unsigned long i;
>> -	u32 sectorsize = btrfs_inode_sectorsize(inode);
>>  	const u8 blocksize_bits = inode->vfs_inode.i_sb->s_blocksize_bits;
>>  	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>>  	int index = 0;
>> @@ -890,7 +889,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>>  			index += (int)num_sectors * csum_size;
>>  			if (index == len)
>>  				goto out;
>> -			disk_bytenr += num_sectors * sectorsize;
>> +			disk_bytenr += num_sectors << fs_info->sectorsize_bits;
>>  		}
>>  	}
>>  out:
>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>> index 54a4f34d4c1c..7babf670c8c2 100644
>> --- a/fs/btrfs/scrub.c
>> +++ b/fs/btrfs/scrub.c
>> @@ -2300,7 +2300,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
>>  	u64 offset;
>>  	u64 nsectors64;
>>  	u32 nsectors;
>> -	int sectorsize = sparity->sctx->fs_info->sectorsize;
>> +	u32 sectorsize_bits = sparity->sctx->fs_info->sectorsize_bits;
>>  
>>  	if (len >= sparity->stripe_len) {
>>  		bitmap_set(bitmap, 0, sparity->nsectors);
>> @@ -2309,8 +2309,8 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity,
>>  
>>  	start -= sparity->logic_start;
>>  	start = div64_u64_rem(start, sparity->stripe_len, &offset);
>> -	offset = div_u64(offset, sectorsize);
>> -	nsectors64 = div_u64(len, sectorsize);
>> +	offset = offset >> sectorsize_bits;
>> +	nsectors64 = len >> sectorsize_bits;
>>  
>>  	ASSERT(nsectors64 < UINT_MAX);
>>  	nsectors = (u32)nsectors64;
>> @@ -2386,10 +2386,10 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
>>  	if (!sum)
>>  		return 0;
>>  
>> -	index = div_u64(logical - sum->bytenr, sctx->fs_info->sectorsize);
>> +	index = (logical - sum->bytenr) >> sctx->fs_info->sectorsize_bits;
>>  	ASSERT(index < UINT_MAX);
>>  
>> -	num_sectors = sum->len / sctx->fs_info->sectorsize;
>> +	num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
>>  	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
>>  	if (index == num_sectors - 1) {
>>  		list_del(&sum->list);
>> @@ -2776,7 +2776,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>>  	int extent_mirror_num;
>>  	int stop_loop = 0;
>>  
>> -	nsectors = div_u64(map->stripe_len, fs_info->sectorsize);
>> +	nsectors = map->stripe_len >> fs_info->sectorsize_bits;
>>  	bitmap_len = scrub_calc_parity_bitmap_len(nsectors);
>>  	sparity = kzalloc(sizeof(struct scrub_parity) + 2 * bitmap_len,
>>  			  GFP_NOFS);
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 8784b74f5232..c0e19917e59b 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -361,7 +361,8 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
>>  		u32 prev_item_size;
>>  
>>  		prev_item_size = btrfs_item_size_nr(leaf, slot - 1);
>> -		prev_csum_end = (prev_item_size / csumsize) * sectorsize;
>> +		prev_csum_end = (prev_item_size / csumsize);
>> +		prev_csum_end <<= fs_info->sectorsize_bits;
>>  		prev_csum_end += prev_key->offset;
>>  		if (prev_csum_end > key->offset) {
>>  			generic_err(leaf, slot - 1,
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[Johannes Thumshirn]

On 02/11/2020 15:20, Qu Wenruo wrote:
> This may sounds like a nitpicking, but what about "ffs(4096) - 1"?
> IMHO it should be a little more faster than ilog2, especially when we
> have ensure all sector size is power of 2 already.

Looking at the actual ilog2() implementation (and considering you're 
passing 4096, a constant) you'll end up in const_ilog() which will 
evaluate to 9.

ffs() on the other hand on x86_64 will evaluate to a bfsl. So ffs() will
evaluated at runtime, while ilog2() at compile time.

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[David Sterba]

On Mon, Nov 02, 2020 at 02:31:35PM +0000, Johannes Thumshirn wrote:
> On 02/11/2020 15:20, Qu Wenruo wrote:
> > This may sounds like a nitpicking, but what about "ffs(4096) - 1"?
> > IMHO it should be a little more faster than ilog2, especially when we
> > have ensure all sector size is power of 2 already.
> 
> Looking at the actual ilog2() implementation (and considering you're 
> passing 4096, a constant) you'll end up in const_ilog() which will 
> evaluate to 9.
> 
> ffs() on the other hand on x86_64 will evaluate to a bfsl. So ffs() will
> evaluated at runtime, while ilog2() at compile time.

As the value is calculated only once for the whole filesystem lifetime,
I'm not concerned about speed but readability.

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[Johannes Thumshirn]

On 02/11/2020 16:10, David Sterba wrote:
> As the value is calculated only once for the whole filesystem lifetime,
> I'm not concerned about speed but readability.
> 

I wonder if a plain 9 with a comment isn't the most readable? ilog2() is pretty
readable as well though.

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[David Sterba]

On Mon, Nov 02, 2020 at 10:05:54PM +0800, Qu Wenruo wrote:
> On 2020/10/29 下午10:27, David Sterba wrote:
> > We do a lot of calculations where we divide or multiply by sectorsize.
> > We also know and make sure that sectorsize is a power of two, so this
> > means all divisions can be turned to shifts and avoid eg. expensive
> > u64/u32 divisions.
> > 
> > The type is u32 as it's more register friendly on x86_64 compared to u8
> > and the resulting assembly is smaller (movzbl vs movl).
> > 
> > There's also superblock s_blocksize_bits but it's usually one more
> > pointer dereference farther than fs_info.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/ctree.h           |  1 +
> >  fs/btrfs/disk-io.c         |  2 ++
> >  fs/btrfs/extent-tree.c     |  2 +-
> >  fs/btrfs/file-item.c       | 11 ++++++-----
> >  fs/btrfs/free-space-tree.c | 12 +++++++-----
> >  fs/btrfs/ordered-data.c    |  3 +--
> >  fs/btrfs/scrub.c           | 12 ++++++------
> >  fs/btrfs/tree-checker.c    |  3 ++-
> >  8 files changed, 26 insertions(+), 20 deletions(-)
> > 
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index 8a83bce3225c..87c40cc5c42e 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -931,6 +931,7 @@ struct btrfs_fs_info {
> >  	/* Cached block sizes */
> >  	u32 nodesize;
> >  	u32 sectorsize;
> > +	u32 sectorsize_bits;
> 
> For the bit shift, it can alwasy be contained in one u8.
> Since one u32 is only to be at most 32 bits, it can be easily contained
> in u8 whose max value is 255.
> 
> This should allow us to pack several u8 together to reduce some memory
> usage.

As mentioned in the changelog, u32 generates slightly better assembly.
The size increase is minimal compared to overall fs_info size so I'd
become worried once we have a lot of u32 in place of u8 that would span
more cachelines than necessary. Reorganizing fs_info for better
cacheline access patterns would be interesting but at the moment we
leave it up to the CPU.

Re: [PATCH 01/10] btrfs: use precalculated sectorsize_bits from fs_info

[David Sterba]

I got some comments,

On Thu, Oct 29, 2020 at 03:27:37PM +0100, David Sterba wrote:
> -	return ncsums * fs_info->sectorsize;
> +	return ncsums << fs_info->sectorsize_bits;

I'll restore all multiplications back, the shifts make it slightly worse
to read

> @@ -465,7 +465,8 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  			start = key.offset;
>  
>  		size = btrfs_item_size_nr(leaf, path->slots[0]);
> -		csum_end = key.offset + (size / csum_size) * fs_info->sectorsize;
> +		csum_end = key.offset +
> +			   ((size / csum_size) >> fs_info->sectorsize_bits);

And this was buggy as I've been told, shift wrong way

[PATCH 02/10] btrfs: replace div_u64 by shift in free_space_bitmap_size

[David Sterba]

Change free_space_bitmap_size to take btrfs_fs_info so we can get the
sectorsize_bits to do calculations.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/free-space-tree.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index f09f62e245a0..0f6a2ee6f235 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -136,9 +136,10 @@ static int btrfs_search_prev_slot(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
-static inline u32 free_space_bitmap_size(u64 size, u32 sectorsize)
+static inline u32 free_space_bitmap_size(const struct btrfs_fs_info *fs_info,
+					 u64 size)
 {
-	return DIV_ROUND_UP((u32)div_u64(size, sectorsize), BITS_PER_BYTE);
+	return DIV_ROUND_UP(size >> fs_info->sectorsize_bits, BITS_PER_BYTE);
 }
 
 static unsigned long *alloc_bitmap(u32 bitmap_size)
@@ -200,8 +201,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
 	int done = 0, nr;
 	int ret;
 
-	bitmap_size = free_space_bitmap_size(block_group->length,
-					     fs_info->sectorsize);
+	bitmap_size = free_space_bitmap_size(fs_info, block_group->length);
 	bitmap = alloc_bitmap(bitmap_size);
 	if (!bitmap) {
 		ret = -ENOMEM;
@@ -290,8 +290,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
 		u32 data_size;
 
 		extent_size = min(end - i, bitmap_range);
-		data_size = free_space_bitmap_size(extent_size,
-						   fs_info->sectorsize);
+		data_size = free_space_bitmap_size(fs_info, extent_size);
 
 		key.objectid = i;
 		key.type = BTRFS_FREE_SPACE_BITMAP_KEY;
@@ -339,8 +338,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
 	int done = 0, nr;
 	int ret;
 
-	bitmap_size = free_space_bitmap_size(block_group->length,
-					     fs_info->sectorsize);
+	bitmap_size = free_space_bitmap_size(fs_info, block_group->length);
 	bitmap = alloc_bitmap(bitmap_size);
 	if (!bitmap) {
 		ret = -ENOMEM;
@@ -383,8 +381,8 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
 						     fs_info->sectorsize *
 						     BITS_PER_BYTE);
 				bitmap_cursor = ((char *)bitmap) + bitmap_pos;
-				data_size = free_space_bitmap_size(found_key.offset,
-								   fs_info->sectorsize);
+				data_size = free_space_bitmap_size(fs_info,
+								found_key.offset);
 
 				ptr = btrfs_item_ptr_offset(leaf, path->slots[0] - 1);
 				read_extent_buffer(leaf, bitmap_cursor, ptr,
-- 
2.25.0

Re: [PATCH 02/10] btrfs: replace div_u64 by shift in free_space_bitmap_size

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2781 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> Change free_space_bitmap_size to take btrfs_fs_info so we can get the
> sectorsize_bits to do calculations.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>  fs/btrfs/free-space-tree.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index f09f62e245a0..0f6a2ee6f235 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -136,9 +136,10 @@ static int btrfs_search_prev_slot(struct btrfs_trans_handle *trans,
>  	return 0;
>  }
>  
> -static inline u32 free_space_bitmap_size(u64 size, u32 sectorsize)
> +static inline u32 free_space_bitmap_size(const struct btrfs_fs_info *fs_info,
> +					 u64 size)
>  {
> -	return DIV_ROUND_UP((u32)div_u64(size, sectorsize), BITS_PER_BYTE);
> +	return DIV_ROUND_UP(size >> fs_info->sectorsize_bits, BITS_PER_BYTE);
>  }
>  
>  static unsigned long *alloc_bitmap(u32 bitmap_size)
> @@ -200,8 +201,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
>  	int done = 0, nr;
>  	int ret;
>  
> -	bitmap_size = free_space_bitmap_size(block_group->length,
> -					     fs_info->sectorsize);
> +	bitmap_size = free_space_bitmap_size(fs_info, block_group->length);
>  	bitmap = alloc_bitmap(bitmap_size);
>  	if (!bitmap) {
>  		ret = -ENOMEM;
> @@ -290,8 +290,7 @@ int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
>  		u32 data_size;
>  
>  		extent_size = min(end - i, bitmap_range);
> -		data_size = free_space_bitmap_size(extent_size,
> -						   fs_info->sectorsize);
> +		data_size = free_space_bitmap_size(fs_info, extent_size);
>  
>  		key.objectid = i;
>  		key.type = BTRFS_FREE_SPACE_BITMAP_KEY;
> @@ -339,8 +338,7 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
>  	int done = 0, nr;
>  	int ret;
>  
> -	bitmap_size = free_space_bitmap_size(block_group->length,
> -					     fs_info->sectorsize);
> +	bitmap_size = free_space_bitmap_size(fs_info, block_group->length);
>  	bitmap = alloc_bitmap(bitmap_size);
>  	if (!bitmap) {
>  		ret = -ENOMEM;
> @@ -383,8 +381,8 @@ int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
>  						     fs_info->sectorsize *
>  						     BITS_PER_BYTE);
>  				bitmap_cursor = ((char *)bitmap) + bitmap_pos;
> -				data_size = free_space_bitmap_size(found_key.offset,
> -								   fs_info->sectorsize);
> +				data_size = free_space_bitmap_size(fs_info,
> +								found_key.offset);
>  
>  				ptr = btrfs_item_ptr_offset(leaf, path->slots[0] - 1);
>  				read_extent_buffer(leaf, bitmap_cursor, ptr,
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 03/10] btrfs: replace s_blocksize_bits with fs_info::sectorsize_bits

[David Sterba]

The value of super_block::s_blocksize_bits is the same as
fs_info::sectorsize_bits, but we don't need to do the extra dereferences
in many functions and storing the bits as u32 (in fs_info) generates
shorter assembly.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h        |  2 +-
 fs/btrfs/extent_io.c    |  2 +-
 fs/btrfs/file-item.c    | 34 +++++++++++++++-------------------
 fs/btrfs/file.c         |  3 +--
 fs/btrfs/inode.c        |  6 +++---
 fs/btrfs/ordered-data.c |  6 +++---
 fs/btrfs/super.c        |  2 +-
 7 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 87c40cc5c42e..a1a0b99c3530 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1405,7 +1405,7 @@ struct btrfs_map_token {
 };
 
 #define BTRFS_BYTES_TO_BLKS(fs_info, bytes) \
-				((bytes) >> (fs_info)->sb->s_blocksize_bits)
+				((bytes) >> (fs_info)->sectorsize_bits)
 
 static inline void btrfs_init_map_token(struct btrfs_map_token *token,
 					struct extent_buffer *eb)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 14d01b76f5c9..cd27a2a4f717 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2652,7 +2652,7 @@ blk_status_t btrfs_submit_read_repair(struct inode *inode,
 	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	struct extent_io_tree *failure_tree = &BTRFS_I(inode)->io_failure_tree;
 	struct btrfs_io_bio *failed_io_bio = btrfs_io_bio(failed_bio);
-	const int icsum = phy_offset >> inode->i_sb->s_blocksize_bits;
+	const int icsum = phy_offset >> fs_info->sectorsize_bits;
 	bool need_validation;
 	struct bio *repair_bio;
 	struct btrfs_io_bio *repair_io_bio;
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index d8cd467b4e0c..ed750dd8a115 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -201,7 +201,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans,
 			goto fail;
 
 		csum_offset = (bytenr - found_key.offset) >>
-				fs_info->sb->s_blocksize_bits;
+				fs_info->sectorsize_bits;
 		csums_in_item = btrfs_item_size_nr(leaf, path->slots[0]);
 		csums_in_item /= csum_size;
 
@@ -279,7 +279,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 	if (!path)
 		return BLK_STS_RESOURCE;
 
-	nblocks = bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
+	nblocks = bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
 	if (!dst) {
 		struct btrfs_io_bio *btrfs_bio = btrfs_io_bio(bio);
 
@@ -372,7 +372,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 		diff = diff >> fs_info->sectorsize_bits;
 		diff = diff * csum_size;
 		count = min_t(int, nblocks, (item_last_offset - disk_bytenr) >>
-					    inode->i_sb->s_blocksize_bits);
+					    fs_info->sectorsize_bits);
 		read_extent_buffer(path->nodes[0], csum,
 				   ((unsigned long)item) + diff,
 				   csum_size * count);
@@ -436,8 +436,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 		btrfs_item_key_to_cpu(leaf, &key, path->slots[0] - 1);
 		if (key.objectid == BTRFS_EXTENT_CSUM_OBJECTID &&
 		    key.type == BTRFS_EXTENT_CSUM_KEY) {
-			offset = (start - key.offset) >>
-				 fs_info->sb->s_blocksize_bits;
+			offset = (start - key.offset) >> fs_info->sectorsize_bits;
 			if (offset * csum_size <
 			    btrfs_item_size_nr(leaf, path->slots[0] - 1))
 				path->slots[0]--;
@@ -488,10 +487,9 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 			sums->bytenr = start;
 			sums->len = (int)size;
 
-			offset = (start - key.offset) >>
-				fs_info->sb->s_blocksize_bits;
+			offset = (start - key.offset) >> fs_info->sectorsize_bits;
 			offset *= csum_size;
-			size >>= fs_info->sb->s_blocksize_bits;
+			size >>= fs_info->sectorsize_bits;
 
 			read_extent_buffer(path->nodes[0],
 					   sums->sums,
@@ -644,11 +642,11 @@ static noinline void truncate_one_csum(struct btrfs_fs_info *fs_info,
 	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	u64 csum_end;
 	u64 end_byte = bytenr + len;
-	u32 blocksize_bits = fs_info->sb->s_blocksize_bits;
+	u32 blocksize_bits = fs_info->sectorsize_bits;
 
 	leaf = path->nodes[0];
 	csum_end = btrfs_item_size_nr(leaf, path->slots[0]) / csum_size;
-	csum_end <<= fs_info->sb->s_blocksize_bits;
+	csum_end <<= blocksize_bits;
 	csum_end += key->offset;
 
 	if (key->offset < bytenr && csum_end <= end_byte) {
@@ -696,7 +694,7 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
 	struct extent_buffer *leaf;
 	int ret;
 	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
-	int blocksize_bits = fs_info->sb->s_blocksize_bits;
+	u32 blocksize_bits = fs_info->sectorsize_bits;
 
 	ASSERT(root == fs_info->csum_root ||
 	       root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
@@ -925,7 +923,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	if (btrfs_leaf_free_space(leaf) >= csum_size) {
 		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
 		csum_offset = (bytenr - found_key.offset) >>
-			fs_info->sb->s_blocksize_bits;
+			fs_info->sectorsize_bits;
 		goto extend_csum;
 	}
 
@@ -943,8 +941,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 
 	leaf = path->nodes[0];
 	btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
-	csum_offset = (bytenr - found_key.offset) >>
-			fs_info->sb->s_blocksize_bits;
+	csum_offset = (bytenr - found_key.offset) >> fs_info->sectorsize_bits;
 
 	if (found_key.type != BTRFS_EXTENT_CSUM_KEY ||
 	    found_key.objectid != BTRFS_EXTENT_CSUM_OBJECTID ||
@@ -960,7 +957,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 		u32 diff;
 
 		tmp = sums->len - total_bytes;
-		tmp >>= fs_info->sb->s_blocksize_bits;
+		tmp >>= fs_info->sectorsize_bits;
 		WARN_ON(tmp < 1);
 
 		extend_nr = max_t(int, 1, (int)tmp);
@@ -985,9 +982,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 		u64 tmp;
 
 		tmp = sums->len - total_bytes;
-		tmp >>= fs_info->sb->s_blocksize_bits;
+		tmp >>= fs_info->sectorsize_bits;
 		tmp = min(tmp, (next_offset - file_key.offset) >>
-					 fs_info->sb->s_blocksize_bits);
+					 fs_info->sectorsize_bits);
 
 		tmp = max_t(u64, 1, tmp);
 		tmp = min_t(u64, tmp, MAX_CSUM_ITEMS(fs_info, csum_size));
@@ -1011,8 +1008,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	item = (struct btrfs_csum_item *)((unsigned char *)item +
 					  csum_offset * csum_size);
 found:
-	ins_size = (u32)(sums->len - total_bytes) >>
-		   fs_info->sb->s_blocksize_bits;
+	ins_size = (u32)(sums->len - total_bytes) >> fs_info->sectorsize_bits;
 	ins_size *= csum_size;
 	ins_size = min_t(u32, (unsigned long)item_end - (unsigned long)item,
 			      ins_size);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 1c97e559aefb..25dc5eb495d8 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1763,8 +1763,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 
 		if (num_sectors > dirty_sectors) {
 			/* release everything except the sectors we dirtied */
-			release_bytes -= dirty_sectors <<
-						fs_info->sb->s_blocksize_bits;
+			release_bytes -= dirty_sectors << fs_info->sectorsize_bits;
 			if (only_release_metadata) {
 				btrfs_delalloc_release_metadata(BTRFS_I(inode),
 							release_bytes, true);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1dcccd212809..5582c1c9c007 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2854,7 +2854,7 @@ int btrfs_verify_data_csum(struct btrfs_io_bio *io_bio, u64 phy_offset,
 		return 0;
 	}
 
-	phy_offset >>= inode->i_sb->s_blocksize_bits;
+	phy_offset >>= root->fs_info->sectorsize_bits;
 	return check_data_csum(inode, io_bio, phy_offset, page, offset, start,
 			       (size_t)(end - start + 1));
 }
@@ -7737,7 +7737,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
 		u64 csum_offset;
 
 		csum_offset = file_offset - dip->logical_offset;
-		csum_offset >>= inode->i_sb->s_blocksize_bits;
+		csum_offset >>= fs_info->sectorsize_bits;
 		csum_offset *= btrfs_super_csum_size(fs_info->super_copy);
 		btrfs_io_bio(bio)->csum = dip->csums + csum_offset;
 	}
@@ -7766,7 +7766,7 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
 		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 		size_t nblocks;
 
-		nblocks = dio_bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
+		nblocks = dio_bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
 		dip_size += csum_size * nblocks;
 	}
 
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 7b62dcc6cd98..ecc731a6bbae 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -868,7 +868,6 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
-	const u8 blocksize_bits = inode->vfs_inode.i_sb->s_blocksize_bits;
 	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int index = 0;
 
@@ -880,8 +879,9 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 	list_for_each_entry_reverse(ordered_sum, &ordered->list, list) {
 		if (disk_bytenr >= ordered_sum->bytenr &&
 		    disk_bytenr < ordered_sum->bytenr + ordered_sum->len) {
-			i = (disk_bytenr - ordered_sum->bytenr) >> blocksize_bits;
-			num_sectors = ordered_sum->len >> blocksize_bits;
+			i = (disk_bytenr - ordered_sum->bytenr) >>
+			    fs_info->sectorsize_bits;
+			num_sectors = ordered_sum->len >> fs_info->sectorsize_bits;
 			num_sectors = min_t(int, len - index, num_sectors - i);
 			memcpy(sum + index, ordered_sum->sums + i * csum_size,
 			       num_sectors * csum_size);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1ffa50bae1dd..87b390a5351f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2205,7 +2205,7 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	u64 total_used = 0;
 	u64 total_free_data = 0;
 	u64 total_free_meta = 0;
-	int bits = dentry->d_sb->s_blocksize_bits;
+	u32 bits = fs_info->sectorsize_bits;
 	__be32 *fsid = (__be32 *)fs_info->fs_devices->fsid;
 	unsigned factor = 1;
 	struct btrfs_block_rsv *block_rsv = &fs_info->global_block_rsv;
-- 
2.25.0

Re: [PATCH 03/10] btrfs: replace s_blocksize_bits with fs_info::sectorsize_bits

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 11526 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> The value of super_block::s_blocksize_bits is the same as
> fs_info::sectorsize_bits, but we don't need to do the extra dereferences
> in many functions and storing the bits as u32 (in fs_info) generates
> shorter assembly.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

This patch is great.

I was just going to kill all "inode->i_sb->s_blocksize_bits" for subpage.

Although for subpage case, we may populate sb->s_blocksize_bits to
PAGE_SHIFT, as current subpage doesn't support real subpage write at all.
Thus we want everything from DIO alignement to reflink alignment to
still be PAGE_SIZE.

With this patch, this allows us to get correct sector size from btrfs
directly, without bothering the superblock block size.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>  fs/btrfs/ctree.h        |  2 +-
>  fs/btrfs/extent_io.c    |  2 +-
>  fs/btrfs/file-item.c    | 34 +++++++++++++++-------------------
>  fs/btrfs/file.c         |  3 +--
>  fs/btrfs/inode.c        |  6 +++---
>  fs/btrfs/ordered-data.c |  6 +++---
>  fs/btrfs/super.c        |  2 +-
>  7 files changed, 25 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 87c40cc5c42e..a1a0b99c3530 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1405,7 +1405,7 @@ struct btrfs_map_token {
>  };
>  
>  #define BTRFS_BYTES_TO_BLKS(fs_info, bytes) \
> -				((bytes) >> (fs_info)->sb->s_blocksize_bits)
> +				((bytes) >> (fs_info)->sectorsize_bits)
>  
>  static inline void btrfs_init_map_token(struct btrfs_map_token *token,
>  					struct extent_buffer *eb)
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 14d01b76f5c9..cd27a2a4f717 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2652,7 +2652,7 @@ blk_status_t btrfs_submit_read_repair(struct inode *inode,
>  	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
>  	struct extent_io_tree *failure_tree = &BTRFS_I(inode)->io_failure_tree;
>  	struct btrfs_io_bio *failed_io_bio = btrfs_io_bio(failed_bio);
> -	const int icsum = phy_offset >> inode->i_sb->s_blocksize_bits;
> +	const int icsum = phy_offset >> fs_info->sectorsize_bits;
>  	bool need_validation;
>  	struct bio *repair_bio;
>  	struct btrfs_io_bio *repair_io_bio;
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index d8cd467b4e0c..ed750dd8a115 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -201,7 +201,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans,
>  			goto fail;
>  
>  		csum_offset = (bytenr - found_key.offset) >>
> -				fs_info->sb->s_blocksize_bits;
> +				fs_info->sectorsize_bits;
>  		csums_in_item = btrfs_item_size_nr(leaf, path->slots[0]);
>  		csums_in_item /= csum_size;
>  
> @@ -279,7 +279,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>  	if (!path)
>  		return BLK_STS_RESOURCE;
>  
> -	nblocks = bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
> +	nblocks = bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
>  	if (!dst) {
>  		struct btrfs_io_bio *btrfs_bio = btrfs_io_bio(bio);
>  
> @@ -372,7 +372,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>  		diff = diff >> fs_info->sectorsize_bits;
>  		diff = diff * csum_size;
>  		count = min_t(int, nblocks, (item_last_offset - disk_bytenr) >>
> -					    inode->i_sb->s_blocksize_bits);
> +					    fs_info->sectorsize_bits);
>  		read_extent_buffer(path->nodes[0], csum,
>  				   ((unsigned long)item) + diff,
>  				   csum_size * count);
> @@ -436,8 +436,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  		btrfs_item_key_to_cpu(leaf, &key, path->slots[0] - 1);
>  		if (key.objectid == BTRFS_EXTENT_CSUM_OBJECTID &&
>  		    key.type == BTRFS_EXTENT_CSUM_KEY) {
> -			offset = (start - key.offset) >>
> -				 fs_info->sb->s_blocksize_bits;
> +			offset = (start - key.offset) >> fs_info->sectorsize_bits;
>  			if (offset * csum_size <
>  			    btrfs_item_size_nr(leaf, path->slots[0] - 1))
>  				path->slots[0]--;
> @@ -488,10 +487,9 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  			sums->bytenr = start;
>  			sums->len = (int)size;
>  
> -			offset = (start - key.offset) >>
> -				fs_info->sb->s_blocksize_bits;
> +			offset = (start - key.offset) >> fs_info->sectorsize_bits;
>  			offset *= csum_size;
> -			size >>= fs_info->sb->s_blocksize_bits;
> +			size >>= fs_info->sectorsize_bits;
>  
>  			read_extent_buffer(path->nodes[0],
>  					   sums->sums,
> @@ -644,11 +642,11 @@ static noinline void truncate_one_csum(struct btrfs_fs_info *fs_info,
>  	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  	u64 csum_end;
>  	u64 end_byte = bytenr + len;
> -	u32 blocksize_bits = fs_info->sb->s_blocksize_bits;
> +	u32 blocksize_bits = fs_info->sectorsize_bits;
>  
>  	leaf = path->nodes[0];
>  	csum_end = btrfs_item_size_nr(leaf, path->slots[0]) / csum_size;
> -	csum_end <<= fs_info->sb->s_blocksize_bits;
> +	csum_end <<= blocksize_bits;
>  	csum_end += key->offset;
>  
>  	if (key->offset < bytenr && csum_end <= end_byte) {
> @@ -696,7 +694,7 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
>  	struct extent_buffer *leaf;
>  	int ret;
>  	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> -	int blocksize_bits = fs_info->sb->s_blocksize_bits;
> +	u32 blocksize_bits = fs_info->sectorsize_bits;
>  
>  	ASSERT(root == fs_info->csum_root ||
>  	       root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
> @@ -925,7 +923,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  	if (btrfs_leaf_free_space(leaf) >= csum_size) {
>  		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
>  		csum_offset = (bytenr - found_key.offset) >>
> -			fs_info->sb->s_blocksize_bits;
> +			fs_info->sectorsize_bits;
>  		goto extend_csum;
>  	}
>  
> @@ -943,8 +941,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  
>  	leaf = path->nodes[0];
>  	btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
> -	csum_offset = (bytenr - found_key.offset) >>
> -			fs_info->sb->s_blocksize_bits;
> +	csum_offset = (bytenr - found_key.offset) >> fs_info->sectorsize_bits;
>  
>  	if (found_key.type != BTRFS_EXTENT_CSUM_KEY ||
>  	    found_key.objectid != BTRFS_EXTENT_CSUM_OBJECTID ||
> @@ -960,7 +957,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  		u32 diff;
>  
>  		tmp = sums->len - total_bytes;
> -		tmp >>= fs_info->sb->s_blocksize_bits;
> +		tmp >>= fs_info->sectorsize_bits;
>  		WARN_ON(tmp < 1);
>  
>  		extend_nr = max_t(int, 1, (int)tmp);
> @@ -985,9 +982,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  		u64 tmp;
>  
>  		tmp = sums->len - total_bytes;
> -		tmp >>= fs_info->sb->s_blocksize_bits;
> +		tmp >>= fs_info->sectorsize_bits;
>  		tmp = min(tmp, (next_offset - file_key.offset) >>
> -					 fs_info->sb->s_blocksize_bits);
> +					 fs_info->sectorsize_bits);
>  
>  		tmp = max_t(u64, 1, tmp);
>  		tmp = min_t(u64, tmp, MAX_CSUM_ITEMS(fs_info, csum_size));
> @@ -1011,8 +1008,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  	item = (struct btrfs_csum_item *)((unsigned char *)item +
>  					  csum_offset * csum_size);
>  found:
> -	ins_size = (u32)(sums->len - total_bytes) >>
> -		   fs_info->sb->s_blocksize_bits;
> +	ins_size = (u32)(sums->len - total_bytes) >> fs_info->sectorsize_bits;
>  	ins_size *= csum_size;
>  	ins_size = min_t(u32, (unsigned long)item_end - (unsigned long)item,
>  			      ins_size);
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 1c97e559aefb..25dc5eb495d8 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1763,8 +1763,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
>  
>  		if (num_sectors > dirty_sectors) {
>  			/* release everything except the sectors we dirtied */
> -			release_bytes -= dirty_sectors <<
> -						fs_info->sb->s_blocksize_bits;
> +			release_bytes -= dirty_sectors << fs_info->sectorsize_bits;
>  			if (only_release_metadata) {
>  				btrfs_delalloc_release_metadata(BTRFS_I(inode),
>  							release_bytes, true);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1dcccd212809..5582c1c9c007 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2854,7 +2854,7 @@ int btrfs_verify_data_csum(struct btrfs_io_bio *io_bio, u64 phy_offset,
>  		return 0;
>  	}
>  
> -	phy_offset >>= inode->i_sb->s_blocksize_bits;
> +	phy_offset >>= root->fs_info->sectorsize_bits;
>  	return check_data_csum(inode, io_bio, phy_offset, page, offset, start,
>  			       (size_t)(end - start + 1));
>  }
> @@ -7737,7 +7737,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
>  		u64 csum_offset;
>  
>  		csum_offset = file_offset - dip->logical_offset;
> -		csum_offset >>= inode->i_sb->s_blocksize_bits;
> +		csum_offset >>= fs_info->sectorsize_bits;
>  		csum_offset *= btrfs_super_csum_size(fs_info->super_copy);
>  		btrfs_io_bio(bio)->csum = dip->csums + csum_offset;
>  	}
> @@ -7766,7 +7766,7 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
>  		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  		size_t nblocks;
>  
> -		nblocks = dio_bio->bi_iter.bi_size >> inode->i_sb->s_blocksize_bits;
> +		nblocks = dio_bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
>  		dip_size += csum_size * nblocks;
>  	}
>  
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 7b62dcc6cd98..ecc731a6bbae 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -868,7 +868,6 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>  	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
>  	unsigned long num_sectors;
>  	unsigned long i;
> -	const u8 blocksize_bits = inode->vfs_inode.i_sb->s_blocksize_bits;
>  	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  	int index = 0;
>  
> @@ -880,8 +879,9 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>  	list_for_each_entry_reverse(ordered_sum, &ordered->list, list) {
>  		if (disk_bytenr >= ordered_sum->bytenr &&
>  		    disk_bytenr < ordered_sum->bytenr + ordered_sum->len) {
> -			i = (disk_bytenr - ordered_sum->bytenr) >> blocksize_bits;
> -			num_sectors = ordered_sum->len >> blocksize_bits;
> +			i = (disk_bytenr - ordered_sum->bytenr) >>
> +			    fs_info->sectorsize_bits;
> +			num_sectors = ordered_sum->len >> fs_info->sectorsize_bits;
>  			num_sectors = min_t(int, len - index, num_sectors - i);
>  			memcpy(sum + index, ordered_sum->sums + i * csum_size,
>  			       num_sectors * csum_size);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 1ffa50bae1dd..87b390a5351f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2205,7 +2205,7 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	u64 total_used = 0;
>  	u64 total_free_data = 0;
>  	u64 total_free_meta = 0;
> -	int bits = dentry->d_sb->s_blocksize_bits;
> +	u32 bits = fs_info->sectorsize_bits;
>  	__be32 *fsid = (__be32 *)fs_info->fs_devices->fsid;
>  	unsigned factor = 1;
>  	struct btrfs_block_rsv *block_rsv = &fs_info->global_block_rsv;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 03/10] btrfs: replace s_blocksize_bits with fs_info::sectorsize_bits

[David Sterba]

On Mon, Nov 02, 2020 at 10:23:43PM +0800, Qu Wenruo wrote:
> 
> 
> On 2020/10/29 下午10:27, David Sterba wrote:
> > The value of super_block::s_blocksize_bits is the same as
> > fs_info::sectorsize_bits, but we don't need to do the extra dereferences
> > in many functions and storing the bits as u32 (in fs_info) generates
> > shorter assembly.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> 
> This patch is great.
> 
> I was just going to kill all "inode->i_sb->s_blocksize_bits" for subpage.
> 
> Although for subpage case, we may populate sb->s_blocksize_bits to
> PAGE_SHIFT, as current subpage doesn't support real subpage write at all.
> Thus we want everything from DIO alignement to reflink alignment to
> still be PAGE_SIZE.

Yeah the different alignment constraints with subpage will be
interesting.

[PATCH 04/10] btrfs: store precalculated csum_size in fs_info

[David Sterba]

In many places we need the checksum size and it is inefficient to read
it from the raw superblock. Store the value into fs_info, actual use
will be in followup patches.  The size is u32 as it allows to generate
better assembly than with u16.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h   | 1 +
 fs/btrfs/disk-io.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a1a0b99c3530..1f2162fc1daa 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -932,6 +932,7 @@ struct btrfs_fs_info {
 	u32 nodesize;
 	u32 sectorsize;
 	u32 sectorsize_bits;
+	u32 csum_size;
 	u32 stripesize;
 
 	/* Block groups and devices containing active swapfiles. */
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4e67c122389c..25dbdfa8bc4b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3078,6 +3078,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	fs_info->nodesize = nodesize;
 	fs_info->sectorsize = sectorsize;
 	fs_info->sectorsize_bits = ilog2(sectorsize);
+	fs_info->csum_size = btrfs_super_csum_size(disk_super);
 	fs_info->stripesize = stripesize;
 
 	/*
-- 
2.25.0

[PATCH 05/10] btrfs: precalculate checksums per leaf once

[David Sterba]

btrfs_csum_bytes_to_leaves shows up in system profiles, which makes it a
candidate for optimizations. After the 64bit division has been replaced
by shift, there's still a calculation done each time the function is
called: checksums per leaf.

As this is a constanat value for the entire filesystem lifetime, we
can calculate it once at mount time and reuse. This also allows to
reduce the division to 64bit/32bit as we know the constant will always
fit to 32bit type.

Replace the open-coded rounding up with a macro that internally handles
the 64bit division.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h       | 1 +
 fs/btrfs/disk-io.c     | 1 +
 fs/btrfs/extent-tree.c | 9 +--------
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1f2162fc1daa..8c4cd79b2810 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -933,6 +933,7 @@ struct btrfs_fs_info {
 	u32 sectorsize;
 	u32 sectorsize_bits;
 	u32 csum_size;
+	u32 csums_per_leaf;
 	u32 stripesize;
 
 	/* Block groups and devices containing active swapfiles. */
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 25dbdfa8bc4b..f870e252aa37 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3079,6 +3079,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	fs_info->sectorsize = sectorsize;
 	fs_info->sectorsize_bits = ilog2(sectorsize);
 	fs_info->csum_size = btrfs_super_csum_size(disk_super);
+	fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;
 	fs_info->stripesize = stripesize;
 
 	/*
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 29ac97248942..81440a0ba106 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2138,17 +2138,10 @@ static u64 find_middle(struct rb_root *root)
  */
 u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
 {
-	u64 csum_size;
-	u64 num_csums_per_leaf;
 	u64 num_csums;
 
-	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
-	num_csums_per_leaf = div64_u64(csum_size,
-			(u64)btrfs_super_csum_size(fs_info->super_copy));
 	num_csums = csum_bytes >> fs_info->sectorsize_bits;
-	num_csums += num_csums_per_leaf - 1;
-	num_csums = div64_u64(num_csums, num_csums_per_leaf);
-	return num_csums;
+	return DIV_ROUND_UP_ULL(num_csums, fs_info->csums_per_leaf);
 }
 
 /*
-- 
2.25.0

Re: [PATCH 05/10] btrfs: precalculate checksums per leaf once

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2923 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> btrfs_csum_bytes_to_leaves shows up in system profiles, which makes it a
> candidate for optimizations. After the 64bit division has been replaced
> by shift, there's still a calculation done each time the function is
> called: checksums per leaf.
> 
> As this is a constanat value for the entire filesystem lifetime, we
> can calculate it once at mount time and reuse. This also allows to
> reduce the division to 64bit/32bit as we know the constant will always
> fit to 32bit type.
> 
> Replace the open-coded rounding up with a macro that internally handles
> the 64bit division.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h       | 1 +
>  fs/btrfs/disk-io.c     | 1 +
>  fs/btrfs/extent-tree.c | 9 +--------
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 1f2162fc1daa..8c4cd79b2810 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -933,6 +933,7 @@ struct btrfs_fs_info {
>  	u32 sectorsize;
>  	u32 sectorsize_bits;
>  	u32 csum_size;
> +	u32 csums_per_leaf;
>  	u32 stripesize;
>  
>  	/* Block groups and devices containing active swapfiles. */
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 25dbdfa8bc4b..f870e252aa37 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3079,6 +3079,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  	fs_info->sectorsize = sectorsize;
>  	fs_info->sectorsize_bits = ilog2(sectorsize);
>  	fs_info->csum_size = btrfs_super_csum_size(disk_super);
> +	fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;

I guess here we don't need any macro for division right?
The BTRFS_MAX_ITEM_SIZE() should follow the type of
BTRFS_MAX_ITEM_SIZE() which is u32, thus u32/u32, we're safe even on
32bit systems, right?

>  	fs_info->stripesize = stripesize;
>  
>  	/*
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 29ac97248942..81440a0ba106 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2138,17 +2138,10 @@ static u64 find_middle(struct rb_root *root)
>   */
>  u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
>  {
> -	u64 csum_size;
> -	u64 num_csums_per_leaf;
>  	u64 num_csums;
>  
> -	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
> -	num_csums_per_leaf = div64_u64(csum_size,
> -			(u64)btrfs_super_csum_size(fs_info->super_copy));
>  	num_csums = csum_bytes >> fs_info->sectorsize_bits;
> -	num_csums += num_csums_per_leaf - 1;
> -	num_csums = div64_u64(num_csums, num_csums_per_leaf);
> -	return num_csums;
> +	return DIV_ROUND_UP_ULL(num_csums, fs_info->csums_per_leaf);

Since it's just a DIV_ROUND_UP_ULL() call, can we make it inline?

Thanks,
Qu
>  }
>  
>  /*
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 05/10] btrfs: precalculate checksums per leaf once

[David Sterba]

On Mon, Nov 02, 2020 at 10:27:25PM +0800, Qu Wenruo wrote:
> On 2020/10/29 下午10:27, David Sterba wrote:
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -3079,6 +3079,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> >  	fs_info->sectorsize = sectorsize;
> >  	fs_info->sectorsize_bits = ilog2(sectorsize);
> >  	fs_info->csum_size = btrfs_super_csum_size(disk_super);
> > +	fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;
> 
> I guess here we don't need any macro for division right?
> The BTRFS_MAX_ITEM_SIZE() should follow the type of
> BTRFS_MAX_ITEM_SIZE() which is u32, thus u32/u32, we're safe even on
> 32bit systems, right?

Yes, 32/32 is safe in general.

> >  	fs_info->stripesize = stripesize;
> >  
> >  	/*
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 29ac97248942..81440a0ba106 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -2138,17 +2138,10 @@ static u64 find_middle(struct rb_root *root)
> >   */
> >  u64 btrfs_csum_bytes_to_leaves(struct btrfs_fs_info *fs_info, u64 csum_bytes)
> >  {
> > -	u64 csum_size;
> > -	u64 num_csums_per_leaf;
> >  	u64 num_csums;
> >  
> > -	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
> > -	num_csums_per_leaf = div64_u64(csum_size,
> > -			(u64)btrfs_super_csum_size(fs_info->super_copy));
> >  	num_csums = csum_bytes >> fs_info->sectorsize_bits;
> > -	num_csums += num_csums_per_leaf - 1;
> > -	num_csums = div64_u64(num_csums, num_csums_per_leaf);
> > -	return num_csums;
> > +	return DIV_ROUND_UP_ULL(num_csums, fs_info->csums_per_leaf);
> 
> Since it's just a DIV_ROUND_UP_ULL() call, can we make it inline?

Good point, but i'll check first if this does not bloat code due to
inlining. As it's called once in all callers, the overhead of call is
not a problem.

Re: [PATCH 05/10] btrfs: precalculate checksums per leaf once

[David Sterba]

On Mon, Nov 02, 2020 at 04:24:45PM +0100, David Sterba wrote:
> On Mon, Nov 02, 2020 at 10:27:25PM +0800, Qu Wenruo wrote:
> > On 2020/10/29 下午10:27, David Sterba wrote:
> > > -	csum_size = BTRFS_MAX_ITEM_SIZE(fs_info);
> > > -	num_csums_per_leaf = div64_u64(csum_size,
> > > -			(u64)btrfs_super_csum_size(fs_info->super_copy));
> > >  	num_csums = csum_bytes >> fs_info->sectorsize_bits;
> > > -	num_csums += num_csums_per_leaf - 1;
> > > -	num_csums = div64_u64(num_csums, num_csums_per_leaf);
> > > -	return num_csums;
> > > +	return DIV_ROUND_UP_ULL(num_csums, fs_info->csums_per_leaf);
> > 
> > Since it's just a DIV_ROUND_UP_ULL() call, can we make it inline?
> 
> Good point, but i'll check first if this does not bloat code due to
> inlining. As it's called once in all callers, the overhead of call is
> not a problem.

On a release config it adds about 40 bytes in resulting asm and reduces
some stack usage due to the removed calls, I guess this speaks in favor
of inlining so I'll switch it. Thanks.

[PATCH 06/10] btrfs: use cached value of fs_info::csum_size everywhere

[David Sterba]

btrfs_get_16 shows up in the system performance profiles (helper to read
16bit values from on-disk structures). This is partially because of the
checksum size that's frequently read along with data reads/writes, other
u16 uses are from item size or directory entries.

Replace all calls to btrfs_super_csum_size by the cached value from
fs_info.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/btrfs_inode.h     |  3 +--
 fs/btrfs/check-integrity.c |  2 +-
 fs/btrfs/compression.c     |  6 +++---
 fs/btrfs/disk-io.c         |  6 +++---
 fs/btrfs/extent_io.c       |  2 +-
 fs/btrfs/file-item.c       | 14 +++++++-------
 fs/btrfs/inode.c           |  6 +++---
 fs/btrfs/ordered-data.c    |  2 +-
 fs/btrfs/ordered-data.h    |  2 +-
 fs/btrfs/scrub.c           |  2 +-
 fs/btrfs/tree-checker.c    |  2 +-
 11 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 92dd86bceae3..f1c9cbd0d184 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -341,8 +341,7 @@ static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
 		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
-	struct btrfs_super_block *sb = root->fs_info->super_copy;
-	const u16 csum_size = btrfs_super_csum_size(sb);
+	const u16 csum_size = root->fs_info->csum_size;
 
 	/* Output minus objectid, which is more meaningful */
 	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 81a8c87a5afb..2905fe5974e6 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -660,7 +660,7 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 		return -1;
 	}
 
-	state->csum_size = btrfs_super_csum_size(selected_super);
+	state->csum_size = state->fs_info->csum_size;
 
 	for (pass = 0; pass < 3; pass++) {
 		int num_copies;
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 972fb68a85ac..00bbd859f31b 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -131,7 +131,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb);
 static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
 				      unsigned long disk_size)
 {
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	return sizeof(struct compressed_bio) +
 		(DIV_ROUND_UP(disk_size, fs_info->sectorsize)) * csum_size;
@@ -142,7 +142,7 @@ static int check_compressed_csum(struct btrfs_inode *inode, struct bio *bio,
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
@@ -628,7 +628,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	struct extent_map *em;
 	blk_status_t ret = BLK_STS_RESOURCE;
 	int faili = 0;
-	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	u8 *sums;
 
 	em_tree = &BTRFS_I(inode)->extent_tree;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f870e252aa37..e22e8de31c07 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -319,7 +319,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	crypto_shash_digest(shash, raw_disk_sb + BTRFS_CSUM_SIZE,
 			    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE, result);
 
-	if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
+	if (memcmp(disk_sb->csum, result, fs_info->csum_size))
 		return 1;
 
 	return 0;
@@ -452,7 +452,7 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
 	u64 start = page_offset(page);
 	u64 found_start;
 	u8 result[BTRFS_CSUM_SIZE];
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	struct extent_buffer *eb;
 	int ret;
 
@@ -541,7 +541,7 @@ int btrfs_validate_metadata_buffer(struct btrfs_io_bio *io_bio, u64 phy_offset,
 
 	eb = (struct extent_buffer *)page->private;
 	fs_info = eb->fs_info;
-	csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	csum_size = fs_info->csum_size;
 
 	/* the pending IO might have been the only thing that kept this buffer
 	 * in memory.  Make sure we have a ref for all this other checks
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cd27a2a4f717..e943fff35608 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2685,7 +2685,7 @@ blk_status_t btrfs_submit_read_repair(struct inode *inode,
 	repair_bio->bi_private = failed_bio->bi_private;
 
 	if (failed_io_bio->csum) {
-		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+		const u16 csum_size = fs_info->csum_size;
 
 		repair_io_bio->csum = repair_io_bio->csum_inline;
 		memcpy(repair_io_bio->csum,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index ed750dd8a115..a4a68a224342 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -181,7 +181,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans,
 	struct btrfs_csum_item *item;
 	struct extent_buffer *leaf;
 	u64 csum_offset = 0;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	int csums_in_item;
 
 	file_key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
@@ -270,7 +270,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 	u32 diff;
 	int nblocks;
 	int count = 0;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	if (!fs_info->csum_root || (BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM))
 		return BLK_STS_OK;
@@ -409,7 +409,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 	int ret;
 	size_t size;
 	u64 csum_end;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
 	       IS_ALIGNED(end + 1, fs_info->sectorsize));
@@ -541,7 +541,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
 	int i;
 	u64 offset;
 	unsigned nofs_flag;
-	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	nofs_flag = memalloc_nofs_save();
 	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
@@ -639,7 +639,7 @@ static noinline void truncate_one_csum(struct btrfs_fs_info *fs_info,
 				       u64 bytenr, u64 len)
 {
 	struct extent_buffer *leaf;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	u64 csum_end;
 	u64 end_byte = bytenr + len;
 	u32 blocksize_bits = fs_info->sectorsize_bits;
@@ -693,7 +693,7 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
 	u64 csum_end;
 	struct extent_buffer *leaf;
 	int ret;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	u32 blocksize_bits = fs_info->sectorsize_bits;
 
 	ASSERT(root == fs_info->csum_root ||
@@ -848,7 +848,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	int index = 0;
 	int found_next;
 	int ret;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	path = btrfs_alloc_path();
 	if (!path)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5582c1c9c007..549dca610f8c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2796,7 +2796,7 @@ static int check_data_csum(struct inode *inode, struct btrfs_io_bio *io_bio,
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	char *kaddr;
-	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	u8 *csum_expected;
 	u8 csum[BTRFS_CSUM_SIZE];
 
@@ -7738,7 +7738,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
 
 		csum_offset = file_offset - dip->logical_offset;
 		csum_offset >>= fs_info->sectorsize_bits;
-		csum_offset *= btrfs_super_csum_size(fs_info->super_copy);
+		csum_offset *= fs_info->csum_size;
 		btrfs_io_bio(bio)->csum = dip->csums + csum_offset;
 	}
 map:
@@ -7763,7 +7763,7 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
 	dip_size = sizeof(*dip);
 	if (!write && csum) {
 		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+		const u16 csum_size = fs_info->csum_size;
 		size_t nblocks;
 
 		nblocks = dio_bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index ecc731a6bbae..4d612105b991 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -868,7 +868,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
-	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 	int index = 0;
 
 	ordered = btrfs_lookup_ordered_extent(inode, offset);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index c3a2325e64a4..4662fd8ca546 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -137,7 +137,7 @@ static inline int btrfs_ordered_sum_size(struct btrfs_fs_info *fs_info,
 					 unsigned long bytes)
 {
 	int num_sectors = (int)DIV_ROUND_UP(bytes, fs_info->sectorsize);
-	int csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csum_size = fs_info->csum_size;
 
 	return sizeof(struct btrfs_ordered_sum) + num_sectors * csum_size;
 }
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 7babf670c8c2..d4f693a4ca38 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -610,7 +610,7 @@ static noinline_for_stack struct scrub_ctx *scrub_setup_ctx(
 	atomic_set(&sctx->bios_in_flight, 0);
 	atomic_set(&sctx->workers_pending, 0);
 	atomic_set(&sctx->cancel_req, 0);
-	sctx->csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	sctx->csum_size = fs_info->csum_size;
 
 	spin_lock_init(&sctx->list_lock);
 	spin_lock_init(&sctx->stat_lock);
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index c0e19917e59b..5efaf1f811e2 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -336,7 +336,7 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
 {
 	struct btrfs_fs_info *fs_info = leaf->fs_info;
 	u32 sectorsize = fs_info->sectorsize;
-	u32 csumsize = btrfs_super_csum_size(fs_info->super_copy);
+	const u16 csumsize = fs_info->csum_size;
 
 	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
 		generic_err(leaf, slot,
-- 
2.25.0

Re: [PATCH 06/10] btrfs: use cached value of fs_info::csum_size everywhere

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 11850 bytes --]



On 2020/10/29 下午10:27, David Sterba wrote:
> btrfs_get_16 shows up in the system performance profiles (helper to read
> 16bit values from on-disk structures). This is partially because of the
> checksum size that's frequently read along with data reads/writes, other
> u16 uses are from item size or directory entries.
> 
> Replace all calls to btrfs_super_csum_size by the cached value from
> fs_info.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/btrfs_inode.h     |  3 +--
>  fs/btrfs/check-integrity.c |  2 +-
>  fs/btrfs/compression.c     |  6 +++---
>  fs/btrfs/disk-io.c         |  6 +++---
>  fs/btrfs/extent_io.c       |  2 +-
>  fs/btrfs/file-item.c       | 14 +++++++-------
>  fs/btrfs/inode.c           |  6 +++---
>  fs/btrfs/ordered-data.c    |  2 +-
>  fs/btrfs/ordered-data.h    |  2 +-
>  fs/btrfs/scrub.c           |  2 +-
>  fs/btrfs/tree-checker.c    |  2 +-
>  11 files changed, 23 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 92dd86bceae3..f1c9cbd0d184 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -341,8 +341,7 @@ static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
>  		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
>  {
>  	struct btrfs_root *root = inode->root;
> -	struct btrfs_super_block *sb = root->fs_info->super_copy;
> -	const u16 csum_size = btrfs_super_csum_size(sb);
> +	const u16 csum_size = root->fs_info->csum_size;
>  
>  	/* Output minus objectid, which is more meaningful */
>  	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 81a8c87a5afb..2905fe5974e6 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -660,7 +660,7 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
>  		return -1;
>  	}
>  
> -	state->csum_size = btrfs_super_csum_size(selected_super);
> +	state->csum_size = state->fs_info->csum_size;
>  
>  	for (pass = 0; pass < 3; pass++) {
>  		int num_copies;
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 972fb68a85ac..00bbd859f31b 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -131,7 +131,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb);
>  static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
>  				      unsigned long disk_size)
>  {
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	return sizeof(struct compressed_bio) +
>  		(DIV_ROUND_UP(disk_size, fs_info->sectorsize)) * csum_size;
> @@ -142,7 +142,7 @@ static int check_compressed_csum(struct btrfs_inode *inode, struct bio *bio,
>  {
>  	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>  	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> -	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	struct page *page;
>  	unsigned long i;
>  	char *kaddr;
> @@ -628,7 +628,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  	struct extent_map *em;
>  	blk_status_t ret = BLK_STS_RESOURCE;
>  	int faili = 0;
> -	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	u8 *sums;
>  
>  	em_tree = &BTRFS_I(inode)->extent_tree;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index f870e252aa37..e22e8de31c07 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -319,7 +319,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	crypto_shash_digest(shash, raw_disk_sb + BTRFS_CSUM_SIZE,
>  			    BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE, result);
>  
> -	if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
> +	if (memcmp(disk_sb->csum, result, fs_info->csum_size))
>  		return 1;
>  
>  	return 0;
> @@ -452,7 +452,7 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
>  	u64 start = page_offset(page);
>  	u64 found_start;
>  	u8 result[BTRFS_CSUM_SIZE];
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	struct extent_buffer *eb;
>  	int ret;
>  
> @@ -541,7 +541,7 @@ int btrfs_validate_metadata_buffer(struct btrfs_io_bio *io_bio, u64 phy_offset,
>  
>  	eb = (struct extent_buffer *)page->private;
>  	fs_info = eb->fs_info;
> -	csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	csum_size = fs_info->csum_size;
>  
>  	/* the pending IO might have been the only thing that kept this buffer
>  	 * in memory.  Make sure we have a ref for all this other checks
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index cd27a2a4f717..e943fff35608 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2685,7 +2685,7 @@ blk_status_t btrfs_submit_read_repair(struct inode *inode,
>  	repair_bio->bi_private = failed_bio->bi_private;
>  
>  	if (failed_io_bio->csum) {
> -		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +		const u16 csum_size = fs_info->csum_size;
>  
>  		repair_io_bio->csum = repair_io_bio->csum_inline;
>  		memcpy(repair_io_bio->csum,
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index ed750dd8a115..a4a68a224342 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -181,7 +181,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans,
>  	struct btrfs_csum_item *item;
>  	struct extent_buffer *leaf;
>  	u64 csum_offset = 0;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	int csums_in_item;
>  
>  	file_key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
> @@ -270,7 +270,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>  	u32 diff;
>  	int nblocks;
>  	int count = 0;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	if (!fs_info->csum_root || (BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM))
>  		return BLK_STS_OK;
> @@ -409,7 +409,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  	int ret;
>  	size_t size;
>  	u64 csum_end;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
>  	       IS_ALIGNED(end + 1, fs_info->sectorsize));
> @@ -541,7 +541,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
>  	int i;
>  	u64 offset;
>  	unsigned nofs_flag;
> -	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	nofs_flag = memalloc_nofs_save();
>  	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
> @@ -639,7 +639,7 @@ static noinline void truncate_one_csum(struct btrfs_fs_info *fs_info,
>  				       u64 bytenr, u64 len)
>  {
>  	struct extent_buffer *leaf;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	u64 csum_end;
>  	u64 end_byte = bytenr + len;
>  	u32 blocksize_bits = fs_info->sectorsize_bits;
> @@ -693,7 +693,7 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
>  	u64 csum_end;
>  	struct extent_buffer *leaf;
>  	int ret;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	u32 blocksize_bits = fs_info->sectorsize_bits;
>  
>  	ASSERT(root == fs_info->csum_root ||
> @@ -848,7 +848,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  	int index = 0;
>  	int found_next;
>  	int ret;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5582c1c9c007..549dca610f8c 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2796,7 +2796,7 @@ static int check_data_csum(struct inode *inode, struct btrfs_io_bio *io_bio,
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>  	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>  	char *kaddr;
> -	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	u8 *csum_expected;
>  	u8 csum[BTRFS_CSUM_SIZE];
>  
> @@ -7738,7 +7738,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
>  
>  		csum_offset = file_offset - dip->logical_offset;
>  		csum_offset >>= fs_info->sectorsize_bits;
> -		csum_offset *= btrfs_super_csum_size(fs_info->super_copy);
> +		csum_offset *= fs_info->csum_size;
>  		btrfs_io_bio(bio)->csum = dip->csums + csum_offset;
>  	}
>  map:
> @@ -7763,7 +7763,7 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
>  	dip_size = sizeof(*dip);
>  	if (!write && csum) {
>  		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> -		const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +		const u16 csum_size = fs_info->csum_size;
>  		size_t nblocks;
>  
>  		nblocks = dio_bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index ecc731a6bbae..4d612105b991 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -868,7 +868,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
>  	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
>  	unsigned long num_sectors;
>  	unsigned long i;
> -	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  	int index = 0;
>  
>  	ordered = btrfs_lookup_ordered_extent(inode, offset);
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index c3a2325e64a4..4662fd8ca546 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -137,7 +137,7 @@ static inline int btrfs_ordered_sum_size(struct btrfs_fs_info *fs_info,
>  					 unsigned long bytes)
>  {
>  	int num_sectors = (int)DIV_ROUND_UP(bytes, fs_info->sectorsize);
> -	int csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csum_size = fs_info->csum_size;
>  
>  	return sizeof(struct btrfs_ordered_sum) + num_sectors * csum_size;
>  }
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 7babf670c8c2..d4f693a4ca38 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -610,7 +610,7 @@ static noinline_for_stack struct scrub_ctx *scrub_setup_ctx(
>  	atomic_set(&sctx->bios_in_flight, 0);
>  	atomic_set(&sctx->workers_pending, 0);
>  	atomic_set(&sctx->cancel_req, 0);
> -	sctx->csum_size = btrfs_super_csum_size(fs_info->super_copy);
> +	sctx->csum_size = fs_info->csum_size;
>  
>  	spin_lock_init(&sctx->list_lock);
>  	spin_lock_init(&sctx->stat_lock);
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index c0e19917e59b..5efaf1f811e2 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -336,7 +336,7 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
>  {
>  	struct btrfs_fs_info *fs_info = leaf->fs_info;
>  	u32 sectorsize = fs_info->sectorsize;
> -	u32 csumsize = btrfs_super_csum_size(fs_info->super_copy);
> +	const u16 csumsize = fs_info->csum_size;
>  
>  	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
>  		generic_err(leaf, slot,
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 07/10] btrfs: switch cached fs_info::csum_size from u16 to u32

[David Sterba]

The fs_info value is 32bit, switch also the local u16 variables. This
leads to a better assembly code generated due to movzwl.

This simple change will shave some bytes on x86_64 and release config:

   text    data     bss     dec     hex filename
1090000   17980   14912 1122892  11224c pre/btrfs.ko
1089794   17980   14912 1122686  11217e post/btrfs.ko

DELTA: -206

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/btrfs_inode.h  |  2 +-
 fs/btrfs/compression.c  |  7 +++----
 fs/btrfs/disk-io.c      |  2 +-
 fs/btrfs/extent_io.c    |  2 +-
 fs/btrfs/file-item.c    | 14 +++++++-------
 fs/btrfs/inode.c        |  5 ++---
 fs/btrfs/ordered-data.c |  2 +-
 fs/btrfs/ordered-data.h |  2 +-
 fs/btrfs/tree-checker.c |  2 +-
 9 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f1c9cbd0d184..7294fae801c9 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -341,7 +341,7 @@ static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
 		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
-	const u16 csum_size = root->fs_info->csum_size;
+	const u32 csum_size = root->fs_info->csum_size;
 
 	/* Output minus objectid, which is more meaningful */
 	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 00bbd859f31b..b2bb4681975e 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -131,8 +131,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb);
 static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
 				      unsigned long disk_size)
 {
-	const u16 csum_size = fs_info->csum_size;
-
+	const u32 csum_size = fs_info->csum_size;
 	return sizeof(struct compressed_bio) +
 		(DIV_ROUND_UP(disk_size, fs_info->sectorsize)) * csum_size;
 }
@@ -142,7 +141,7 @@ static int check_compressed_csum(struct btrfs_inode *inode, struct bio *bio,
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
@@ -628,7 +627,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	struct extent_map *em;
 	blk_status_t ret = BLK_STS_RESOURCE;
 	int faili = 0;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	u8 *sums;
 
 	em_tree = &BTRFS_I(inode)->extent_tree;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index e22e8de31c07..aa89d5c15f6e 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -452,7 +452,7 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
 	u64 start = page_offset(page);
 	u64 found_start;
 	u8 result[BTRFS_CSUM_SIZE];
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	struct extent_buffer *eb;
 	int ret;
 
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index e943fff35608..6ff67d57d088 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2685,7 +2685,7 @@ blk_status_t btrfs_submit_read_repair(struct inode *inode,
 	repair_bio->bi_private = failed_bio->bi_private;
 
 	if (failed_io_bio->csum) {
-		const u16 csum_size = fs_info->csum_size;
+		const u32 csum_size = fs_info->csum_size;
 
 		repair_io_bio->csum = repair_io_bio->csum_inline;
 		memcpy(repair_io_bio->csum,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index a4a68a224342..d7fdfd6ea6f4 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -181,7 +181,7 @@ btrfs_lookup_csum(struct btrfs_trans_handle *trans,
 	struct btrfs_csum_item *item;
 	struct extent_buffer *leaf;
 	u64 csum_offset = 0;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	int csums_in_item;
 
 	file_key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
@@ -270,7 +270,7 @@ blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
 	u32 diff;
 	int nblocks;
 	int count = 0;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 
 	if (!fs_info->csum_root || (BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM))
 		return BLK_STS_OK;
@@ -409,7 +409,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 	int ret;
 	size_t size;
 	u64 csum_end;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 
 	ASSERT(IS_ALIGNED(start, fs_info->sectorsize) &&
 	       IS_ALIGNED(end + 1, fs_info->sectorsize));
@@ -541,7 +541,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
 	int i;
 	u64 offset;
 	unsigned nofs_flag;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 
 	nofs_flag = memalloc_nofs_save();
 	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
@@ -639,7 +639,7 @@ static noinline void truncate_one_csum(struct btrfs_fs_info *fs_info,
 				       u64 bytenr, u64 len)
 {
 	struct extent_buffer *leaf;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	u64 csum_end;
 	u64 end_byte = bytenr + len;
 	u32 blocksize_bits = fs_info->sectorsize_bits;
@@ -693,7 +693,7 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
 	u64 csum_end;
 	struct extent_buffer *leaf;
 	int ret;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	u32 blocksize_bits = fs_info->sectorsize_bits;
 
 	ASSERT(root == fs_info->csum_root ||
@@ -848,7 +848,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	int index = 0;
 	int found_next;
 	int ret;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 
 	path = btrfs_alloc_path();
 	if (!path)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 549dca610f8c..6a3a0e1cbf1c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2796,7 +2796,7 @@ static int check_data_csum(struct inode *inode, struct btrfs_io_bio *io_bio,
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
 	char *kaddr;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	u8 *csum_expected;
 	u8 csum[BTRFS_CSUM_SIZE];
 
@@ -7763,11 +7763,10 @@ static struct btrfs_dio_private *btrfs_create_dio_private(struct bio *dio_bio,
 	dip_size = sizeof(*dip);
 	if (!write && csum) {
 		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-		const u16 csum_size = fs_info->csum_size;
 		size_t nblocks;
 
 		nblocks = dio_bio->bi_iter.bi_size >> fs_info->sectorsize_bits;
-		dip_size += csum_size * nblocks;
+		dip_size += fs_info->csum_size * nblocks;
 	}
 
 	dip = kzalloc(dip_size, GFP_NOFS);
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 4d612105b991..8bb97168afa0 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -868,7 +868,7 @@ int btrfs_find_ordered_sum(struct btrfs_inode *inode, u64 offset,
 	struct btrfs_ordered_inode_tree *tree = &inode->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 	int index = 0;
 
 	ordered = btrfs_lookup_ordered_extent(inode, offset);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 4662fd8ca546..e48810104e4a 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -137,7 +137,7 @@ static inline int btrfs_ordered_sum_size(struct btrfs_fs_info *fs_info,
 					 unsigned long bytes)
 {
 	int num_sectors = (int)DIV_ROUND_UP(bytes, fs_info->sectorsize);
-	const u16 csum_size = fs_info->csum_size;
+	const u32 csum_size = fs_info->csum_size;
 
 	return sizeof(struct btrfs_ordered_sum) + num_sectors * csum_size;
 }
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 5efaf1f811e2..f047640c1b42 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -336,7 +336,7 @@ static int check_csum_item(struct extent_buffer *leaf, struct btrfs_key *key,
 {
 	struct btrfs_fs_info *fs_info = leaf->fs_info;
 	u32 sectorsize = fs_info->sectorsize;
-	const u16 csumsize = fs_info->csum_size;
+	const u32 csumsize = fs_info->csum_size;
 
 	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
 		generic_err(leaf, slot,
-- 
2.25.0

[PATCH 08/10] btrfs: remove unnecessary local variables for checksum size

[David Sterba]

Remove local variable that is then used just once and replace it with
fs_info::csum_tree.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c  | 6 ++----
 fs/btrfs/disk-io.c      | 3 +--
 fs/btrfs/file-item.c    | 3 +--
 fs/btrfs/ordered-data.h | 3 +--
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b2bb4681975e..4e022ed72d2f 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -131,9 +131,8 @@ static int btrfs_decompress_bio(struct compressed_bio *cb);
 static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
 				      unsigned long disk_size)
 {
-	const u32 csum_size = fs_info->csum_size;
 	return sizeof(struct compressed_bio) +
-		(DIV_ROUND_UP(disk_size, fs_info->sectorsize)) * csum_size;
+		(DIV_ROUND_UP(disk_size, fs_info->sectorsize)) * fs_info->csum_size;
 }
 
 static int check_compressed_csum(struct btrfs_inode *inode, struct bio *bio,
@@ -627,7 +626,6 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	struct extent_map *em;
 	blk_status_t ret = BLK_STS_RESOURCE;
 	int faili = 0;
-	const u32 csum_size = fs_info->csum_size;
 	u8 *sums;
 
 	em_tree = &BTRFS_I(inode)->extent_tree;
@@ -727,7 +725,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 			nr_sectors = DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
 						  fs_info->sectorsize);
-			sums += csum_size * nr_sectors;
+			sums += fs_info->csum_size * nr_sectors;
 
 			ret = btrfs_map_bio(fs_info, comp_bio, mirror_num);
 			if (ret) {
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index aa89d5c15f6e..128eb2d56862 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -452,7 +452,6 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
 	u64 start = page_offset(page);
 	u64 found_start;
 	u8 result[BTRFS_CSUM_SIZE];
-	const u32 csum_size = fs_info->csum_size;
 	struct extent_buffer *eb;
 	int ret;
 
@@ -489,7 +488,7 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
 		WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
 		return ret;
 	}
-	write_extent_buffer(eb, result, 0, csum_size);
+	write_extent_buffer(eb, result, 0, fs_info->csum_size);
 
 	return 0;
 }
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index d7fdfd6ea6f4..3356bd3ae0e0 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -541,7 +541,6 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
 	int i;
 	u64 offset;
 	unsigned nofs_flag;
-	const u32 csum_size = fs_info->csum_size;
 
 	nofs_flag = memalloc_nofs_save();
 	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
@@ -609,7 +608,7 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio,
 					    fs_info->sectorsize,
 					    sums->sums + index);
 			kunmap_atomic(data);
-			index += csum_size;
+			index += fs_info->csum_size;
 			offset += fs_info->sectorsize;
 			this_sum_bytes += fs_info->sectorsize;
 			total_bytes += fs_info->sectorsize;
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index e48810104e4a..367269effd6a 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -137,9 +137,8 @@ static inline int btrfs_ordered_sum_size(struct btrfs_fs_info *fs_info,
 					 unsigned long bytes)
 {
 	int num_sectors = (int)DIV_ROUND_UP(bytes, fs_info->sectorsize);
-	const u32 csum_size = fs_info->csum_size;
 
-	return sizeof(struct btrfs_ordered_sum) + num_sectors * csum_size;
+	return sizeof(struct btrfs_ordered_sum) + num_sectors * fs_info->csum_size;
 }
 
 static inline void
-- 
2.25.0

Re: [PATCH 08/10] btrfs: remove unnecessary local variables for checksum size

[Johannes Thumshirn]

On 29/10/2020 15:29, David Sterba wrote:
> Remove local variable that is then used just once and replace it with
> fs_info::csum_tree.
> 

s/csum_tree/csum_size/

[PATCH 09/10] btrfs: check integrity: remove local copy of csum_size

[David Sterba]

The state structure unnecessarily stores copy of the checksum size, that
can be now easily obtained from fs_info.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/check-integrity.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 2905fe5974e6..d9d95d4a3f12 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -233,7 +233,6 @@ struct btrfsic_stack_frame {
 struct btrfsic_state {
 	u32 print_mask;
 	int include_extent_data;
-	int csum_size;
 	struct list_head all_blocks_list;
 	struct btrfsic_block_hashtable block_hashtable;
 	struct btrfsic_block_link_hashtable block_link_hashtable;
@@ -660,8 +659,6 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 		return -1;
 	}
 
-	state->csum_size = state->fs_info->csum_size;
-
 	for (pass = 0; pass < 3; pass++) {
 		int num_copies;
 		int mirror_num;
@@ -1723,7 +1720,7 @@ static noinline_for_stack int btrfsic_test_for_metadata(
 		crypto_shash_update(shash, data, sublen);
 	}
 	crypto_shash_final(shash, csum);
-	if (memcmp(csum, h->csum, state->csum_size))
+	if (memcmp(csum, h->csum, fs_info->csum_size))
 		return 1;
 
 	return 0; /* is metadata */
@@ -2797,7 +2794,6 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info,
 	state->fs_info = fs_info;
 	state->print_mask = print_mask;
 	state->include_extent_data = including_extent_data;
-	state->csum_size = 0;
 	state->metablock_size = fs_info->nodesize;
 	state->datablock_size = fs_info->sectorsize;
 	INIT_LIST_HEAD(&state->all_blocks_list);
-- 
2.25.0

Re: [PATCH 09/10] btrfs: check integrity: remove local copy of csum_size

[Johannes Thumshirn]

On 29/10/2020 15:29, David Sterba wrote:
> The state structure unnecessarily stores copy of the checksum size, that
> can be now easily obtained from fs_info.

Any reason this is not folded into the previous one?

[PATCH 10/10] btrfs: scrub: remove local copy of csum_size from context

[David Sterba]

The context structure unnecessarily stores copy of the checksum size,
that can be now easily obtained from fs_info.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/scrub.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index d4f693a4ca38..d1c0bfcf368b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -161,7 +161,6 @@ struct scrub_ctx {
 	atomic_t		workers_pending;
 	spinlock_t		list_lock;
 	wait_queue_head_t	list_wait;
-	u16			csum_size;
 	struct list_head	csum_list;
 	atomic_t		cancel_req;
 	int			readonly;
@@ -610,7 +609,6 @@ static noinline_for_stack struct scrub_ctx *scrub_setup_ctx(
 	atomic_set(&sctx->bios_in_flight, 0);
 	atomic_set(&sctx->workers_pending, 0);
 	atomic_set(&sctx->cancel_req, 0);
-	sctx->csum_size = fs_info->csum_size;
 
 	spin_lock_init(&sctx->list_lock);
 	spin_lock_init(&sctx->stat_lock);
@@ -1349,7 +1347,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
 			if (have_csum)
 				memcpy(page->csum,
 				       original_sblock->pagev[0]->csum,
-				       sctx->csum_size);
+				       sctx->fs_info->csum_size);
 
 			scrub_stripe_index_and_offset(logical,
 						      bbio->map_type,
@@ -1800,7 +1798,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	crypto_shash_init(shash);
 	crypto_shash_digest(shash, kaddr, PAGE_SIZE, csum);
 
-	if (memcmp(csum, spage->csum, sctx->csum_size))
+	if (memcmp(csum, spage->csum, sctx->fs_info->csum_size))
 		sblock->checksum_error = 1;
 
 	return sblock->checksum_error;
@@ -1823,7 +1821,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	spage = sblock->pagev[0];
 	kaddr = page_address(spage->page);
 	h = (struct btrfs_header *)kaddr;
-	memcpy(on_disk_csum, h->csum, sctx->csum_size);
+	memcpy(on_disk_csum, h->csum, sctx->fs_info->csum_size);
 
 	/*
 	 * we don't use the getter functions here, as we
@@ -1856,7 +1854,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	}
 
 	crypto_shash_final(shash, calculated_csum);
-	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
+	if (memcmp(calculated_csum, on_disk_csum, sctx->fs_info->csum_size))
 		sblock->checksum_error = 1;
 
 	return sblock->header_error || sblock->checksum_error;
@@ -1893,7 +1891,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 	crypto_shash_digest(shash, kaddr + BTRFS_CSUM_SIZE,
 			BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE, calculated_csum);
 
-	if (memcmp(calculated_csum, s->csum, sctx->csum_size))
+	if (memcmp(calculated_csum, s->csum, sctx->fs_info->csum_size))
 		++fail_cor;
 
 	if (fail_cor + fail_gen) {
@@ -2198,7 +2196,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len,
 		spage->mirror_num = mirror_num;
 		if (csum) {
 			spage->have_csum = 1;
-			memcpy(spage->csum, csum, sctx->csum_size);
+			memcpy(spage->csum, csum, sctx->fs_info->csum_size);
 		} else {
 			spage->have_csum = 0;
 		}
@@ -2390,7 +2388,8 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
 	ASSERT(index < UINT_MAX);
 
 	num_sectors = sum->len >> sctx->fs_info->sectorsize_bits;
-	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
+	memcpy(csum, sum->sums + index * sctx->fs_info->csum_size,
+		sctx->fs_info->csum_size);
 	if (index == num_sectors - 1) {
 		list_del(&sum->list);
 		kfree(sum);
@@ -2508,7 +2507,7 @@ static int scrub_pages_for_parity(struct scrub_parity *sparity,
 		spage->mirror_num = mirror_num;
 		if (csum) {
 			spage->have_csum = 1;
-			memcpy(spage->csum, csum, sctx->csum_size);
+			memcpy(spage->csum, csum, sctx->fs_info->csum_size);
 		} else {
 			spage->have_csum = 0;
 		}
-- 
2.25.0

Re: [PATCH 10/10] btrfs: scrub: remove local copy of csum_size from context

[Johannes Thumshirn]

On 29/10/2020 15:29, David Sterba wrote:
> The context structure unnecessarily stores copy of the checksum size,
> that can be now easily obtained from fs_info.

Same question here, I think this can go into 8/10

Re: [PATCH 10/10] btrfs: scrub: remove local copy of csum_size from context

[David Sterba]

On Thu, Oct 29, 2020 at 02:45:50PM +0000, Johannes Thumshirn wrote:
> On 29/10/2020 15:29, David Sterba wrote:
> > The context structure unnecessarily stores copy of the checksum size,
> > that can be now easily obtained from fs_info.
> 
> Same question here, I think this can go into 8/10

One logical thing per patch: first is removing function local variables, second
is removing a member from integrity checker state and it's use, and the third
is removing member for scrub context.  They're split for ease of review.

Re: [PATCH 10/10] btrfs: scrub: remove local copy of csum_size from context

[Johannes Thumshirn]

On 29/10/2020 15:55, David Sterba wrote:
> One logical thing per patch: first is removing function local variables, second
> is removing a member from integrity checker state and it's use, and the third
> is removing member for scrub context.  They're split for ease of review.
> 

Right then, apart from the typo in 8/10's commit message.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 00/10] Sectorsize, csum_size lifted to fs_info

[Johannes Thumshirn]

On 29/10/2020 15:29, David Sterba wrote:
> Clean up usage of multiplication or division by sectorsize by shifts,
> checksums per leaf are calculated once and csum_size has a copy in
> fs_info so we don't have to read it from raw superblocks.


Currently all checksum sizes are power of 2 as well. Did you check if there
was any benefit if we'd cache csum_size_bits and shift instead of the 
multiplications and divisions?

Re: [PATCH 00/10] Sectorsize, csum_size lifted to fs_info

[David Sterba]

On Thu, Oct 29, 2020 at 02:50:07PM +0000, Johannes Thumshirn wrote:
> On 29/10/2020 15:29, David Sterba wrote:
> > Clean up usage of multiplication or division by sectorsize by shifts,
> > checksums per leaf are calculated once and csum_size has a copy in
> > fs_info so we don't have to read it from raw superblocks.
> 
> Currently all checksum sizes are power of 2 as well. Did you check if there
> was any benefit if we'd cache csum_size_bits and shift instead of the 
> multiplications and divisions?

I had not before, quick grep shows way more csum_size operations that
for sectorsize, so that would be a lot of code churn.