From patchwork Sun Jan 27 12:41:55 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Liu Bo X-Patchwork-Id: 2051561 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id EBF38DFE86 for ; Sun, 27 Jan 2013 12:45:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756679Ab3A0MpO (ORCPT ); Sun, 27 Jan 2013 07:45:14 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:19352 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756658Ab3A0Moy (ORCPT ); Sun, 27 Jan 2013 07:44:54 -0500 Received: from acsinet21.oracle.com (acsinet21.oracle.com [141.146.126.237]) by aserp1040.oracle.com (Sentrion-MTA-4.3.1/Sentrion-MTA-4.3.1) with ESMTP id r0RCiaWx027349 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 27 Jan 2013 12:44:36 GMT Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id r0RCiZnG008033 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 27 Jan 2013 12:44:35 GMT Received: from abhmt114.oracle.com (abhmt114.oracle.com [141.146.116.66]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id r0RCiYAd021084; Sun, 27 Jan 2013 06:44:34 -0600 Received: from liubo (/222.90.237.181) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sun, 27 Jan 2013 04:44:33 -0800 Date: Sun, 27 Jan 2013 20:41:55 +0800 From: Liu Bo To: Mitch Harder Cc: linux-btrfs@vger.kernel.org, chris.mason@fusionio.com, JBacik@fusionio.com, dave@jikos.cz, kitayama@cl.bb4u.ne.jp, miaox@cn.fujitsu.com Subject: Re: [PATCH V5] Btrfs: snapshot-aware defrag Message-ID: <20130127124154.GA16722@liubo> Reply-To: bo.li.liu@oracle.com References: <1358339768-2314-1-git-send-email-bo.li.liu@oracle.com> <20130123075155.GE17162@liubo.jp.oracle.com> <20130124005221.GA28406@liubo> <20130125154236.GA3997@liubo> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: acsinet21.oracle.com [141.146.126.237] Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Fri, Jan 25, 2013 at 12:16:29PM -0600, Mitch Harder wrote: [...] > >> > >> I've changed up my reproducer to try some things that may hit the > >> issue quicker and more reliably. > >> > >> It gave me a slightly different set of warnings in dmesg, which seem > >> to suggest issues in the dead_root list. > > > > Great! Many thanks for nail it down, we really shouldn't iput() > > after btrfs_iget(). > > > > Could you please try this(remove iput()) and see if it gets us rid of > > the trouble? > > > > thanks, > > liubo > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 1683f48..c7a0fb7 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -2337,7 +2337,6 @@ out_free_path: > > out_unlock: > > unlock_extent_cached(&BTRFS_I(inode)->io_tree, lock_start, > > lock_end, > > &cached, GFP_NOFS); > > - iput(inode); > > return ret; > > } > > > > With this patch, the cleaner never runs to delete the old roots. Hi Mitch, Many thanks for testing it! Well, after some debugging, I finally figure out the whys: (1) btrfs_ioctl_snap_destroy() will free the inode of snapshot and set root's refs to zero(btrfs_set_root_refs()), if this inode happens to be the only one in the rbtree of the snapshot's root at this moment, we add this root to the dead_root list. (2) Unfortunately, after (1), our snapshot-aware defrag work may read another inode in this snapshot into memory during 'relink' stage, and later after we finish relink work and iput() will force us to add the snapshot's root to the dead_root list again. So that's why we get double list_add and list_del corruption. And IMO, it can also take place without snapshot-aware defrag, but it's a rare case. So could you please try this? thanks, liubo --- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index f154946..d4ee66b 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -885,7 +885,15 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans, int btrfs_add_dead_root(struct btrfs_root *root) { spin_lock(&root->fs_info->trans_lock); + if (!list_empty(&root->root_list)) { + struct btrfs_root *tmp; + list_for_each_entry(tmp, &root->fs_info->dead_roots, root_list) + if (tmp == root) + goto unlock; + } + list_add(&root->root_list, &root->fs_info->dead_roots); +unlock: spin_unlock(&root->fs_info->trans_lock); return 0; }