From patchwork Sun Apr 27 19:44:25 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Filipe Manana X-Patchwork-Id: 4072871 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 50B2EBFF02 for ; Sun, 27 Apr 2014 18:46:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 308CB2020E for ; Sun, 27 Apr 2014 18:46:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 15C3720179 for ; Sun, 27 Apr 2014 18:46:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751846AbaD0Sqv (ORCPT ); Sun, 27 Apr 2014 14:46:51 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:47768 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751335AbaD0Sqt (ORCPT ); Sun, 27 Apr 2014 14:46:49 -0400 Received: by mail-wi0-f176.google.com with SMTP id r20so4721594wiv.9 for ; Sun, 27 Apr 2014 11:46:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=nSminGhV+Bw80E8x64gRzMF7ljpfiM2A/citGbbkv2U=; b=hO/nFX7ainUeEVAsgwPkmpQYwMOSLAvxEbD2sqqZS3BQW0SZU31JKjorVrN72mhq1f 9CoGq0mx81W7fOFcmH/k32UcN2WvYB2PDMro2HtSvhxhNz80InmAldHODK93E2aXB5K8 IphhDoOk3wR9h6Zf3b8iTGBzTXwkwVteTyVAs+vf1jIVxvhrK+4Ln6r3ypXTYd0moVs3 VrmxbxLi+uE9wEj6BObUGDlW2bUW7qWfW7s+LzUGtT+Ni4uf+ADBeSqghlb7RuJv9G4y vUrZF6M8579xy3yU7s/NwzzHcrZm9+6QQ+ch+raAzNQgR+WX7/tevo3SRW23qQbqqmQt tkFA== X-Received: by 10.180.39.175 with SMTP id q15mr12205698wik.4.1398624408067; Sun, 27 Apr 2014 11:46:48 -0700 (PDT) Received: from debian-vm3.lan (bl10-196-46.dsl.telepac.pt. [85.243.196.46]) by mx.google.com with ESMTPSA id km2sm22207085wjb.13.2014.04.27.11.46.46 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 27 Apr 2014 11:46:47 -0700 (PDT) From: Filipe David Borba Manana To: linux-btrfs@vger.kernel.org Cc: clm@fb.com, Filipe David Borba Manana Subject: [PATCH] Btrfs: fix leaf corruption caused by ENOSPC while hole punching Date: Sun, 27 Apr 2014 20:44:25 +0100 Message-Id: <1398627865-12223-1-git-send-email-fdmanana@gmail.com> X-Mailer: git-send-email 1.9.1 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP While running a stress test with multiple threads writing to the same btrfs file system, I ended up with a situation where a leaf was corrupted in that it had 2 file extent item keys that had the same exact key. I was able to detect this quickly thanks to the following patch which triggers an assertion as soon as a leaf is marked dirty if there are duplicated keys or out of order keys: Btrfs: check if items are ordered when a leaf is marked dirty (https://patchwork.kernel.org/patch/3955431/) Basically while running the test, I got the following in dmesg: [28877.415877] WARNING: CPU: 2 PID: 10706 at fs/btrfs/file.c:553 btrfs_drop_extent_cache+0x435/0x440 [btrfs]() (...) [28877.415917] Call Trace: [28877.415922] [] dump_stack+0x4e/0x68 [28877.415926] [] warn_slowpath_common+0x8c/0xc0 [28877.415929] [] warn_slowpath_null+0x1a/0x20 [28877.415944] [] btrfs_drop_extent_cache+0x435/0x440 [btrfs] [28877.415949] [] ? kmem_cache_alloc+0xfe/0x1c0 [28877.415962] [] fill_holes+0x229/0x3e0 [btrfs] [28877.415972] [] ? block_rsv_add_bytes+0x55/0x80 [btrfs] [28877.415984] [] btrfs_fallocate+0xb6b/0xc20 [btrfs] (...) [29854.132560] BTRFS critical (device sdc): corrupt leaf, bad key order: block=955232256,root=1, slot=24 [29854.132565] BTRFS info (device sdc): leaf 955232256 total ptrs 40 free space 778 (...) [29854.132637] item 23 key (3486 108 667648) itemoff 2694 itemsize 53 [29854.132638] extent data disk bytenr 14574411776 nr 286720 [29854.132639] extent data offset 0 nr 286720 ram 286720 [29854.132640] item 24 key (3486 108 954368) itemoff 2641 itemsize 53 [29854.132641] extent data disk bytenr 0 nr 0 [29854.132643] extent data offset 0 nr 0 ram 0 [29854.132644] item 25 key (3486 108 954368) itemoff 2588 itemsize 53 [29854.132645] extent data disk bytenr 8699670528 nr 77824 [29854.132646] extent data offset 0 nr 77824 ram 77824 [29854.132647] item 26 key (3486 108 1146880) itemoff 2535 itemsize 53 [29854.132648] extent data disk bytenr 8699670528 nr 77824 [29854.132649] extent data offset 0 nr 77824 ram 77824 (...) [29854.132707] kernel BUG at fs/btrfs/ctree.h:3901! (...) [29854.132771] Call Trace: [29854.132779] [] setup_items_for_insert+0x2dc/0x400 [btrfs] [29854.132791] [] __btrfs_drop_extents+0xba7/0xdd0 [btrfs] [29854.132794] [] ? trace_hardirqs_on_caller+0x16/0x1d0 [29854.132797] [] ? trace_hardirqs_on+0xd/0x10 [29854.132800] [] ? kmem_cache_alloc+0xfe/0x1c0 [29854.132810] [] insert_reserved_file_extent.constprop.66+0xab/0x310 [btrfs] [29854.132820] [] __btrfs_prealloc_file_range+0x116/0x340 [btrfs] [29854.132830] [] btrfs_prealloc_file_range+0x23/0x30 [btrfs] (...) So this is caused by getting an -ENOSPC error while punching a file hole, more specifically, we get -ENOSPC error from __btrfs_drop_extents in the while loop of file.c:btrfs_punch_hole() when it's unable to modify the btree to delete one or more file extent items due to lack of enough free space. When this happens, in btrfs_punch_hole(), we attempt to reclaim free space by switching our transaction block reservation object to root->fs_info->trans_block_rsv, end our transaction and start a new transaction basically - and, we keep increasing our current offset (cur_offset) as long as it's smaller than the end of the target range (lockend) - this makes use leave the loop with cur_offset == drop_end which in turn makes us call fill_holes() for inserting a file extent item that represents a 0 bytes range hole (and this insertion succeeds, as in the meanwhile more space became available). This 0 bytes file hole extent item is a problem because any subsequent caller of __btrfs_drop_extents (regular file writes, or fallocate calls for e.g.), with a start file offset that is equal to the offset of the hole, will not remove this extent item due to the following conditional in the while loop of __btrfs_drop_extents: if (extent_end <= search_start) { path->slots[0]++; goto next_slot; } This later makes the call to setup_items_for_insert() (at the very end of __btrfs_drop_extents), insert a new file extent item with the same offset as the 0 bytes file hole extent item that follows it. Needless is to say that this causes chaos, either when reading the leaf from disk (btree_readpage_end_io_hook), where we perform leaf sanity checks or in subsequent operations that manipulate file extent items, as in the fallocate call as shown by the dmesg trace above. Without my other patch to perform the leaf sanity checks once a leaf is marked as dirty (if the integrity checker is enabled), it would have been much harder to debug this issue. This change might fix a few similar issues reported by users in the mailing list regarding assertion failures in btrfs_set_item_key_safe calls performed by __btrfs_drop_extents, such as the following report: http://comments.gmane.org/gmane.comp.file-systems.btrfs/32938 Asking fill_holes() to create a 0 bytes wide file hole item also produced the first warning in the trace above, as we passed a range to btrfs_drop_extent_cache that has an end smaller (by -1) than its start. Signed-off-by: Filipe David Borba Manana --- fs/btrfs/file.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 49e5fbf..cac902a 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -785,7 +785,17 @@ next_slot: extent_end = search_start; } - if (extent_end <= search_start) { + /* + * Don't skip extent items representing 0 byte lengths. They + * used to be created (bug) if while punching holes we hit + * -ENOSPC condition. So if we find one here, just ensure we + * delete it, otherwise we would insert a new file extent item + * with the same key (offset) as that 0 bytes length file + * extent item in the call to setup_items_for_insert() later + * in this function. + */ + if (extent_end <= search_start && + !(extent_end == key.offset && extent_end == search_start)) { path->slots[0]++; goto next_slot; } @@ -2353,7 +2363,12 @@ static int btrfs_punch_hole(struct inode *inode, loff_t offset, loff_t len) } trans->block_rsv = &root->fs_info->trans_block_rsv; - if (cur_offset < ino_size) { + /* + * Don't insert file hole extent item if it's for a range beyond eof + * (because it's useless) or if it represents a 0 bytes range (when + * cur_offset == drop_end). + */ + if (cur_offset < ino_size && cur_offset < drop_end) { ret = fill_holes(trans, inode, path, cur_offset, drop_end); if (ret) { err = ret;