[v3,2/3] btrfs: inode: Cleanup the log tree exceptions in btrfs_truncate_inode_items()
diff mbox series

Message ID 20200515060142.23609-3-wqu@suse.com
State New
Headers show
Series
  • btrfs: REF_COWS bit rework
Related show

Commit Message

Qu Wenruo May 15, 2020, 6:01 a.m. UTC
There are a lot of root owner check in btrfs_truncate_inode_items()
like:

	if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
	    root == fs_info->tree_root)

But considering that, there are only those trees can have INODE_ITEMs:
- tree root (For v1 space cache)
- subvolume trees
- tree reloc trees
- data reloc tree
- log trees

And since subvolume/tree reloc/data reloc trees all have SHAREABLE bit,
and we're checking tree root manually, so above check is just excluding
log trees.

This patch will replace two of such checks to a much simpler one:

	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)

This would merge btrfs_drop_extent_cache() and lock_extent_bits() call
into the same if branch.

Finally replace ALIGN() with round_up(), as I'm always bad at determing
the alignement direction.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

David Sterba May 15, 2020, 7:28 p.m. UTC | #1
On Fri, May 15, 2020 at 02:01:41PM +0800, Qu Wenruo wrote:
> There are a lot of root owner check in btrfs_truncate_inode_items()
> like:
> 
> 	if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
> 	    root == fs_info->tree_root)
> 
> But considering that, there are only those trees can have INODE_ITEMs:
> - tree root (For v1 space cache)
> - subvolume trees
> - tree reloc trees
> - data reloc tree
> - log trees
> 
> And since subvolume/tree reloc/data reloc trees all have SHAREABLE bit,
> and we're checking tree root manually, so above check is just excluding
> log trees.
> 
> This patch will replace two of such checks to a much simpler one:
> 
> 	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)
> 
> This would merge btrfs_drop_extent_cache() and lock_extent_bits() call
> into the same if branch.
> 
> Finally replace ALIGN() with round_up(), as I'm always bad at determing
> the alignement direction.

Alert

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/inode.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a6c26c10ffc5..ae6b47edabc5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4121,20 +4121,18 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
>  		return -ENOMEM;
>  	path->reada = READA_BACK;
>  
> -	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)
> +	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
> +		/*
> +		 * We want to drop from the next block forward in case this
> +		 * new size is not block aligned since we will be keeping the
> +		 * last block of the extent just the way it is.
> +		 */

The comment is misplaced, it belongs to the 'drop extent cache' part

>  		lock_extent_bits(&BTRFS_I(inode)->io_tree, lock_start, (u64)-1,
>  				 &cached_state);
> -
> -	/*
> -	 * We want to drop from the next block forward in case this new size is
> -	 * not block aligned since we will be keeping the last block of the
> -	 * extent just the way it is.
> -	 */
> -	if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
> -	    root == fs_info->tree_root)
> -		btrfs_drop_extent_cache(BTRFS_I(inode), ALIGN(new_size,

here.

Regarding ALIGN -> round_up, please don't put such unrelated changes to
one patch.

Patch
diff mbox series

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a6c26c10ffc5..ae6b47edabc5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4121,20 +4121,18 @@  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 		return -ENOMEM;
 	path->reada = READA_BACK;
 
-	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)
+	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
+		/*
+		 * We want to drop from the next block forward in case this
+		 * new size is not block aligned since we will be keeping the
+		 * last block of the extent just the way it is.
+		 */
 		lock_extent_bits(&BTRFS_I(inode)->io_tree, lock_start, (u64)-1,
 				 &cached_state);
-
-	/*
-	 * We want to drop from the next block forward in case this new size is
-	 * not block aligned since we will be keeping the last block of the
-	 * extent just the way it is.
-	 */
-	if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
-	    root == fs_info->tree_root)
-		btrfs_drop_extent_cache(BTRFS_I(inode), ALIGN(new_size,
+		btrfs_drop_extent_cache(BTRFS_I(inode), round_up(new_size,
 					fs_info->sectorsize),
 					(u64)-1, 0);
+	}
 
 	/*
 	 * This function is also used to drop the items in the log tree before
@@ -4335,8 +4333,7 @@  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 		should_throttle = false;
 
 		if (found_extent &&
-		    (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
-		     root == fs_info->tree_root)) {
+		    root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
 			struct btrfs_ref ref = { 0 };
 
 			bytes_deleted += extent_num_bytes;