On 2017年12月20日 12:57, Su Yue wrote:
> Since extents can be avoid overwrite by excluding or new chunk
> allocation. It's unnessesary to do all repairs in one transaction.
> 
> This patch removes parameter @trans of repair_extent_data_item().
> repair_extent_data_item() calls try_avoid_extents_overwrite()
> and starts a transaction by itself.
> 
> Note: This patch and next patches cause error in lowmem repair like:
> "Error: Commit_root already set when starting transaction".
> This error will disappear after removing @trans finished.
> 
> Signed-off-by: Su Yue <[email protected]>
> ---
>  cmds-check.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index bc405d0f3fc0..20bf3d230946 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -12064,18 +12064,19 @@ out:
>       return err;
>  }
>  
> +static int try_avoid_extents_overwrite(struct btrfs_fs_info *fs_info);
>  /*
>   * If @err contains BACKREF_MISSING then add extent of the
>   * file_extent_data_item.
>   *
>   * Returns error bits after reapir.
>   */
> -static int repair_extent_data_item(struct btrfs_trans_handle *trans,
> -                                struct btrfs_root *root,
> +static int repair_extent_data_item(struct btrfs_root *root,
>                                  struct btrfs_path *pathp,
>                                  struct node_refs *nrefs,
>                                  int err)
>  {
> +     struct btrfs_trans_handle *trans = NULL;
>       struct btrfs_file_extent_item *fi;
>       struct btrfs_key fi_key;
>       struct btrfs_key key;
> @@ -12092,6 +12093,7 @@ static int repair_extent_data_item(struct 
> btrfs_trans_handle *trans,
>       u64 file_offset;
>       int generation;
>       int slot;
> +     int need_insert = 0;
>       int ret = 0;
>  
>       eb = pathp->nodes[0];
> @@ -12130,9 +12132,20 @@ static int repair_extent_data_item(struct 
> btrfs_trans_handle *trans,
>               ret = -EIO;
>               goto out;
>       }
> +     need_insert = ret;
>  
> +     ret = try_avoid_extents_overwrite(root->fs_info);

Even it's much cheaper to pin extent tree using chunk unit, I don't
think it's a good idea to do it every time you are going to fix some
extent tree problem.

Why not just do it at the beginning of extent tree check?

Thanks,
Qu

> +     if (ret)
> +             goto out;
> +     trans = btrfs_start_transaction(root, 1);
> +     if (IS_ERR(trans)) {
> +             ret = PTR_ERR(trans);
> +             trans = NULL;
> +             error("fail to start transaction %s", strerror(-ret));
> +             goto out;
> +     }
>       /* insert an extent item */
> -     if (ret > 0) {
> +     if (need_insert) {
>               key.objectid = disk_bytenr;
>               key.type = BTRFS_EXTENT_ITEM_KEY;
>               key.offset = num_bytes;
> @@ -12172,6 +12185,8 @@ static int repair_extent_data_item(struct 
> btrfs_trans_handle *trans,
>  
>       err &= ~BACKREF_MISSING;
>  out:
> +     if (trans)
> +             btrfs_commit_transaction(trans, root);
>       btrfs_release_path(&path);
>       if (ret)
>               error("can't repair root %llu extent data item[%llu %llu]",
> @@ -13445,8 +13460,7 @@ again:
>       case BTRFS_EXTENT_DATA_KEY:
>               ret = check_extent_data_item(root, path, nrefs, account_bytes);
>               if (repair && ret)
> -                     ret = repair_extent_data_item(trans, root, path, nrefs,
> -                                                   ret);
> +                     ret = repair_extent_data_item(root, path, nrefs, ret);
>               err |= ret;
>               break;
>       case BTRFS_BLOCK_GROUP_ITEM_KEY:
> 

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to