From patchwork Tue Oct 18 01:31:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Qu Wenruo X-Patchwork-Id: 9380975 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8486160487 for ; Tue, 18 Oct 2016 01:32:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 85AE62909D for ; Tue, 18 Oct 2016 01:32:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 79E2E2916D; Tue, 18 Oct 2016 01:32:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0930A29081 for ; Tue, 18 Oct 2016 01:32:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758599AbcJRBb6 (ORCPT ); Mon, 17 Oct 2016 21:31:58 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:55403 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1757828AbcJRBb4 (ORCPT ); Mon, 17 Oct 2016 21:31:56 -0400 X-IronPort-AV: E=Sophos;i="5.20,367,1444665600"; d="scan'208";a="912435" Received: from unknown (HELO cn.fujitsu.com) ([10.167.250.3]) by song.cn.fujitsu.com with ESMTP; 18 Oct 2016 09:31:32 +0800 Received: from adam-work.localdomain (unknown [10.167.226.34]) by cn.fujitsu.com (Postfix) with ESMTP id 658C141B4BC8; Tue, 18 Oct 2016 09:31:31 +0800 (CST) From: Qu Wenruo To: linux-btrfs@vger.kernel.org Cc: rgoldwyn@suse.com, rgoldwyn@suse.de Subject: [PATCH 4/4] btrfs: qgroup: Fix qgroup data leaking by using subtree tracing Date: Tue, 18 Oct 2016 09:31:29 +0800 Message-Id: <20161018013129.23331-5-quwenruo@cn.fujitsu.com> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20161018013129.23331-1-quwenruo@cn.fujitsu.com> References: <20161018013129.23331-1-quwenruo@cn.fujitsu.com> MIME-Version: 1.0 X-yoursite-MailScanner-ID: 658C141B4BC8.AD4C9 X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: quwenruo@cn.fujitsu.com Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commit 62b99540a1d91e464 (btrfs: relocation: Fix leaking qgroups numbers on data extents) only fixes the problem partly. The previous fix is to trace all new data extents at transaction commit time when balance finishes. However balance is not done in a large transaction, every path replacement can happen in its own transaction. This makes the fix useless if transaction commits during relocation. For example: relocate_block_group() |-merge_reloc_roots() | |- merge_reloc_root() | |- btrfs_start_transaction() <- Trans X | |- replace_path() <- Cause leak | |- btrfs_end_transaction_throttle() <- Trans X commits here | | Leak not fixed | | | |- btrfs_start_transaction() <- Trans Y | |- replace_path() <- Cause leak | |- btrfs_end_transaction_throttle() <- Trans Y ends | but not committed |-btrfs_join_transaction() <- Still trans Y |-qgroup_fix() <- Only fixes data leak | in trans Y |-btrfs_commit_transaction() <- Trans Y commits In that case, qgroup fixup can only fix data leak in trans Y, data leak in trans X is out of fix. So the correct fix should happens in the same transaction of replace_path(). This patch fixes it by tracing both subtrees of tree block swap, so it can fix the problem and ensure all leaking and fix are in the same transaction, so no leak again. Reported-by: Goldwyn Rodrigues Signed-off-by: Qu Wenruo --- fs/btrfs/relocation.c | 119 ++++++++++---------------------------------------- 1 file changed, 23 insertions(+), 96 deletions(-) diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 33cde30..540f225 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -1901,6 +1901,29 @@ again: BUG_ON(ret); /* + * Info qgroup to trace both subtrees. + * + * We must trace both trees. + * 1) Tree reloc subtree + * If not traced, we will leak data numbers + * 2) Fs subtree + * If not traced, we will double count old data + * and tree block numbers, if current trans doesn't free + * data reloc tree inode. + */ + ret = btrfs_qgroup_trace_subtree(trans, src, parent, + btrfs_header_generation(parent), + btrfs_header_level(parent)); + if (ret < 0) + break; + ret = btrfs_qgroup_trace_subtree(trans, dest, + path->nodes[level], + btrfs_header_generation(path->nodes[level]), + btrfs_header_level(path->nodes[level])); + if (ret < 0) + break; + + /* * swap blocks in fs tree and reloc tree. */ btrfs_set_node_blockptr(parent, slot, new_bytenr); @@ -3942,90 +3965,6 @@ 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 (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)) - 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, &key, path, 0, 0); - if (ret < 0) - goto out; - - lock_extent(&BTRFS_I(inode)->io_tree, 0, (u64)-1); - while (1) { - struct btrfs_file_extent_item *fi; - - btrfs_item_key_to_cpu(path->nodes[0], &key, 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_trace_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 = btrfs_next_item(data_reloc_root, path); - if (ret < 0) - break; - if (ret > 0) { - ret = 0; - break; - } - } - unlock_extent(&BTRFS_I(inode)->io_tree, 0 , (u64)-1); -out: - btrfs_free_path(path); - return ret; -} - static noinline_for_stack int relocate_block_group(struct reloc_control *rc) { struct rb_root blocks = RB_ROOT; @@ -4216,13 +4155,6 @@ restart: err = PTR_ERR(trans); goto out_free; } - ret = qgroup_fix_relocated_data_extents(trans, rc); - if (ret < 0) { - btrfs_abort_transaction(trans, ret); - if (!err) - err = ret; - goto out_free; - } btrfs_commit_transaction(trans, rc->extent_root); out_free: btrfs_free_block_rsv(rc->extent_root, rc->block_rsv); @@ -4591,11 +4523,6 @@ int btrfs_recover_relocation(struct btrfs_root *root) err = PTR_ERR(trans); goto out_free; } - err = qgroup_fix_relocated_data_extents(trans, rc); - if (err < 0) { - btrfs_abort_transaction(trans, err); - goto out_free; - } err = btrfs_commit_transaction(trans, rc->extent_root); out_free: kfree(rc);