Re: [PATCH v3.1 2/3] btrfs: relocation: Fix leaking qgroups numbers on data extents

[Goldwyn Rodrigues]

On 08/14/2016 09:36 PM, Qu Wenruo wrote:
> This patch fixes a REGRESSION introduced in 4.2, caused by the big quota
> rework.
> 
> When balancing data extents, qgroup will leak all its numbers for
> relocated data extents.
> 
> The relocation is done in the following steps for data extents:
> 1) Create data reloc tree and inode
> 2) Copy all data extents to data reloc tree
>And commit transaction
> 3) Create tree reloc tree(special snapshot) for any related subvolumes
> 4) Replace file extent in tree reloc tree with new extents in data reloc
>tree
>And commit transaction
> 5) Merge tree reloc tree with original fs, by swapping tree blocks
> 
> For 1)~4), since tree reloc tree and data reloc tree doesn't count to
> qgroup, everything is OK.
> 
> But for 5), the swapping of tree blocks will only info qgroup to track
> metadata extents.
> 
> If metadata extents contain file extents, qgroup number for file extents
> will get lost, leading to corrupted qgroup accounting.
> 
> The fix is, before commit transaction of step 5), manually info qgroup to
> track all file extents in data reloc tree.
> Since at commit transaction time, the tree swapping is done, and qgroup
> will account these data extents correctly.
> 
> Cc: Mark Fasheh 
> Reported-by: Mark Fasheh 
> Reported-by: Filipe Manana 
> Signed-off-by: Qu Wenruo 
> Tested-by: Goldwyn Rodrigues 
> ---
>  fs/btrfs/relocation.c | 109 
> +++---
>  1 file changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index b26a5ae..27480ef 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -31,6 +31,7 @@
>  #include "async-thread.h"
>  #include "free-space-cache.h"
>  #include "inode-map.h"
> +#include "qgroup.h"
>  
>  /*
>   * backref_node, mapping_node and tree_block start with this
> @@ -3916,6 +3917,90 @@ int prepare_to_relocate(struct reloc_control *rc)
>   return 0;
>  }
>  
> +/*
> + * Qgroup fixer for data chunk relocation.
> + * The data relocation is done in the following steps
> + * 1) Copy data extents into data reloc tree
> + * 2) Create tree reloc tree(special snapshot) for related subvolumes
> + * 3) Modify file extents in tree reloc tree
> + * 4) Merge tree reloc tree with original fs tree, by swapping tree blocks
> + *
> + * The problem is, data and tree reloc tree are not accounted to qgroup,
> + * and 4) will only info qgroup to track tree blocks change, not file extents
> + * in the tree blocks.
> + *
> + * The good news is, related data extents are all in data reloc tree, so we
> + * only need to info qgroup to track all file extents in data reloc tree
> + * before commit trans.
> + */
> +static int qgroup_fix_relocated_data_extents(struct btrfs_trans_handle 
> *trans,
> +  struct reloc_control *rc)
> +{
> + struct btrfs_fs_info *fs_info = rc->extent_root->fs_info;
> + struct inode *inode = rc->data_inode;
> + struct btrfs_root *data_reloc_root = BTRFS_I(inode)->root;
> + struct btrfs_path *path;
> + struct btrfs_key key;
> + int ret = 0;
> +
> + if (!fs_info->quota_enabled)
> + return 0;
> +
> + /*
> +  * Only for stage where we update data pointers the qgroup fix is
> +  * valid.
> +  * For MOVING_DATA stage, we will miss the timing of swapping tree
> +  * blocks, and won't fix it.
> +  */
> + if (!(rc->stage == UPDATE_DATA_PTRS && rc->extents_found))
> + return 0;
> +
> + path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
> + key.objectid = btrfs_ino(inode);
> + key.type = BTRFS_EXTENT_DATA_KEY;
> + key.offset = 0;
> +
> + ret = btrfs_search_slot(NULL, data_reloc_root, , path, 0, 0);
> + if (ret < 0)
> + goto out;
> +
> + lock_extent(_I(inode)->io_tree, 0, (u64)-1);
> + while (1) {
> + struct btrfs_file_extent_item *fi;
> +
> + btrfs_item_key_to_cpu(path->nodes[0], , path->slots[0]);
> + if (key.objectid > btrfs_ino(inode))
> + break;
> + if (key.type != BTRFS_EXTENT_DATA_KEY)
> + goto next;
> + fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
> + struct btrfs_file_extent_item);
> + if (btrfs_file_extent_type(path->nodes[0], fi) !=
> + BTRFS_FILE_EXTENT_REG)
> + goto next;
> + ret = btrfs_qgroup_insert_dirty_extent(trans, fs_info,
> + btrfs_file_extent_disk_bytenr(path->nodes[0], fi),
> + btrfs_file_extent_disk_num_bytes(path->nodes[0], fi),
> + GFP_NOFS);
> + if (ret < 0)
> + break;
> +next:
> + ret = 

Re: Problem with btrfs fs in 4.1.25 (also 4.8.0) - getting EBUSY on file create - kernel.org bug 173001

[Dave Olson]

Dave Olson  wrote:

> The full details are in
> https://bugzilla.kernel.org/show_bug.cgi?id=173001
> 
> Basicly, logrotate has rotated a file to a new name, tries to open a new
> file with the original name, and gets EBUSY.  The file is not created.
> 
> Later on the file can be created with no problems.
> 
> I've turned on most of the BTRFS config options, and there are no BTRFS
> messages related to the failure.
> 
> The file is on the root filesystem.

This problem is also reproducible on a 4.8 kernel.org kernel with no
modifications, built today from the 4.8 tag.

It happened on the 77th powercycle, after about 3 hours.

Dave Olson
ol...@cumulusnetworks.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html