From patchwork Tue Jun 14 03:24:45 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Yan, Zheng" X-Patchwork-Id: 877682 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5E3Onsr027589 for ; Tue, 14 Jun 2011 03:24:49 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751509Ab1FNDYq (ORCPT ); Mon, 13 Jun 2011 23:24:46 -0400 Received: from mail-iw0-f174.google.com ([209.85.214.174]:61168 "EHLO mail-iw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751225Ab1FNDYp convert rfc822-to-8bit (ORCPT ); Mon, 13 Jun 2011 23:24:45 -0400 Received: by iwn34 with SMTP id 34so4345215iwn.19 for ; Mon, 13 Jun 2011 20:24:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=QY+lAfLTuEjQGK7GIBfPMV7/S2DiE+4r6XZl4F3FfzQ=; b=R83SkVqKvOopUakTy7/KXtOzUCXI8yn8pzXHTdt/gK4Yrmx3kO+0+bfY2/mNlGSiNG 2TasOBR3fkKZYg8vRqr1E6eMZg72lWCF0GoqkHGdcccMjX39U6I3bQP0Q2WYwvQKPy0R oYCjEiY8SRc5/PltKnXy6eJYUvgvKoRnAZiUQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=bznmSZX4G5DQjFhxGetZIM7NHZQE8OsmExXdf9DemsFMV8VcE+W2VVDESZKMj/Tl9n QGIrM5XtSrLSI+k/m6NJhCGkPo9Eozw+Fv3yeuhYJnz2AKu+Ay+LVf3M8zieNRo7tpxu oc2xuDMzZgguNbA+Y4bQOiwh0BhV9wEiocHWA= MIME-Version: 1.0 Received: by 10.42.146.196 with SMTP id k4mr7026654icv.105.1308021885371; Mon, 13 Jun 2011 20:24:45 -0700 (PDT) Received: by 10.231.13.133 with HTTP; Mon, 13 Jun 2011 20:24:45 -0700 (PDT) In-Reply-To: <1307994828-sup-6461@shiny> References: <4DEC90CB.4050609@jp.fujitsu.com> <4DEDB7AF.2060308@cn.fujitsu.com> <4DEDBE4B.2020403@jp.fujitsu.com> <4DEDC293.30105@jp.fujitsu.com> <4DEDE03B.9050907@jp.fujitsu.com> <4DEDE328.5060405@cn.fujitsu.com> <1307461229-sup-9822@shiny> <4DEF04CF.8010502@jp.fujitsu.com> <4DF5B889.5080202@cn.fujitsu.com> <1307994828-sup-6461@shiny> Date: Tue, 14 Jun 2011 11:24:45 +0800 X-Google-Sender-Auth: BHF7QG-vCy1dUg8FG4QQpvQv09U Message-ID: Subject: Re: bug caused by removal of trans_mutex? (Was: Re: kernel BUG at fs/btrfs/extent-tree.c:6164!) From: "Yan, Zheng " To: Chris Mason Cc: Li Zefan , Tsutomu Itoh , liubo , Linux Btrfs , Josef Bacik Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Tue, 14 Jun 2011 03:24:49 +0000 (UTC) On Tue, Jun 14, 2011 at 3:55 AM, Chris Mason wrote: > Excerpts from Yan, Zheng's message of 2011-06-13 10:58:35 -0400: >> The usage of trans_mutex in relocation code is subtle. It controls >> interaction of relocation >> with transaction start, transaction commit and snapshot creation. >> Simple replacing >> trans_mutex with trans_lock is wrong. > > So, I've got a mutex around the reloc_root here and that was almost but > not quite enough.  It looks like the biggest problem is that we need to > wait in btrfs_record_root_in_trans for anyone inside merge_reloc_roots. > > I'm surviving much longer with a patch in place that synchronizes > btrfs_record_root_in_trans better. > > Zheng if you have other comments on the locking please let me know. > following untested patch may help. --- trans->block_rsv = rc->block_rsv; @@ -1353,6 +1355,8 @@ int btrfs_init_reloc_root(struct btrfs_trans_handle *trans, __add_reloc_root(reloc_root); root->reloc_root = reloc_root; +unlock: + mutex_unlock(&root->fs_info->reloc_mutex); return 0; } @@ -1367,8 +1371,9 @@ int btrfs_update_reloc_root(struct btrfs_trans_handle *trans, int del = 0; int ret; + mutex_lock(&root->fs_info->reloc_mutex); if (!root->reloc_root) - return 0; + goto unlock; reloc_root = root->reloc_root; root_item = &reloc_root->root_item; @@ -1390,6 +1395,8 @@ int btrfs_update_reloc_root(struct btrfs_trans_handle *trans, ret = btrfs_update_root(trans, root->fs_info->tree_root, &reloc_root->root_key, root_item); BUG_ON(ret); +unlock: + mutex_unlock(&root->fs_info->reloc_mutex); return 0; } @@ -2142,10 +2149,10 @@ int prepare_to_merge(struct reloc_control *rc, int err) u64 num_bytes = 0; int ret; - spin_lock(&root->fs_info->trans_lock); + mutex_lock(&root->fs_info->reloc_mutex); rc->merging_rsv_size += root->nodesize * (BTRFS_MAX_LEVEL - 1) * 2; rc->merging_rsv_size += rc->nodes_relocated * 2; - spin_unlock(&root->fs_info->trans_lock); + mutex_unlock(&root->fs_info->reloc_mutex); again: if (!err) { num_bytes = rc->merging_rsv_size; @@ -2214,9 +2221,9 @@ int merge_reloc_roots(struct reloc_control *rc) int ret; again: root = rc->extent_root; - spin_lock(&root->fs_info->trans_lock); + mutex_lock(&root->fs_info->reloc_mutex); list_splice_init(&rc->reloc_roots, &reloc_roots); - spin_unlock(&root->fs_info->trans_lock); + mutex_unlock(&root->fs_info->reloc_mutex); while (!list_empty(&reloc_roots)) { found = 1; @@ -3590,17 +3597,17 @@ next: static void set_reloc_control(struct reloc_control *rc) { struct btrfs_fs_info *fs_info = rc->extent_root->fs_info; - spin_lock(&fs_info->trans_lock); + mutex_lock(&fs_info->reloc_mutex); fs_info->reloc_ctl = rc; - spin_unlock(&fs_info->trans_lock); + mutex_unlock(&fs_info->reloc_mutex); } static void unset_reloc_control(struct reloc_control *rc) { struct btrfs_fs_info *fs_info = rc->extent_root->fs_info; - spin_lock(&fs_info->trans_lock); + mutex_lock(&fs_info->reloc_mutex); fs_info->reloc_ctl = NULL; - spin_unlock(&fs_info->trans_lock); + mutex_unlock(&fs_info->reloc_mutex); } static int check_extent_flags(u64 flags) @@ -4335,16 +4342,16 @@ void btrfs_reloc_pre_snapshot(struct btrfs_trans_handle *trans, struct btrfs_pending_snapshot *pending, u64 *bytes_to_reserve) { - struct btrfs_root *root; + struct btrfs_root *root = pending->root; struct reloc_control *rc; - root = pending->root; + mutex_lock(&root->fs_info->reloc_mutex); if (!root->reloc_root) - return; + goto unlock; rc = root->fs_info->reloc_ctl; if (!rc->merge_reloc_tree) - return; + goto unlock; root = root->reloc_root; BUG_ON(btrfs_root_refs(&root->root_item) == 0); @@ -4359,6 +4366,8 @@ void btrfs_reloc_pre_snapshot(struct btrfs_trans_handle *trans, * reserve extra space. */ *bytes_to_reserve += rc->nodes_relocated; +unlock: + mutex_unlock(&root->fs_info->reloc_mutex); } /* @@ -4374,8 +4383,9 @@ void btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans, struct reloc_control *rc; int ret; + mutex_lock(&root->fs_info->reloc_mutex); if (!root->reloc_root) - return; + goto unlock; rc = root->fs_info->reloc_ctl; rc->merging_rsv_size += rc->nodes_relocated; @@ -4398,4 +4408,6 @@ void btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans, ret = clone_backref_node(trans, rc, root, reloc_root); BUG_ON(ret); } +unlock: + mutex_unlock(&root->fs_info->reloc_mutex); } -- 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/ctree.h b/fs/btrfs/ctree.h index 378b5b4..0b20dda 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -951,6 +951,7 @@ struct btrfs_fs_info { struct mutex cleaner_mutex; struct mutex chunk_mutex; struct mutex volume_mutex; + struct mutex reloc_mutex; /* * this protects the ordered operations list only while we are * processing all of the entries on it. This way we make diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 9f68c68..28f8b11 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1714,6 +1714,7 @@ struct btrfs_root *open_ctree(struct super_block *sb, mutex_init(&fs_info->transaction_kthread_mutex); mutex_init(&fs_info->cleaner_mutex); mutex_init(&fs_info->volume_mutex); + mutex_init(&fs_info->reloc_mutex); init_rwsem(&fs_info->extent_commit_sem); init_rwsem(&fs_info->cleanup_work_sem); init_rwsem(&fs_info->subvol_sem); diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index b1ef27c..620e4af 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -1330,18 +1330,20 @@ int btrfs_init_reloc_root(struct btrfs_trans_handle *trans, struct btrfs_root *root) { struct btrfs_root *reloc_root; - struct reloc_control *rc = root->fs_info->reloc_ctl; + struct reloc_control *rc; int clear_rsv = 0; + mutex_lock(&root->fs_info->reloc_mutex); if (root->reloc_root) { reloc_root = root->reloc_root; reloc_root->last_trans = trans->transid; - return 0; + goto unlock; } + rc = root->fs_info->reloc_ctl; if (!rc || !rc->create_reloc_tree || root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) - return 0; + goto unlock; if (!trans->block_rsv) {