From patchwork Thu May 19 10:49:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xiaoguang Wang X-Patchwork-Id: 9126321 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E64C29F37F for ; Thu, 19 May 2016 10:50:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 66958200EC for ; Thu, 19 May 2016 10:50:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 21842200E3 for ; Thu, 19 May 2016 10:50:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754193AbcESKt4 (ORCPT ); Thu, 19 May 2016 06:49:56 -0400 Received: from cn.fujitsu.com ([59.151.112.132]:7102 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1754181AbcESKtz (ORCPT ); Thu, 19 May 2016 06:49:55 -0400 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="6705632" Received: from unknown (HELO cn.fujitsu.com) ([10.167.33.5]) by heian.cn.fujitsu.com with ESMTP; 19 May 2016 18:49:53 +0800 Received: from G08CNEXCHPEKD01.g08.fujitsu.local (unknown [10.167.33.80]) by cn.fujitsu.com (Postfix) with ESMTP id 70C97489F960 for ; Thu, 19 May 2016 18:49:49 +0800 (CST) Received: from localhost.localdomain (10.167.226.107) by G08CNEXCHPEKD01.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP Server (TLS) id 14.3.279.2; Thu, 19 May 2016 18:49:49 +0800 From: Wang Xiaoguang To: CC: Wang Xiaoguang Subject: [RFC PATCH] btrfs: correct inode's outstanding_extents computation Date: Thu, 19 May 2016 18:49:09 +0800 Message-ID: <1463654949-8295-1-git-send-email-wangxg.fnst@cn.fujitsu.com> X-Mailer: git-send-email 2.5.0 MIME-Version: 1.0 X-Originating-IP: [10.167.226.107] X-yoursite-MailScanner-ID: 70C97489F960.A5E0D X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: wangxg.fnst@cn.fujitsu.com X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This issue was revealed by modifing BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, When modifing BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often gets these warnings from btrfs_destroy_inode(): WARN_ON(BTRFS_I(inode)->outstanding_extents); WARN_ON(BTRFS_I(inode)->reserved_extents); Simple test program below can reproduce this issue steadily. #include #include #include #include #include int main(void) { int fd; char buf[1024*1024]; memset(buf, 0, 1024 * 1024); fd = open("testfile", O_CREAT | O_EXCL | O_RDWR); pwrite(fd, buf, 69954, 693581); return; } Assume the BTRFS_MAX_EXTENT_SIZE is 64KB, and data range is: 692224 765951 |----------------------------------------------------------------------------------| len(73728) 1) for the above data range, btrfs_delalloc_reserve_metadata() will reserve metadata and BTRFS_I(inode)->outstanding_extents will be 2. (73728 + 65535) / 65536 == 2 2) then btrfs_dirty_page() will be called to dirty pages and set EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook will be called 3 times. For first call, there will be such extent io map. 692224 696319 696320 765951 |----------------------| |-----------------------------------------------------| len(4096) len(69632) have EXTENT_DELALLOC and because of having EXTENT_FIRST_DELALLOC, btrfs_set_bit_hook() won't change BTRFS_I(inode)->outstanding_extents, still be 2. see code logic in btrfs_set_bit_hook(); 3) second btrfs_set_bit_hook() call. Because of EXTENT_FIRST_DELALLOC have been unset by previous btrfs_set_bit_hook(), btrfs_set_bit_hook will increase BTRFS_I(inode)->outstanding_extents by one, so now BTRFS_I(inode)->outstanding_extents, sitll is 3. There will be such extent_io map: 692224 696319 696320 761855 761856 765951 |--------------------| |---------------------| |--------------------------| len(4096) len(65536) len(4096) have EXTENT_DELALLOC have EXTENT_DELALLOC And because (692224, 696319) and (696320, 761855) is adjacent, btrfs_merge_extent_hook() will merge them into one delalloc extent, but according to the compulation logic in btrfs_merge_extent_hook(), BTRFS_I(inode)->outstanding_extents will still be 3. After merge, tehre will bu such extent_io map: 692224 761855 761856 765951 |-------------------------------------------------| |--------------------------| len(69632) len(4096) have EXTENT_DELALLOC 4) third btrfs_set_bit_hook() call. Also because of EXTENT_FIRST_DELALLOC have not been set, btrfs_set_bit_hook will increase BTRFS_I(inode)->outstanding_extents by one, so now BTRFS_I(inode)->outstanding_extents is 4. The extent io map is: 692224 761855 761856 765951 |-------------------------------------------------| |--------------------------| len(69632) len(4096) have EXTENT_DELALLOC have EXTENT_DELALLOC Also because (692224, 761855) and (761856, 765951) is adjacent, btrfs_merge_extent_hook() will merge them into one delalloc extent, according to the compulation logic in btrfs_merge_extent_hook(), BTRFS_I(inode)->outstanding_extents will decrease by one, be 3. so after merge, tehre will bu such extent_io map: 692224 765951 |-----------------------------------------------------------------------------------| len(73728) have EXTENT_DELALLOC But indeed for original data range(start:692224 end:765951 len:73728), we just should have 2 outstanding extents, so it will trigger the above WARNINGs. The root casue is that btrfs_delalloc_reserve_metadata() will always add needed outstanding extents first, and if later btrfs_set_extent_delalloc call multiple btrfs_set_bit_hook(), it may wrongly update BTRFS_I(inode)->outstanding_extents, This patch choose to also add BTRFS_I(inode)->outstanding_extents in btrfs_set_bit_hook() according to the data range length, and the added value is the correct number of outstanding_extents for this data range, then decrease the value which was added in btrfs_delalloc_reserve_metadata(). As for why BTRFS_MAX_EXTENT_SIZE(128M) does not trigger above WARNINGs, this is because __btrfs_buffered_write() internally have write limits for every iteration(it seems 2MB), so btrfs_dirty_pages() will always make data range into one outstanding extent. Signed-off-by: Wang Xiaoguang --- fs/btrfs/ctree.h | 2 ++ fs/btrfs/inode.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- fs/btrfs/ioctl.c | 5 ++--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 84a6a5b..da9ee24 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -4072,6 +4072,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput, int nr); int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, struct extent_state **cached_state); +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end, + struct extent_state **cached_state); int btrfs_create_subvol_root(struct btrfs_trans_handle *trans, struct btrfs_root *new_root, struct btrfs_root *parent_root, diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 41a5688..5144f45 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -1713,13 +1713,16 @@ static void btrfs_set_bit_hook(struct inode *inode, if (!(state->state & EXTENT_DELALLOC) && (*bits & EXTENT_DELALLOC)) { struct btrfs_root *root = BTRFS_I(inode)->root; u64 len = state->end + 1 - state->start; + u64 num_extents = div64_u64(len + BTRFS_MAX_EXTENT_SIZE - 1, + BTRFS_MAX_EXTENT_SIZE); bool do_list = !btrfs_is_free_space_inode(inode); - if (*bits & EXTENT_FIRST_DELALLOC) { + if (*bits & EXTENT_FIRST_DELALLOC) *bits &= ~EXTENT_FIRST_DELALLOC; - } else { + + if (root != root->fs_info->tree_root) { spin_lock(&BTRFS_I(inode)->lock); - BTRFS_I(inode)->outstanding_extents++; + BTRFS_I(inode)->outstanding_extents += num_extents; spin_unlock(&BTRFS_I(inode)->lock); } @@ -1960,9 +1963,43 @@ static noinline int add_pending_csums(struct btrfs_trans_handle *trans, int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, struct extent_state **cached_state) { + int ret; + struct btrfs_root *root = BTRFS_I(inode)->root; + u64 num_extents = div64_u64(end - start + BTRFS_MAX_EXTENT_SIZE, + BTRFS_MAX_EXTENT_SIZE); + + WARN_ON((end & (PAGE_CACHE_SIZE - 1)) == 0); + ret = set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, + cached_state, GFP_NOFS); + + if (root != root->fs_info->tree_root) { + spin_lock(&BTRFS_I(inode)->lock); + BTRFS_I(inode)->outstanding_extents -= num_extents; + spin_unlock(&BTRFS_I(inode)->lock); + } + + return ret; +} + +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end, + struct extent_state **cached_state) +{ + int ret; + struct btrfs_root *root = BTRFS_I(inode)->root; + u64 num_extents = div64_u64(end - start + BTRFS_MAX_EXTENT_SIZE, + BTRFS_MAX_EXTENT_SIZE); + WARN_ON((end & (PAGE_CACHE_SIZE - 1)) == 0); - return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, - cached_state, GFP_NOFS); + ret = set_extent_defrag(&BTRFS_I(inode)->io_tree, start, end, + cached_state, GFP_NOFS); + + if (root != root->fs_info->tree_root) { + spin_lock(&BTRFS_I(inode)->lock); + BTRFS_I(inode)->outstanding_extents -= num_extents; + spin_unlock(&BTRFS_I(inode)->lock); + } + + return ret; } /* see btrfs_writepage_start_hook for details on why this is required */ diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 21423dd..149d11e 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1227,9 +1227,8 @@ again: } - set_extent_defrag(&BTRFS_I(inode)->io_tree, page_start, page_end - 1, - &cached_state, GFP_NOFS); - + btrfs_set_extent_defrag(inode, page_start, + page_end - 1, &cached_state); unlock_extent_cached(&BTRFS_I(inode)->io_tree, page_start, page_end - 1, &cached_state, GFP_NOFS);