[PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit
@ 2014-02-20 10:08 Miao Xie
  2014-02-20 10:08 ` [PATCH 2/9] Btrfs: fix the skipped transaction commit during the file sync Miao Xie
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

->last_trans_log_full_commit may be changed by the other tasks without lock,
so we need prevent the compiler from the optimize access just like
	tmp = fs_info->last_trans_log_full_commit
	if (tmp == ...)
		...

	<do something>

	if (tmp == ...)
		...

In fact, we need get the new value of ->last_trans_log_full_commit during
the second access. Fix it by ACCESS_ONCE().

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/tree-log.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 7c449c6..5a4e10b 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2375,14 +2375,14 @@ static int wait_log_commit(struct btrfs_trans_handle *trans,
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
 
-		if (root->fs_info->last_trans_log_full_commit !=
+		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
 		    trans->transid && root->log_transid < transid + 2 &&
 		    atomic_read(&root->log_commit[index]))
 			schedule();
 
 		finish_wait(&root->log_commit_wait[index], &wait);
 		mutex_lock(&root->log_mutex);
-	} while (root->fs_info->last_trans_log_full_commit !=
+	} while (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
 		 trans->transid && root->log_transid < transid + 2 &&
 		 atomic_read(&root->log_commit[index]));
 	return 0;
@@ -2392,12 +2392,12 @@ static void wait_for_writer(struct btrfs_trans_handle *trans,
 			    struct btrfs_root *root)
 {
 	DEFINE_WAIT(wait);
-	while (root->fs_info->last_trans_log_full_commit !=
+	while (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
 	       trans->transid && atomic_read(&root->log_writers)) {
 		prepare_to_wait(&root->log_writer_wait,
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
-		if (root->fs_info->last_trans_log_full_commit !=
+		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
 		    trans->transid && atomic_read(&root->log_writers))
 			schedule();
 		mutex_lock(&root->log_mutex);
@@ -2456,7 +2456,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	}
 
 	/* bail out if we need to do a full commit */
-	if (root->fs_info->last_trans_log_full_commit == trans->transid) {
+	if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
+	    trans->transid) {
 		ret = -EAGAIN;
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&root->log_mutex);
@@ -2515,7 +2516,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 			mutex_unlock(&log_root_tree->log_mutex);
 			goto out;
 		}
-		root->fs_info->last_trans_log_full_commit = trans->transid;
+		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
+								trans->transid;
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
@@ -2547,7 +2549,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	 * now that we've moved on to the tree of log tree roots,
 	 * check the full commit flag again
 	 */
-	if (root->fs_info->last_trans_log_full_commit == trans->transid) {
+	if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
+	    trans->transid) {
 		blk_finish_plug(&plug);
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
 		btrfs_free_logged_extents(log, log_transid);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/9] Btrfs: fix the skipped transaction commit during the file sync
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-20 10:08 ` [PATCH 3/9] Btrfs: don't start the log transaction if the log tree init fails Miao Xie
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

We may abort the wait earlier if ->last_trans_log_full_commit was set to
the current transaction id, at this case, we need commit the current
transaction instead of the log sub-transaction. But the current code
didn't tell the caller to do it (return 0, not -EAGAIN). Fix it.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/tree-log.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 5a4e10b..8a03b39 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2364,6 +2364,7 @@ static int wait_log_commit(struct btrfs_trans_handle *trans,
 {
 	DEFINE_WAIT(wait);
 	int index = transid % 2;
+	int ret = 0;
 
 	/*
 	 * we only allow two pending log transactions at a time,
@@ -2371,21 +2372,26 @@ static int wait_log_commit(struct btrfs_trans_handle *trans,
 	 * current transaction, we're done
 	 */
 	do {
+		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
+		    trans->transid) {
+			ret = -EAGAIN;
+			break;
+		}
+
 		prepare_to_wait(&root->log_commit_wait[index],
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
 
-		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
-		    trans->transid && root->log_transid < transid + 2 &&
+		if (root->log_transid < transid + 2 &&
 		    atomic_read(&root->log_commit[index]))
 			schedule();
 
 		finish_wait(&root->log_commit_wait[index], &wait);
 		mutex_lock(&root->log_mutex);
-	} while (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
-		 trans->transid && root->log_transid < transid + 2 &&
+	} while (root->log_transid < transid + 2 &&
 		 atomic_read(&root->log_commit[index]));
-	return 0;
+
+	return ret;
 }
 
 static void wait_for_writer(struct btrfs_trans_handle *trans,
@@ -2433,15 +2439,16 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	log_transid = root->log_transid;
 	index1 = root->log_transid % 2;
 	if (atomic_read(&root->log_commit[index1])) {
-		wait_log_commit(trans, root, root->log_transid);
+		ret = wait_log_commit(trans, root, root->log_transid);
 		mutex_unlock(&root->log_mutex);
-		return 0;
+		return ret;
 	}
 	atomic_set(&root->log_commit[index1], 1);
 
 	/* wait for previous tree log sync to complete */
 	if (atomic_read(&root->log_commit[(index1 + 1) % 2]))
 		wait_log_commit(trans, root, root->log_transid - 1);
+
 	while (1) {
 		int batch = atomic_read(&root->log_batch);
 		/* when we're on an ssd, just kick the log commit out */
@@ -2529,11 +2536,10 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	if (atomic_read(&log_root_tree->log_commit[index2])) {
 		blk_finish_plug(&plug);
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
-		wait_log_commit(trans, log_root_tree,
-				log_root_tree->log_transid);
+		ret = wait_log_commit(trans, log_root_tree,
+				      log_root_tree->log_transid);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
-		ret = 0;
 		goto out;
 	}
 	atomic_set(&log_root_tree->log_commit[index2], 1);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/9] Btrfs: don't start the log transaction if the log tree init fails
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
  2014-02-20 10:08 ` [PATCH 2/9] Btrfs: fix the skipped transaction commit during the file sync Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

The old code would start the log transaction even the log tree init
failed, it was unnecessary. Fix it.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/tree-log.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8a03b39..ca960ad 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -139,7 +139,6 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root)
 {
 	int ret;
-	int err = 0;
 
 	mutex_lock(&root->log_mutex);
 	if (root->log_root) {
@@ -155,24 +154,27 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 		mutex_unlock(&root->log_mutex);
 		return 0;
 	}
-	root->log_multiple_pids = false;
-	root->log_start_pid = current->pid;
+
+	ret = 0;
 	mutex_lock(&root->fs_info->tree_log_mutex);
-	if (!root->fs_info->log_root_tree) {
+	if (!root->fs_info->log_root_tree)
 		ret = btrfs_init_log_root_tree(trans, root->fs_info);
-		if (ret)
-			err = ret;
-	}
-	if (err == 0 && !root->log_root) {
+	mutex_unlock(&root->fs_info->tree_log_mutex);
+	if (ret)
+		goto out;
+
+	if (!root->log_root) {
 		ret = btrfs_add_log_tree(trans, root);
 		if (ret)
-			err = ret;
+			goto out;
 	}
-	mutex_unlock(&root->fs_info->tree_log_mutex);
+	root->log_multiple_pids = false;
+	root->log_start_pid = current->pid;
 	atomic_inc(&root->log_batch);
 	atomic_inc(&root->log_writers);
+out:
 	mutex_unlock(&root->log_mutex);
-	return err;
+	return ret;
 }
 
 /*
@@ -4116,7 +4118,7 @@ static int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
 
 	ret = start_log_trans(trans, root);
 	if (ret)
-		goto end_trans;
+		goto end_no_trans;
 
 	ret = btrfs_log_inode(trans, root, inode, inode_only);
 	if (ret)
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
  2014-02-20 10:08 ` [PATCH 2/9] Btrfs: fix the skipped transaction commit during the file sync Miao Xie
  2014-02-20 10:08 ` [PATCH 3/9] Btrfs: don't start the log transaction if the log tree init fails Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-22  0:23   ` David Sterba
                     ` (2 more replies)
  2014-02-20 10:08 ` [PATCH 5/9] Btrfs: remove unnecessary memory barrier in btrfs_sync_log() Miao Xie
                   ` (4 subsequent siblings)
  7 siblings, 3 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.c       | 25 ++++++++++++++-----------
 fs/btrfs/ctree.h       | 39 +++++++++++++++++++++------------------
 fs/btrfs/disk-io.c     | 33 +++++++++++++++------------------
 fs/btrfs/extent-tree.c |  6 +++---
 fs/btrfs/file.c        |  4 +++-
 fs/btrfs/inode.c       | 29 ++++++++++++++++++-----------
 fs/btrfs/ioctl.c       |  4 ++--
 fs/btrfs/relocation.c  | 17 +++++++++--------
 fs/btrfs/root-tree.c   |  2 +-
 fs/btrfs/transaction.c | 33 +++++++++++++++++----------------
 fs/btrfs/tree-defrag.c |  2 +-
 fs/btrfs/tree-log.c    |  9 +++++----
 12 files changed, 109 insertions(+), 94 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cbd3a7d..9067d79 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -224,7 +224,8 @@ static struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 static void add_root_to_dirty_list(struct btrfs_root *root)
 {
 	spin_lock(&root->fs_info->trans_lock);
-	if (root->track_dirty && list_empty(&root->dirty_list)) {
+	if (test_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state) &&
+	    list_empty(&root->dirty_list)) {
 		list_add(&root->dirty_list,
 			 &root->fs_info->dirty_cowonly_roots);
 	}
@@ -246,9 +247,10 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
 	int level;
 	struct btrfs_disk_key disk_key;
 
-	WARN_ON(root->ref_cows && trans->transid !=
-		root->fs_info->running_transaction->transid);
-	WARN_ON(root->ref_cows && trans->transid != root->last_trans);
+	WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
+		trans->transid != root->fs_info->running_transaction->transid);
+	WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
+		trans->transid != root->last_trans);
 
 	level = btrfs_header_level(buf);
 	if (level == 0)
@@ -997,14 +999,14 @@ int btrfs_block_can_be_shared(struct btrfs_root *root,
 	 * snapshot and the block was not allocated by tree relocation,
 	 * we know the block is not shared.
 	 */
-	if (root->ref_cows &&
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
 	    buf != root->node && buf != root->commit_root &&
 	    (btrfs_header_generation(buf) <=
 	     btrfs_root_last_snapshot(&root->root_item) ||
 	     btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)))
 		return 1;
 #ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-	if (root->ref_cows &&
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
 	    btrfs_header_backref_rev(buf) < BTRFS_MIXED_BACKREF_REV)
 		return 1;
 #endif
@@ -1146,9 +1148,10 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
 
 	btrfs_assert_tree_locked(buf);
 
-	WARN_ON(root->ref_cows && trans->transid !=
-		root->fs_info->running_transaction->transid);
-	WARN_ON(root->ref_cows && trans->transid != root->last_trans);
+	WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
+		trans->transid != root->fs_info->running_transaction->transid);
+	WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
+		trans->transid != root->last_trans);
 
 	level = btrfs_header_level(buf);
 
@@ -1193,7 +1196,7 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
 		return ret;
 	}
 
-	if (root->ref_cows) {
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state)) {
 		ret = btrfs_reloc_cow_block(trans, root, buf, cow);
 		if (ret)
 			return ret;
@@ -1556,7 +1559,7 @@ static inline int should_cow_block(struct btrfs_trans_handle *trans,
 	    !btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN) &&
 	    !(root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID &&
 	      btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)) &&
-	    !root->force_cow)
+	    !test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		return 0;
 	return 1;
 }
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 277f627..c45a10a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1683,6 +1683,26 @@ struct btrfs_fs_info {
 };
 
 /*
+ * The state of btrfs root
+ */
+/*
+ * btrfs_record_root_in_trans is a multi-step process,
+ * and it can race with the balancing code.   But the
+ * race is very small, and only the first time the root
+ * is added to each transaction.  So IN_TRANS_SETUP
+ * is used to tell us when more checks are required
+ */
+#define BTRFS_ROOT_IN_TRANS_SETUP	0
+#define BTRFS_ROOT_REF_COWS		1
+#define BTRFS_ROOT_TRACK_DIRTY		2
+#define BTRFS_ROOT_IN_RADIX		3
+#define BTRFS_ROOT_DUMMY_ROOT		4
+#define BTRFS_ROOT_ORPHAN_ITEM_INSERTED	5
+#define BTRFS_ROOT_DEFRAG_RUNNING	6
+#define BTRFS_ROOT_FORCE_COW		7
+#define BTRFS_ROOT_MULTI_LOG_TASKS	8
+
+/*
  * in ram representation of the tree.  extent_root is used for all allocations
  * and for the extent tree extent_root root.
  */
@@ -1693,6 +1713,7 @@ struct btrfs_root {
 	struct btrfs_root *log_root;
 	struct btrfs_root *reloc_root;
 
+	unsigned long state;
 	struct btrfs_root_item root_item;
 	struct btrfs_key root_key;
 	struct btrfs_fs_info *fs_info;
@@ -1724,7 +1745,6 @@ struct btrfs_root {
 	unsigned long log_transid;
 	unsigned long last_log_commit;
 	pid_t log_start_pid;
-	bool log_multiple_pids;
 
 	u64 objectid;
 	u64 last_trans;
@@ -1744,23 +1764,9 @@ struct btrfs_root {
 
 	u64 highest_objectid;
 
-	/* btrfs_record_root_in_trans is a multi-step process,
-	 * and it can race with the balancing code.   But the
-	 * race is very small, and only the first time the root
-	 * is added to each transaction.  So in_trans_setup
-	 * is used to tell us when more checks are required
-	 */
-	unsigned long in_trans_setup;
-	int ref_cows;
-	int track_dirty;
-	int in_radix;
-#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
-	int dummy_root;
-#endif
 	u64 defrag_trans_start;
 	struct btrfs_key defrag_progress;
 	struct btrfs_key defrag_max;
-	int defrag_running;
 	char *name;
 
 	/* the dirty list is only used by non-reference counted roots */
@@ -1774,7 +1780,6 @@ struct btrfs_root {
 	spinlock_t orphan_lock;
 	atomic_t orphan_inodes;
 	struct btrfs_block_rsv *orphan_block_rsv;
-	int orphan_item_inserted;
 	int orphan_cleanup_state;
 
 	spinlock_t inode_lock;
@@ -1792,8 +1797,6 @@ struct btrfs_root {
 	 */
 	dev_t anon_dev;
 
-	int force_cow;
-
 	spinlock_t root_item_lock;
 	atomic_t refs;
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index de6a48f..cc0af87 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1163,10 +1163,7 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
 	root->nodesize = nodesize;
 	root->leafsize = leafsize;
 	root->stripesize = stripesize;
-	root->ref_cows = 0;
-	root->track_dirty = 0;
-	root->in_radix = 0;
-	root->orphan_item_inserted = 0;
+	root->state = 0;
 	root->orphan_cleanup_state = 0;
 
 	root->objectid = objectid;
@@ -1221,7 +1218,6 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
 	else
 		root->defrag_trans_start = 0;
 	init_completion(&root->kobj_unregister);
-	root->defrag_running = 0;
 	root->root_key.objectid = objectid;
 	root->anon_dev = 0;
 
@@ -1246,7 +1242,7 @@ struct btrfs_root *btrfs_alloc_dummy_root(void)
 	if (!root)
 		return ERR_PTR(-ENOMEM);
 	__setup_root(4096, 4096, 4096, 4096, root, NULL, 1);
-	root->dummy_root = 1;
+	set_bit(BTRFS_ROOT_DUMMY_ROOT, &root->state);
 
 	return root;
 }
@@ -1297,8 +1293,7 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(leaf);
 
 	root->commit_root = btrfs_root_node(root);
-	root->track_dirty = 1;
-
+	set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
 
 	root->root_item.flags = 0;
 	root->root_item.byte_limit = 0;
@@ -1352,13 +1347,15 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans,
 	root->root_key.objectid = BTRFS_TREE_LOG_OBJECTID;
 	root->root_key.type = BTRFS_ROOT_ITEM_KEY;
 	root->root_key.offset = BTRFS_TREE_LOG_OBJECTID;
+
 	/*
+	 * DON'T set REF_COWS for log trees
+	 *
 	 * log trees do not get reference counted because they go away
 	 * before a real commit is actually done.  They do store pointers
 	 * to file data extents, and those reference counts still get
 	 * updated (along with back refs to the log tree).
 	 */
-	root->ref_cows = 0;
 
 	leaf = btrfs_alloc_free_block(trans, root, root->leafsize, 0,
 				      BTRFS_TREE_LOG_OBJECTID, NULL,
@@ -1491,7 +1488,7 @@ struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root,
 		return root;
 
 	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
-		root->ref_cows = 1;
+		set_bit(BTRFS_ROOT_REF_COWS, &root->state);
 		btrfs_check_and_init_root_item(&root->root_item);
 	}
 
@@ -1551,7 +1548,7 @@ int btrfs_insert_fs_root(struct btrfs_fs_info *fs_info,
 				(unsigned long)root->root_key.objectid,
 				root);
 	if (ret == 0)
-		root->in_radix = 1;
+		set_bit(BTRFS_ROOT_IN_RADIX, &root->state);
 	spin_unlock(&fs_info->fs_roots_radix_lock);
 	radix_tree_preload_end();
 
@@ -1607,7 +1604,7 @@ again:
 	if (ret < 0)
 		goto fail;
 	if (ret == 0)
-		root->orphan_item_inserted = 1;
+		set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state);
 
 	ret = btrfs_insert_fs_root(fs_info, root);
 	if (ret) {
@@ -2047,7 +2044,7 @@ static void del_fs_roots(struct btrfs_fs_info *fs_info)
 				     struct btrfs_root, root_list);
 		list_del(&gang[0]->root_list);
 
-		if (gang[0]->in_radix) {
+		if (test_bit(BTRFS_ROOT_IN_RADIX, &gang[0]->state)) {
 			btrfs_drop_and_free_fs_root(fs_info, gang[0]);
 		} else {
 			free_extent_buffer(gang[0]->node);
@@ -2673,7 +2670,7 @@ retry_root_backup:
 		ret = PTR_ERR(extent_root);
 		goto recovery_tree_root;
 	}
-	extent_root->track_dirty = 1;
+	set_bit(BTRFS_ROOT_TRACK_DIRTY, &extent_root->state);
 	fs_info->extent_root = extent_root;
 
 	location.objectid = BTRFS_DEV_TREE_OBJECTID;
@@ -2682,7 +2679,7 @@ retry_root_backup:
 		ret = PTR_ERR(dev_root);
 		goto recovery_tree_root;
 	}
-	dev_root->track_dirty = 1;
+	set_bit(BTRFS_ROOT_TRACK_DIRTY, &dev_root->state);
 	fs_info->dev_root = dev_root;
 	btrfs_init_devices_late(fs_info);
 
@@ -2692,13 +2689,13 @@ retry_root_backup:
 		ret = PTR_ERR(csum_root);
 		goto recovery_tree_root;
 	}
-	csum_root->track_dirty = 1;
+	set_bit(BTRFS_ROOT_TRACK_DIRTY, &csum_root->state);
 	fs_info->csum_root = csum_root;
 
 	location.objectid = BTRFS_QUOTA_TREE_OBJECTID;
 	quota_root = btrfs_read_tree_root(tree_root, &location);
 	if (!IS_ERR(quota_root)) {
-		quota_root->track_dirty = 1;
+		set_bit(BTRFS_ROOT_TRACK_DIRTY, &quota_root->state);
 		fs_info->quota_enabled = 1;
 		fs_info->pending_quota_state = 1;
 		fs_info->quota_root = quota_root;
@@ -2713,7 +2710,7 @@ retry_root_backup:
 		create_uuid_tree = true;
 		check_uuid_tree = false;
 	} else {
-		uuid_root->track_dirty = 1;
+		set_bit(BTRFS_ROOT_TRACK_DIRTY, &uuid_root->state);
 		fs_info->uuid_root = uuid_root;
 		create_uuid_tree = false;
 		check_uuid_tree =
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 32312e0..a8155c9 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2981,7 +2981,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
 	nritems = btrfs_header_nritems(buf);
 	level = btrfs_header_level(buf);
 
-	if (!root->ref_cows && level == 0)
+	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state) && level == 0)
 		return 0;
 
 	if (inc)
@@ -4372,7 +4372,7 @@ static struct btrfs_block_rsv *get_block_rsv(
 {
 	struct btrfs_block_rsv *block_rsv = NULL;
 
-	if (root->ref_cows)
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		block_rsv = trans->block_rsv;
 
 	if (root == root->fs_info->csum_root && trans->adding_csums)
@@ -7735,7 +7735,7 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
 		}
 	}
 
-	if (root->in_radix) {
+	if (test_bit(BTRFS_ROOT_IN_RADIX, &root->state)) {
 		btrfs_drop_and_free_fs_root(tree_root->fs_info, root);
 	} else {
 		free_extent_buffer(root->node);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 0165b86..76ff550 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -713,7 +713,7 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
 	int recow;
 	int ret;
 	int modify_tree = -1;
-	int update_refs = (root->ref_cows || root == root->fs_info->tree_root);
+	int update_refs;
 	int found = 0;
 	int leafs_visited = 0;
 
@@ -723,6 +723,8 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
 	if (start >= BTRFS_I(inode)->disk_i_size)
 		modify_tree = 0;
 
+	update_refs = (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
+		       root == root->fs_info->tree_root);
 	while (1) {
 		recow = 0;
 		ret = btrfs_lookup_file_extent(trans, root, path, ino,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1af34d0..558c2e3 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2917,14 +2917,15 @@ void btrfs_orphan_commit_root(struct btrfs_trans_handle *trans,
 	root->orphan_block_rsv = NULL;
 	spin_unlock(&root->orphan_lock);
 
-	if (root->orphan_item_inserted &&
+	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state) &&
 	    btrfs_root_refs(&root->root_item) > 0) {
 		ret = btrfs_del_orphan_item(trans, root->fs_info->tree_root,
 					    root->root_key.objectid);
 		if (ret)
 			btrfs_abort_transaction(trans, root, ret);
 		else
-			root->orphan_item_inserted = 0;
+			clear_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED,
+				  &root->state);
 	}
 
 	if (block_rsv) {
@@ -3241,7 +3242,8 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
 		btrfs_block_rsv_release(root, root->orphan_block_rsv,
 					(u64)-1);
 
-	if (root->orphan_block_rsv || root->orphan_item_inserted) {
+	if (root->orphan_block_rsv ||
+	    test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state)) {
 		trans = btrfs_join_transaction(root);
 		if (!IS_ERR(trans))
 			btrfs_end_transaction(trans, root);
@@ -3968,7 +3970,8 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 	 * not block aligned since we will be keeping the last block of the
 	 * extent just the way it is.
 	 */
-	if (root->ref_cows || root == root->fs_info->tree_root)
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
+	    root == root->fs_info->tree_root)
 		btrfs_drop_extent_cache(inode, ALIGN(new_size,
 					root->sectorsize), (u64)-1, 0);
 
@@ -4061,7 +4064,9 @@ search_again:
 							 extent_num_bytes);
 				num_dec = (orig_num_bytes -
 					   extent_num_bytes);
-				if (root->ref_cows && extent_start != 0)
+				if (test_bit(BTRFS_ROOT_REF_COWS,
+					     &root->state) &&
+				    extent_start != 0)
 					inode_sub_bytes(inode, num_dec);
 				btrfs_mark_buffer_dirty(leaf);
 			} else {
@@ -4075,7 +4080,8 @@ search_again:
 				num_dec = btrfs_file_extent_num_bytes(leaf, fi);
 				if (extent_start != 0) {
 					found_extent = 1;
-					if (root->ref_cows)
+					if (test_bit(BTRFS_ROOT_REF_COWS,
+						     &root->state))
 						inode_sub_bytes(inode, num_dec);
 				}
 			}
@@ -4090,10 +4096,9 @@ search_again:
 			    btrfs_file_extent_other_encoding(leaf, fi) == 0) {
 				u32 size = new_size - found_key.offset;
 
-				if (root->ref_cows) {
+				if (test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 					inode_sub_bytes(inode, item_end + 1 -
 							new_size);
-				}
 
 				/*
 				 * update the ram bytes to properly reflect
@@ -4103,7 +4108,8 @@ search_again:
 				size =
 				    btrfs_file_extent_calc_inline_size(size);
 				btrfs_truncate_item(root, path, size, 1);
-			} else if (root->ref_cows) {
+			} else if (test_bit(BTRFS_ROOT_REF_COWS,
+					    &root->state)) {
 				inode_sub_bytes(inode, item_end + 1 -
 						found_key.offset);
 			}
@@ -4125,8 +4131,9 @@ delete:
 		} else {
 			break;
 		}
-		if (found_extent && (root->ref_cows ||
-				     root == root->fs_info->tree_root)) {
+		if (found_extent &&
+		    (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
+		     root == root->fs_info->tree_root)) {
 			btrfs_set_path_blocking(path);
 			ret = btrfs_free_extent(trans, root, extent_start,
 						extent_num_bytes, 0,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b7dc0f7..b061124 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -621,7 +621,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	struct btrfs_trans_handle *trans;
 	int ret;
 
-	if (!root->ref_cows)
+	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		return -EINVAL;
 
 	ret = btrfs_start_delalloc_inodes(root, 0);
@@ -2300,7 +2300,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	dest->root_item.drop_level = 0;
 	btrfs_set_root_refs(&dest->root_item, 0);
 
-	if (!xchg(&dest->orphan_item_inserted, 1)) {
+	if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &dest->state)) {
 		ret = btrfs_insert_orphan_item(trans,
 					root->fs_info->tree_root,
 					dest->root_key.objectid);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 07b3b36..6425c5b 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -528,7 +528,7 @@ static int should_ignore_root(struct btrfs_root *root)
 {
 	struct btrfs_root *reloc_root;
 
-	if (!root->ref_cows)
+	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		return 0;
 
 	reloc_root = root->reloc_root;
@@ -610,7 +610,7 @@ struct btrfs_root *find_tree_root(struct reloc_control *rc,
 	root = read_fs_root(rc->extent_root->fs_info, root_objectid);
 	BUG_ON(IS_ERR(root));
 
-	if (root->ref_cows &&
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
 	    generation != btrfs_root_generation(&root->root_item))
 		return NULL;
 
@@ -887,7 +887,7 @@ again:
 			goto out;
 		}
 
-		if (!root->ref_cows)
+		if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 			cur->cowonly = 1;
 
 		if (btrfs_root_level(&root->root_item) == cur->level) {
@@ -954,7 +954,8 @@ again:
 				upper->bytenr = eb->start;
 				upper->owner = btrfs_header_owner(eb);
 				upper->level = lower->level + 1;
-				if (!root->ref_cows)
+				if (!test_bit(BTRFS_ROOT_REF_COWS,
+					      &root->state))
 					upper->cowonly = 1;
 
 				/*
@@ -2462,7 +2463,7 @@ struct btrfs_root *select_reloc_root(struct btrfs_trans_handle *trans,
 		next = walk_up_backref(next, edges, &index);
 		root = next->root;
 		BUG_ON(!root);
-		BUG_ON(!root->ref_cows);
+		BUG_ON(!test_bit(BTRFS_ROOT_REF_COWS, &root->state));
 
 		if (root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
 			record_reloc_root_in_trans(trans, root);
@@ -2527,7 +2528,7 @@ struct btrfs_root *select_one_root(struct btrfs_trans_handle *trans,
 		BUG_ON(!root);
 
 		/* no other choice for non-references counted tree */
-		if (!root->ref_cows)
+		if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 			return root;
 
 		if (root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID)
@@ -2914,14 +2915,14 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 
-	if (!root || root->ref_cows) {
+	if (!root || test_bit(BTRFS_ROOT_REF_COWS, &root->state)) {
 		ret = reserve_metadata_space(trans, rc, node);
 		if (ret)
 			goto out;
 	}
 
 	if (root) {
-		if (root->ref_cows) {
+		if (test_bit(BTRFS_ROOT_REF_COWS, &root->state)) {
 			BUG_ON(node->new_bytenr);
 			BUG_ON(!list_empty(&node->list));
 			btrfs_record_root_in_trans(trans, root);
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 1389b69..a1a3fbf 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -305,7 +305,7 @@ int btrfs_find_orphan_roots(struct btrfs_root *tree_root)
 			break;
 		}
 
-		root->orphan_item_inserted = 1;
+		set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state);
 
 		err = btrfs_insert_fs_root(root->fs_info, root);
 		if (err) {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index d888f26..9f32566 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -229,18 +229,19 @@ loop:
 static int record_root_in_trans(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root)
 {
-	if (root->ref_cows && root->last_trans < trans->transid) {
+	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
+	    root->last_trans < trans->transid) {
 		WARN_ON(root == root->fs_info->extent_root);
 		WARN_ON(root->commit_root != root->node);
 
 		/*
-		 * see below for in_trans_setup usage rules
+		 * see below for IN_TRANS_SETUP usage rules
 		 * we have the reloc mutex held now, so there
 		 * is only one writer in this function
 		 */
-		root->in_trans_setup = 1;
+		set_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state);
 
-		/* make sure readers find in_trans_setup before
+		/* make sure readers find IN_TRANS_SETUP before
 		 * they find our root->last_trans update
 		 */
 		smp_wmb();
@@ -267,7 +268,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
 		 * But, we have to set root->last_trans before we
 		 * init the relocation root, otherwise, we trip over warnings
 		 * in ctree.c.  The solution used here is to flag ourselves
-		 * with root->in_trans_setup.  When this is 1, we're still
+		 * with root IN_TRANS_SETUP.  When this is 1, we're still
 		 * fixing up the reloc trees and everyone must wait.
 		 *
 		 * When this is zero, they can trust root->last_trans and fly
@@ -276,8 +277,8 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
 		 * done before we pop in the zero below
 		 */
 		btrfs_init_reloc_root(trans, root);
-		smp_wmb();
-		root->in_trans_setup = 0;
+		smp_mb__before_clear_bit();
+		clear_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state);
 	}
 	return 0;
 }
@@ -286,16 +287,16 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
 int btrfs_record_root_in_trans(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root)
 {
-	if (!root->ref_cows)
+	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		return 0;
 
 	/*
-	 * see record_root_in_trans for comments about in_trans_setup usage
+	 * see record_root_in_trans for comments about IN_TRANS_SETUP usage
 	 * and barriers
 	 */
 	smp_rmb();
 	if (root->last_trans == trans->transid &&
-	    !root->in_trans_setup)
+	    !test_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state))
 		return 0;
 
 	mutex_lock(&root->fs_info->reloc_mutex);
@@ -353,7 +354,7 @@ static int may_wait_transaction(struct btrfs_root *root, int type)
 static inline bool need_reserve_reloc_root(struct btrfs_root *root)
 {
 	if (!root->fs_info->reloc_ctl ||
-	    !root->ref_cows ||
+	    !test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
 	    root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID ||
 	    root->reloc_root)
 		return false;
@@ -1044,8 +1045,8 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans,
 			btrfs_save_ino_cache(root, trans);
 
 			/* see comments in should_cow_block() */
-			root->force_cow = 0;
-			smp_wmb();
+			clear_bit(BTRFS_ROOT_FORCE_COW, &root->state);
+			smp_mb__after_clear_bit();
 
 			if (root->commit_root != root->node) {
 				mutex_lock(&root->fs_commit_mutex);
@@ -1079,7 +1080,7 @@ int btrfs_defrag_root(struct btrfs_root *root)
 	struct btrfs_trans_handle *trans;
 	int ret;
 
-	if (xchg(&root->defrag_running, 1))
+	if (test_and_set_bit(BTRFS_ROOT_DEFRAG_RUNNING, &root->state))
 		return 0;
 
 	while (1) {
@@ -1102,7 +1103,7 @@ int btrfs_defrag_root(struct btrfs_root *root)
 			break;
 		}
 	}
-	root->defrag_running = 0;
+	clear_bit(BTRFS_ROOT_DEFRAG_RUNNING, &root->state);
 	return ret;
 }
 
@@ -1269,7 +1270,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	}
 
 	/* see comments in should_cow_block() */
-	root->force_cow = 1;
+	set_bit(BTRFS_ROOT_FORCE_COW, &root->state);
 	smp_wmb();
 
 	btrfs_set_root_node(new_root_item, tmp);
diff --git a/fs/btrfs/tree-defrag.c b/fs/btrfs/tree-defrag.c
index 76928ca..a63719c 100644
--- a/fs/btrfs/tree-defrag.c
+++ b/fs/btrfs/tree-defrag.c
@@ -49,7 +49,7 @@ int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 
-	if (root->ref_cows == 0)
+	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		goto out;
 
 	if (btrfs_test_opt(root, SSD))
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index ca960ad..51d1bd9 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -144,9 +144,9 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 	if (root->log_root) {
 		if (!root->log_start_pid) {
 			root->log_start_pid = current->pid;
-			root->log_multiple_pids = false;
+			clear_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
 		} else if (root->log_start_pid != current->pid) {
-			root->log_multiple_pids = true;
+			set_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
 		}
 
 		atomic_inc(&root->log_batch);
@@ -168,7 +168,7 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 		if (ret)
 			goto out;
 	}
-	root->log_multiple_pids = false;
+	clear_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
 	root->log_start_pid = current->pid;
 	atomic_inc(&root->log_batch);
 	atomic_inc(&root->log_writers);
@@ -2454,7 +2454,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	while (1) {
 		int batch = atomic_read(&root->log_batch);
 		/* when we're on an ssd, just kick the log commit out */
-		if (!btrfs_test_opt(root, SSD) && root->log_multiple_pids) {
+		if (!btrfs_test_opt(root, SSD) &&
+		    test_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state)) {
 			mutex_unlock(&root->log_mutex);
 			schedule_timeout_uninterruptible(1);
 			mutex_lock(&root->log_mutex);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
@ 2014-02-22  0:23   ` David Sterba
  2014-02-26  9:10     ` Miao Xie
  2014-03-07 23:54   ` Josef Bacik
  2014-03-08  0:00   ` Josef Bacik
  2 siblings, 1 reply; 17+ messages in thread
From: David Sterba @ 2014-02-22  0:23 UTC (permalink / raw)
  To: Miao Xie; +Cc: linux-btrfs

On Thu, Feb 20, 2014 at 06:08:54PM +0800, Miao Xie wrote:
> @@ -1352,13 +1347,15 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans,
>  	root->root_key.objectid = BTRFS_TREE_LOG_OBJECTID;
>  	root->root_key.type = BTRFS_ROOT_ITEM_KEY;
>  	root->root_key.offset = BTRFS_TREE_LOG_OBJECTID;
> +
>  	/*
> +	 * DON'T set REF_COWS for log trees
> +	 *
>  	 * log trees do not get reference counted because they go away
>  	 * before a real commit is actually done.  They do store pointers
>  	 * to file data extents, and those reference counts still get
>  	 * updated (along with back refs to the log tree).
>  	 */
> -	root->ref_cows = 0;

This looks like a bugfix hidden in a cleanup patch. If it is standalone
and not related to changes in this patchset, it makes sense to send it
separately (and possibly CC stable).

Thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-22  0:23   ` David Sterba
@ 2014-02-26  9:10     ` Miao Xie
  2014-02-27 17:02       ` David Sterba
  0 siblings, 1 reply; 17+ messages in thread
From: Miao Xie @ 2014-02-26  9:10 UTC (permalink / raw)
  To: dsterba, linux-btrfs

On Sat, 22 Feb 2014 01:23:37 +0100, David Sterba wrote:
> On Thu, Feb 20, 2014 at 06:08:54PM +0800, Miao Xie wrote:
>> @@ -1352,13 +1347,15 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans,
>>  	root->root_key.objectid = BTRFS_TREE_LOG_OBJECTID;
>>  	root->root_key.type = BTRFS_ROOT_ITEM_KEY;
>>  	root->root_key.offset = BTRFS_TREE_LOG_OBJECTID;
>> +
>>  	/*
>> +	 * DON'T set REF_COWS for log trees
>> +	 *
>>  	 * log trees do not get reference counted because they go away
>>  	 * before a real commit is actually done.  They do store pointers
>>  	 * to file data extents, and those reference counts still get
>>  	 * updated (along with back refs to the log tree).
>>  	 */
>> -	root->ref_cows = 0;
> 
> This looks like a bugfix hidden in a cleanup patch. If it is standalone
> and not related to changes in this patchset, it makes sense to send it
> separately (and possibly CC stable).

It is a cleanup because we have set it to 0 before.

I add this comment just to remind the other developer that don't set this flag.
(The old one is not so striking, I think.)

Thanks
Miao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-26  9:10     ` Miao Xie
@ 2014-02-27 17:02       ` David Sterba
  0 siblings, 0 replies; 17+ messages in thread
From: David Sterba @ 2014-02-27 17:02 UTC (permalink / raw)
  To: Miao Xie; +Cc: linux-btrfs

On Wed, Feb 26, 2014 at 05:10:05PM +0800, Miao Xie wrote:
> On Sat, 22 Feb 2014 01:23:37 +0100, David Sterba wrote:
> > On Thu, Feb 20, 2014 at 06:08:54PM +0800, Miao Xie wrote:
> >> @@ -1352,13 +1347,15 @@ static struct btrfs_root *alloc_log_tree(struct btrfs_trans_handle *trans,
> >>  	root->root_key.objectid = BTRFS_TREE_LOG_OBJECTID;
> >>  	root->root_key.type = BTRFS_ROOT_ITEM_KEY;
> >>  	root->root_key.offset = BTRFS_TREE_LOG_OBJECTID;
> >> +
> >>  	/*
> >> +	 * DON'T set REF_COWS for log trees
> >> +	 *
> >>  	 * log trees do not get reference counted because they go away
> >>  	 * before a real commit is actually done.  They do store pointers
> >>  	 * to file data extents, and those reference counts still get
> >>  	 * updated (along with back refs to the log tree).
> >>  	 */
> >> -	root->ref_cows = 0;
> > 
> > This looks like a bugfix hidden in a cleanup patch. If it is standalone
> > and not related to changes in this patchset, it makes sense to send it
> > separately (and possibly CC stable).
> 
> It is a cleanup because we have set it to 0 before.
> 
> I add this comment just to remind the other developer that don't set this flag.
> (The old one is not so striking, I think.)

Ox, thanks for explanation.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
  2014-02-22  0:23   ` David Sterba
@ 2014-03-07 23:54   ` Josef Bacik
  2014-03-08  0:00   ` Josef Bacik
  2 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2014-03-07 23:54 UTC (permalink / raw)
  To: Miao Xie, linux-btrfs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/20/2014 05:08 AM, Miao Xie wrote:
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/ctree.c
> | 25 ++++++++++++++----------- fs/btrfs/ctree.h       | 39
> +++++++++++++++++++++------------------ fs/btrfs/disk-io.c     | 33
> +++++++++++++++------------------ fs/btrfs/extent-tree.c |  6
> +++--- fs/btrfs/file.c        |  4 +++- fs/btrfs/inode.c       | 29
> ++++++++++++++++++----------- fs/btrfs/ioctl.c       |  4 ++-- 
> fs/btrfs/relocation.c  | 17 +++++++++-------- fs/btrfs/root-tree.c
> |  2 +- fs/btrfs/transaction.c | 33
> +++++++++++++++++---------------- fs/btrfs/tree-defrag.c |  2 +- 
> fs/btrfs/tree-log.c    |  9 +++++---- 12 files changed, 109
> insertions(+), 94 deletions(-)
> 

This patch causes xfstests to lock up the box.  I could reproduce by
running

./check generic/070 generic/074 generic/075

in a loop.  It blows up at 075 and usually happens right away, but
sometimes doesn't blow up until the second go around.  I'm dropping it
for now.  Thanks,

Josef

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTGlwuAAoJEANb+wAKly3BrE4P+wY0BhYvT1PjMBu6nFSZFxSd
5an4GGW5aoRrQ3G0wZkEpWPJK34exLvuzzaNxHH9FSesz27l1bnibNTUy7BzI7Mv
tj92Dxmckpcvl+z3QI+CcPkI6NpVzBNETRxBpRVtkqxWKioEhk4oOwvctBqbqaI1
lEOxylu6/09k2BjjeehJFLjzhVzq9taTUt7ETAaBaJLsPFdD9/E8S1EO/uJyabSo
yGSWSHEnXUMjRCaXZ1bZJKDowTZ10wviU4rDPyFi99h4h1m6FZe5jNStHojNcNkc
Qt1xv3s2bkuz7yAC9/yJuz4HFiWtiyRHD609vBiIBO93hLCFi5Xc4jXtSRVVC+Xx
891gfD0c8/SVJpviTOFnxewozmIgudcC4y3i2AglRI+IXEkNRr66G6v/H0uZEN+n
lhiNTLQ+/Kd+mxUA7Y3To86Hnb55TqX7G8AL2zdFKBb1LJLBCazcEQZkDDraG2MW
x77H38VvKyQ+pBgv/lZFnKHadPkveFrGJUYCx49SOHAvWDoDVcIx2liayR8WYBMA
reRT6Vd+sH4xkgVXXNhrFcDFsnbJp1R9LkHuk4HW56QASaMcLqwZ/RmwL/3ldX0c
17IyUZkXZx79rbzBwL8EmU9Gb+oDkNeu8nXIzVnBERDBQbDQqSThaLeiPfuJNPjU
mGmvLKc/fXNN3RofWrSx
=N9yr
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
  2014-02-22  0:23   ` David Sterba
  2014-03-07 23:54   ` Josef Bacik
@ 2014-03-08  0:00   ` Josef Bacik
  2014-03-10  1:16     ` Miao Xie
  2 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2014-03-08  0:00 UTC (permalink / raw)
  To: Miao Xie, linux-btrfs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/20/2014 05:08 AM, Miao Xie wrote:
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/ctree.c
> | 25 ++++++++++++++----------- fs/btrfs/ctree.h       | 39
> +++++++++++++++++++++------------------ fs/btrfs/disk-io.c     | 33
> +++++++++++++++------------------ fs/btrfs/extent-tree.c |  6
> +++--- fs/btrfs/file.c        |  4 +++- fs/btrfs/inode.c       | 29
> ++++++++++++++++++----------- fs/btrfs/ioctl.c       |  4 ++-- 
> fs/btrfs/relocation.c  | 17 +++++++++-------- fs/btrfs/root-tree.c
> |  2 +- fs/btrfs/transaction.c | 33
> +++++++++++++++++---------------- fs/btrfs/tree-defrag.c |  2 +- 
> fs/btrfs/tree-log.c    |  9 +++++---- 12 files changed, 109
> insertions(+), 94 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index
> cbd3a7d..9067d79 100644 --- a/fs/btrfs/ctree.c +++
> b/fs/btrfs/ctree.c @@ -224,7 +224,8 @@ static struct extent_buffer
> *btrfs_read_lock_root_node(struct btrfs_root *root) static void
> add_root_to_dirty_list(struct btrfs_root *root) { 
> spin_lock(&root->fs_info->trans_lock); -	if (root->track_dirty &&
> list_empty(&root->dirty_list)) { +	if
> (test_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state) && +
> list_empty(&root->dirty_list)) { list_add(&root->dirty_list, 
> &root->fs_info->dirty_cowonly_roots); } @@ -246,9 +247,10 @@ int
> btrfs_copy_root(struct btrfs_trans_handle *trans, int level; struct
> btrfs_disk_key disk_key;
> 
> -	WARN_ON(root->ref_cows && trans->transid != -
> root->fs_info->running_transaction->transid); -
> WARN_ON(root->ref_cows && trans->transid != root->last_trans); +
> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
> trans->transid != root->fs_info->running_transaction->transid); +
> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
> trans->transid != root->last_trans);
> 
> level = btrfs_header_level(buf); if (level == 0) @@ -997,14 +999,14
> @@ int btrfs_block_can_be_shared(struct btrfs_root *root, *
> snapshot and the block was not allocated by tree relocation, * we
> know the block is not shared. */ -	if (root->ref_cows && +	if
> (test_bit(BTRFS_ROOT_REF_COWS, &root->state) && buf != root->node
> && buf != root->commit_root && (btrfs_header_generation(buf) <= 
> btrfs_root_last_snapshot(&root->root_item) || 
> btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC))) return 1; #ifdef
> BTRFS_COMPAT_EXTENT_TREE_V0 -	if (root->ref_cows && +	if
> (test_bit(BTRFS_ROOT_REF_COWS, &root->state) && 
> btrfs_header_backref_rev(buf) < BTRFS_MIXED_BACKREF_REV) return 1; 
> #endif @@ -1146,9 +1148,10 @@ static noinline int
> __btrfs_cow_block(struct btrfs_trans_handle *trans,
> 
> btrfs_assert_tree_locked(buf);
> 
> -	WARN_ON(root->ref_cows && trans->transid != -
> root->fs_info->running_transaction->transid); -
> WARN_ON(root->ref_cows && trans->transid != root->last_trans); +
> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
> trans->transid != root->fs_info->running_transaction->transid); +
> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
> trans->transid != root->last_trans);
> 
> level = btrfs_header_level(buf);
> 
> @@ -1193,7 +1196,7 @@ static noinline int __btrfs_cow_block(struct
> btrfs_trans_handle *trans, return ret; }
> 
> -	if (root->ref_cows) { +	if (test_bit(BTRFS_ROOT_REF_COWS,
> &root->state)) { ret = btrfs_reloc_cow_block(trans, root, buf,
> cow); if (ret) return ret; @@ -1556,7 +1559,7 @@ static inline int
> should_cow_block(struct btrfs_trans_handle *trans, 
> !btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN) && 
> !(root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID && 
> btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)) && -
> !root->force_cow) +	    !test_bit(BTRFS_ROOT_REF_COWS,
> &root->state))

This may or may not be the problem.  Thanks,

Josef

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTGl2IAAoJEANb+wAKly3Br0EQAKWfEahMQ1ZV9jkgSYF/9c6r
ytoH2/Y3u7HYeeqf+Z/j2lkMyvAaWQ9EQCkWSUyCqoU7RPXHEROSCQ11UCk8qYXh
fhV2r3plOWIZ/KDHFvqPTN5FTg97OLvDalyvOR6UP/Ws6Z4Ycm0ePm7kb+25iK9u
N+PkiqHoQqUVw8Z2EEJ2SN/82SyNnuPDG2RkDD9xW8el5fBplAPgUox8W8Z+ubIo
FPiSNv4euly2Zco+Vs3NDFb2tqQBPyAVzE2IC4Nyq1Hci/vyC9k8YfIcCJnsP0Dk
10mVDhSSStWLuqt2L7fUV8nOTyjKT1gBNgoz/eMBeOzWLDbRKD2hpS6wpkw/6/iQ
/ff9Spikw7a87epYo4dxft32aQsDIu6JfgFPggL+VkXyn114MK4U5z7KNOPQUSBq
neFOVELgN1L75TI9v9/p1qKGeZ47vV1lvd6GP717SDF0yv9wgvHR0Ma47KSaLq79
WlwzmqXDYJYdOedKGQky6GZ7EFji5DDlazx7h1pQTN5rEdiQJTEFxrxtMOKzAttF
3EL9wxVAi2ggD2EYWWsk+SJNJxgU59bxTR9ZiOH+tj29+gFcGUpRsXMsj4KampK1
j0a0BSLj+1yUlLcT8qem05aJk720zxy4UBiZGcRP7qySlrOTwU5Xtk0BXXyfUXLg
guOgjR0pj494irbM53RT
=H5nB
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root
  2014-03-08  0:00   ` Josef Bacik
@ 2014-03-10  1:16     ` Miao Xie
  0 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-03-10  1:16 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs

On Fri, 7 Mar 2014 19:00:08 -0500, Josef Bacik wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/20/2014 05:08 AM, Miao Xie wrote:
>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/ctree.c
>> | 25 ++++++++++++++----------- fs/btrfs/ctree.h       | 39
>> +++++++++++++++++++++------------------ fs/btrfs/disk-io.c     | 33
>> +++++++++++++++------------------ fs/btrfs/extent-tree.c |  6
>> +++--- fs/btrfs/file.c        |  4 +++- fs/btrfs/inode.c       | 29
>> ++++++++++++++++++----------- fs/btrfs/ioctl.c       |  4 ++-- 
>> fs/btrfs/relocation.c  | 17 +++++++++-------- fs/btrfs/root-tree.c
>> |  2 +- fs/btrfs/transaction.c | 33
>> +++++++++++++++++---------------- fs/btrfs/tree-defrag.c |  2 +- 
>> fs/btrfs/tree-log.c    |  9 +++++---- 12 files changed, 109
>> insertions(+), 94 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index
>> cbd3a7d..9067d79 100644 --- a/fs/btrfs/ctree.c +++
>> b/fs/btrfs/ctree.c @@ -224,7 +224,8 @@ static struct extent_buffer
>> *btrfs_read_lock_root_node(struct btrfs_root *root) static void
>> add_root_to_dirty_list(struct btrfs_root *root) { 
>> spin_lock(&root->fs_info->trans_lock); -	if (root->track_dirty &&
>> list_empty(&root->dirty_list)) { +	if
>> (test_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state) && +
>> list_empty(&root->dirty_list)) { list_add(&root->dirty_list, 
>> &root->fs_info->dirty_cowonly_roots); } @@ -246,9 +247,10 @@ int
>> btrfs_copy_root(struct btrfs_trans_handle *trans, int level; struct
>> btrfs_disk_key disk_key;
>>
>> -	WARN_ON(root->ref_cows && trans->transid != -
>> root->fs_info->running_transaction->transid); -
>> WARN_ON(root->ref_cows && trans->transid != root->last_trans); +
>> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
>> trans->transid != root->fs_info->running_transaction->transid); +
>> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
>> trans->transid != root->last_trans);
>>
>> level = btrfs_header_level(buf); if (level == 0) @@ -997,14 +999,14
>> @@ int btrfs_block_can_be_shared(struct btrfs_root *root, *
>> snapshot and the block was not allocated by tree relocation, * we
>> know the block is not shared. */ -	if (root->ref_cows && +	if
>> (test_bit(BTRFS_ROOT_REF_COWS, &root->state) && buf != root->node
>> && buf != root->commit_root && (btrfs_header_generation(buf) <= 
>> btrfs_root_last_snapshot(&root->root_item) || 
>> btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC))) return 1; #ifdef
>> BTRFS_COMPAT_EXTENT_TREE_V0 -	if (root->ref_cows && +	if
>> (test_bit(BTRFS_ROOT_REF_COWS, &root->state) && 
>> btrfs_header_backref_rev(buf) < BTRFS_MIXED_BACKREF_REV) return 1; 
>> #endif @@ -1146,9 +1148,10 @@ static noinline int
>> __btrfs_cow_block(struct btrfs_trans_handle *trans,
>>
>> btrfs_assert_tree_locked(buf);
>>
>> -	WARN_ON(root->ref_cows && trans->transid != -
>> root->fs_info->running_transaction->transid); -
>> WARN_ON(root->ref_cows && trans->transid != root->last_trans); +
>> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
>> trans->transid != root->fs_info->running_transaction->transid); +
>> WARN_ON(test_bit(BTRFS_ROOT_REF_COWS, &root->state) && +
>> trans->transid != root->last_trans);
>>
>> level = btrfs_header_level(buf);
>>
>> @@ -1193,7 +1196,7 @@ static noinline int __btrfs_cow_block(struct
>> btrfs_trans_handle *trans, return ret; }
>>
>> -	if (root->ref_cows) { +	if (test_bit(BTRFS_ROOT_REF_COWS,
>> &root->state)) { ret = btrfs_reloc_cow_block(trans, root, buf,
>> cow); if (ret) return ret; @@ -1556,7 +1559,7 @@ static inline int
>> should_cow_block(struct btrfs_trans_handle *trans, 
>> !btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN) && 
>> !(root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID && 
>> btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)) && -
>> !root->force_cow) +	    !test_bit(BTRFS_ROOT_REF_COWS,
>> &root->state))
> 
> This may or may not be the problem.  Thanks,

You are right. I will update it in the next version.

Thanks
Miao

> 
> Josef
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
> 
> iQIcBAEBAgAGBQJTGl2IAAoJEANb+wAKly3Br0EQAKWfEahMQ1ZV9jkgSYF/9c6r
> ytoH2/Y3u7HYeeqf+Z/j2lkMyvAaWQ9EQCkWSUyCqoU7RPXHEROSCQ11UCk8qYXh
> fhV2r3plOWIZ/KDHFvqPTN5FTg97OLvDalyvOR6UP/Ws6Z4Ycm0ePm7kb+25iK9u
> N+PkiqHoQqUVw8Z2EEJ2SN/82SyNnuPDG2RkDD9xW8el5fBplAPgUox8W8Z+ubIo
> FPiSNv4euly2Zco+Vs3NDFb2tqQBPyAVzE2IC4Nyq1Hci/vyC9k8YfIcCJnsP0Dk
> 10mVDhSSStWLuqt2L7fUV8nOTyjKT1gBNgoz/eMBeOzWLDbRKD2hpS6wpkw/6/iQ
> /ff9Spikw7a87epYo4dxft32aQsDIu6JfgFPggL+VkXyn114MK4U5z7KNOPQUSBq
> neFOVELgN1L75TI9v9/p1qKGeZ47vV1lvd6GP717SDF0yv9wgvHR0Ma47KSaLq79
> WlwzmqXDYJYdOedKGQky6GZ7EFji5DDlazx7h1pQTN5rEdiQJTEFxrxtMOKzAttF
> 3EL9wxVAi2ggD2EYWWsk+SJNJxgU59bxTR9ZiOH+tj29+gFcGUpRsXMsj4KampK1
> j0a0BSLj+1yUlLcT8qem05aJk720zxy4UBiZGcRP7qySlrOTwU5Xtk0BXXyfUXLg
> guOgjR0pj494irbM53RT
> =H5nB
> -----END PGP SIGNATURE-----
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 5/9] Btrfs: remove unnecessary memory barrier in btrfs_sync_log()
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
                   ` (2 preceding siblings ...)
  2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-20 10:08 ` [PATCH 6/9] Btrfs: use signed integer instead of unsigned long integer for log transid Miao Xie
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

Mutex unlock implies certain memory barriers to make sure all the memory
operation completes before the unlock, and the next mutex lock implies memory
barriers to make sure the all the memory happens after the lock. So it is
a full memory barrier(smp_mb), we needn't add memory barriers. Remove them.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/tree-log.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 51d1bd9..c9f2479 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2497,7 +2497,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	root->log_transid++;
 	log->log_transid = root->log_transid;
 	root->log_start_pid = 0;
-	smp_mb();
 	/*
 	 * IO has been started, blocks of the log tree have WRITTEN flag set
 	 * in their headers. new modifications of the log will be written to
@@ -2590,8 +2589,6 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 				btrfs_header_level(log_root_tree->node));
 
 	log_root_tree->log_transid++;
-	smp_mb();
-
 	mutex_unlock(&log_root_tree->log_mutex);
 
 	/*
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/9] Btrfs: use signed integer instead of unsigned long integer for log transid
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
                   ` (3 preceding siblings ...)
  2014-02-20 10:08 ` [PATCH 5/9] Btrfs: remove unnecessary memory barrier in btrfs_sync_log() Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-20 10:08 ` [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails Miao Xie
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

The log trans id is initialized to be 0 every time we create a log tree,
and the log tree need be re-created after a new transaction is started,
it means the log trans id is unlikely to be a huge number, so we can use
signed integer instead of unsigned long integer to save a bit space.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/btrfs_inode.h | 14 +++++++-------
 fs/btrfs/ctree.h       |  4 ++--
 fs/btrfs/tree-log.c    |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 8fed212..c9a2444 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -109,14 +109,17 @@ struct btrfs_inode {
 	u64 last_trans;
 
 	/*
-	 * log transid when this inode was last modified
+	 * transid that last logged this inode
 	 */
-	u64 last_sub_trans;
+	u64 logged_trans;
 
 	/*
-	 * transid that last logged this inode
+	 * log transid when this inode was last modified
 	 */
-	u64 logged_trans;
+	int last_sub_trans;
+
+	/* a local copy of root's last_log_commit */
+	int last_log_commit;
 
 	/* total number of bytes pending delalloc, used by stat to calc the
 	 * real block usage of the file
@@ -155,9 +158,6 @@ struct btrfs_inode {
 	/* flags field from the on disk inode */
 	u32 flags;
 
-	/* a local copy of root's last_log_commit */
-	unsigned long last_log_commit;
-
 	/*
 	 * Counters to keep track of the number of extent item's we may use due
 	 * to delalloc and such.  outstanding_extents is the number of extent
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index c45a10a..76b6bff 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1742,8 +1742,8 @@ struct btrfs_root {
 	atomic_t log_writers;
 	atomic_t log_commit[2];
 	atomic_t log_batch;
-	unsigned long log_transid;
-	unsigned long last_log_commit;
+	int log_transid;
+	int last_log_commit;
 	pid_t log_start_pid;
 
 	u64 objectid;
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index c9f2479..c27e2c9 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2362,7 +2362,7 @@ static int update_log_root(struct btrfs_trans_handle *trans,
 }
 
 static int wait_log_commit(struct btrfs_trans_handle *trans,
-			   struct btrfs_root *root, unsigned long transid)
+			   struct btrfs_root *root, int transid)
 {
 	DEFINE_WAIT(wait);
 	int index = transid % 2;
@@ -2434,7 +2434,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	int ret;
 	struct btrfs_root *log = root->log_root;
 	struct btrfs_root *log_root_tree = root->fs_info->log_root_tree;
-	unsigned long log_transid = 0;
+	int log_transid = 0;
 	struct blk_plug plug;
 
 	mutex_lock(&root->log_mutex);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
                   ` (4 preceding siblings ...)
  2014-02-20 10:08 ` [PATCH 6/9] Btrfs: use signed integer instead of unsigned long integer for log transid Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-22  0:27   ` David Sterba
  2014-02-20 10:08 ` [PATCH 8/9] Btrfs: fix skipped error handle when log sync failed Miao Xie
  2014-02-20 10:08 ` [PATCH 9/9] Btrfs: just wait or commit our own log sub-transaction Miao Xie
  7 siblings, 1 reply; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

If the log sync fails, there is something wrong in the log tree, we
should not continue to join the log transaction and log the metadata.
What we should do is to do a full commit.

This patch fixes this problem by setting ->last_trans_log_full_commit
to the current transaction id, it will tell the tasks not to join
the log transaction, and do a full commit.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/tree-log.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index c27e2c9..4326da9 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -142,6 +142,12 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 
 	mutex_lock(&root->log_mutex);
 	if (root->log_root) {
+		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
+		    trans->transid) {
+			ret = -EAGAIN;
+			goto out;
+		}
+
 		if (!root->log_start_pid) {
 			root->log_start_pid = current->pid;
 			clear_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state);
@@ -2488,6 +2494,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 		blk_finish_plug(&plug);
 		btrfs_abort_transaction(trans, root, ret);
 		btrfs_free_logged_extents(log, log_transid);
+		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
+								trans->transid;
 		mutex_unlock(&root->log_mutex);
 		goto out;
 	}
@@ -2520,13 +2528,13 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 
 	if (ret) {
 		blk_finish_plug(&plug);
+		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
+								trans->transid;
 		if (ret != -ENOSPC) {
 			btrfs_abort_transaction(trans, root, ret);
 			mutex_unlock(&log_root_tree->log_mutex);
 			goto out;
 		}
-		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
-								trans->transid;
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
@@ -2572,6 +2580,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 					 EXTENT_DIRTY | EXTENT_NEW);
 	blk_finish_plug(&plug);
 	if (ret) {
+		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
+								trans->transid;
 		btrfs_abort_transaction(trans, root, ret);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
@@ -2600,6 +2610,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	 */
 	ret = write_ctree_super(trans, root->fs_info->tree_root, 1);
 	if (ret) {
+		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
+								trans->transid;
 		btrfs_abort_transaction(trans, root, ret);
 		goto out_wake_log_root;
 	}
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails
  2014-02-20 10:08 ` [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails Miao Xie
@ 2014-02-22  0:27   ` David Sterba
  2014-02-26  8:51     ` Miao Xie
  0 siblings, 1 reply; 17+ messages in thread
From: David Sterba @ 2014-02-22  0:27 UTC (permalink / raw)
  To: Miao Xie; +Cc: linux-btrfs

On Thu, Feb 20, 2014 at 06:08:57PM +0800, Miao Xie wrote:
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -142,6 +142,12 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
>  
>  	mutex_lock(&root->log_mutex);
>  	if (root->log_root) {
> +		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
> +		    trans->transid) {
> +			ret = -EAGAIN;
> +			goto out;
> +		}

So last_trans_log_full_commit needs special handling, but opencoding it
everywhere is no very clean. Documented set/get helpers would be much
better.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails
  2014-02-22  0:27   ` David Sterba
@ 2014-02-26  8:51     ` Miao Xie
  0 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-26  8:51 UTC (permalink / raw)
  To: dsterba, linux-btrfs

On Sat, 22 Feb 2014 01:27:51 +0100, David Sterba wrote:
> On Thu, Feb 20, 2014 at 06:08:57PM +0800, Miao Xie wrote:
>> --- a/fs/btrfs/tree-log.c
>> +++ b/fs/btrfs/tree-log.c
>> @@ -142,6 +142,12 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
>>  
>>  	mutex_lock(&root->log_mutex);
>>  	if (root->log_root) {
>> +		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
>> +		    trans->transid) {
>> +			ret = -EAGAIN;
>> +			goto out;
>> +		}
> 
> So last_trans_log_full_commit needs special handling, but opencoding it
> everywhere is no very clean. Documented set/get helpers would be much
> better.
> 

Right, I'll update it in the next version.

Thanks for your review.
Miao

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 8/9] Btrfs: fix skipped error handle when log sync failed
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
                   ` (5 preceding siblings ...)
  2014-02-20 10:08 ` [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  2014-02-20 10:08 ` [PATCH 9/9] Btrfs: just wait or commit our own log sub-transaction Miao Xie
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

It is possible that many tasks sync the log tree at the same time, but
only one task can do the sync work, the others will wait for it. But those
wait tasks didn't get the result of the log sync, and returned 0 when they
ended the wait. It caused those tasks skipped the error handle, and the
serious problem was they told the users the file sync succeeded but in
fact they failed.

This patch fixes this problem by introducing a log context structure,
we insert it into the a global list. When the sync fails, we will set
the error number of every log context in the list, then the waiting tasks
get the error number of the log context and handle the error if need.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h    |   1 +
 fs/btrfs/disk-io.c  |   2 +
 fs/btrfs/file.c     |   9 +++--
 fs/btrfs/tree-log.c | 114 ++++++++++++++++++++++++++++++++++++++++------------
 fs/btrfs/tree-log.h |  16 +++++++-
 5 files changed, 111 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 76b6bff..1bfce32 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1739,6 +1739,7 @@ struct btrfs_root {
 	struct mutex log_mutex;
 	wait_queue_head_t log_writer_wait;
 	wait_queue_head_t log_commit_wait[2];
+	struct list_head log_ctxs[2];
 	atomic_t log_writers;
 	atomic_t log_commit[2];
 	atomic_t log_batch;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cc0af87..f85f83b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1197,6 +1197,8 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
 	init_waitqueue_head(&root->log_writer_wait);
 	init_waitqueue_head(&root->log_commit_wait[0]);
 	init_waitqueue_head(&root->log_commit_wait[1]);
+	INIT_LIST_HEAD(&root->log_ctxs[0]);
+	INIT_LIST_HEAD(&root->log_ctxs[1]);
 	atomic_set(&root->log_commit[0], 0);
 	atomic_set(&root->log_commit[1], 0);
 	atomic_set(&root->log_writers, 0);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 76ff550..0660e16 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1858,8 +1858,9 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	struct dentry *dentry = file->f_path.dentry;
 	struct inode *inode = dentry->d_inode;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
-	int ret = 0;
 	struct btrfs_trans_handle *trans;
+	struct btrfs_log_ctx ctx;
+	int ret = 0;
 	bool full_sync = 0;
 
 	trace_btrfs_sync_file(file, datasync);
@@ -1953,7 +1954,9 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	}
 	trans->sync = true;
 
-	ret = btrfs_log_dentry_safe(trans, root, dentry);
+	btrfs_init_log_ctx(&ctx);
+
+	ret = btrfs_log_dentry_safe(trans, root, dentry, &ctx);
 	if (ret < 0) {
 		/* Fallthrough and commit/free transaction. */
 		ret = 1;
@@ -1973,7 +1976,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 
 	if (ret != BTRFS_NO_LOG_SYNC) {
 		if (!ret) {
-			ret = btrfs_sync_log(trans, root);
+			ret = btrfs_sync_log(trans, root, &ctx);
 			if (!ret) {
 				ret = btrfs_end_transaction(trans, root);
 				goto out;
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 4326da9..3289eb5 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -136,8 +136,10 @@ static noinline int replay_dir_deletes(struct btrfs_trans_handle *trans,
  * syncing the tree wait for us to finish
  */
 static int start_log_trans(struct btrfs_trans_handle *trans,
-			   struct btrfs_root *root)
+			   struct btrfs_root *root,
+			   struct btrfs_log_ctx *ctx)
 {
+	int index;
 	int ret;
 
 	mutex_lock(&root->log_mutex);
@@ -157,6 +159,10 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 
 		atomic_inc(&root->log_batch);
 		atomic_inc(&root->log_writers);
+		if (ctx) {
+			index = root->log_transid % 2;
+			list_add_tail(&ctx->list, &root->log_ctxs[index]);
+		}
 		mutex_unlock(&root->log_mutex);
 		return 0;
 	}
@@ -178,6 +184,10 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 	root->log_start_pid = current->pid;
 	atomic_inc(&root->log_batch);
 	atomic_inc(&root->log_writers);
+	if (ctx) {
+		index = root->log_transid % 2;
+		list_add_tail(&ctx->list, &root->log_ctxs[index]);
+	}
 out:
 	mutex_unlock(&root->log_mutex);
 	return ret;
@@ -2367,12 +2377,11 @@ static int update_log_root(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static int wait_log_commit(struct btrfs_trans_handle *trans,
-			   struct btrfs_root *root, int transid)
+static void wait_log_commit(struct btrfs_trans_handle *trans,
+			    struct btrfs_root *root, int transid)
 {
 	DEFINE_WAIT(wait);
 	int index = transid % 2;
-	int ret = 0;
 
 	/*
 	 * we only allow two pending log transactions at a time,
@@ -2380,12 +2389,6 @@ static int wait_log_commit(struct btrfs_trans_handle *trans,
 	 * current transaction, we're done
 	 */
 	do {
-		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) ==
-		    trans->transid) {
-			ret = -EAGAIN;
-			break;
-		}
-
 		prepare_to_wait(&root->log_commit_wait[index],
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
@@ -2398,27 +2401,55 @@ static int wait_log_commit(struct btrfs_trans_handle *trans,
 		mutex_lock(&root->log_mutex);
 	} while (root->log_transid < transid + 2 &&
 		 atomic_read(&root->log_commit[index]));
-
-	return ret;
 }
 
 static void wait_for_writer(struct btrfs_trans_handle *trans,
 			    struct btrfs_root *root)
 {
 	DEFINE_WAIT(wait);
-	while (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
-	       trans->transid && atomic_read(&root->log_writers)) {
+
+	while (atomic_read(&root->log_writers)) {
 		prepare_to_wait(&root->log_writer_wait,
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
-		if (ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) !=
-		    trans->transid && atomic_read(&root->log_writers))
+		if (atomic_read(&root->log_writers))
 			schedule();
 		mutex_lock(&root->log_mutex);
 		finish_wait(&root->log_writer_wait, &wait);
 	}
 }
 
+static inline void btrfs_remove_log_ctx(struct btrfs_root *root,
+					struct btrfs_log_ctx *ctx)
+{
+	if (!ctx)
+		return;
+
+	mutex_lock(&root->log_mutex);
+	list_del_init(&ctx->list);
+	mutex_unlock(&root->log_mutex);
+}
+
+/* 
+ * Invoked in log mutex context, or be sure there is no other task which
+ * can access the list.
+ */
+static inline void btrfs_remove_all_log_ctxs(struct btrfs_root *root,
+					     int index, int error)
+{
+	struct btrfs_log_ctx *ctx;
+
+	if (!error) {
+		INIT_LIST_HEAD(&root->log_ctxs[index]);
+		return;
+	}
+
+	list_for_each_entry(ctx, &root->log_ctxs[index], list)
+		ctx->log_ret = error;
+
+	INIT_LIST_HEAD(&root->log_ctxs[index]);
+}
+
 /*
  * btrfs_sync_log does sends a given tree log down to the disk and
  * updates the super blocks to record it.  When this call is done,
@@ -2432,7 +2463,7 @@ static void wait_for_writer(struct btrfs_trans_handle *trans,
  * that has happened.
  */
 int btrfs_sync_log(struct btrfs_trans_handle *trans,
-		   struct btrfs_root *root)
+		   struct btrfs_root *root, struct btrfs_log_ctx *ctx)
 {
 	int index1;
 	int index2;
@@ -2441,15 +2472,16 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	struct btrfs_root *log = root->log_root;
 	struct btrfs_root *log_root_tree = root->fs_info->log_root_tree;
 	int log_transid = 0;
+	struct btrfs_log_ctx root_log_ctx;
 	struct blk_plug plug;
 
 	mutex_lock(&root->log_mutex);
 	log_transid = root->log_transid;
 	index1 = root->log_transid % 2;
 	if (atomic_read(&root->log_commit[index1])) {
-		ret = wait_log_commit(trans, root, root->log_transid);
+		wait_log_commit(trans, root, root->log_transid);
 		mutex_unlock(&root->log_mutex);
-		return ret;
+		return ctx->log_ret;
 	}
 	atomic_set(&root->log_commit[index1], 1);
 
@@ -2543,13 +2575,18 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	}
 
 	index2 = log_root_tree->log_transid % 2;
+
+	btrfs_init_log_ctx(&root_log_ctx);
+	list_add_tail(&root_log_ctx.list, &log_root_tree->log_ctxs[index2]);
+
 	if (atomic_read(&log_root_tree->log_commit[index2])) {
 		blk_finish_plug(&plug);
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
-		ret = wait_log_commit(trans, log_root_tree,
-				      log_root_tree->log_transid);
+		wait_log_commit(trans, log_root_tree,
+				log_root_tree->log_transid);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
+		ret = root_log_ctx.log_ret;
 		goto out;
 	}
 	atomic_set(&log_root_tree->log_commit[index2], 1);
@@ -2622,12 +2659,31 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	mutex_unlock(&root->log_mutex);
 
 out_wake_log_root:
+	/*
+	 * We needn't get log_mutex here because we are sure all
+	 * the other tasks are blocked.
+	 */
+	btrfs_remove_all_log_ctxs(log_root_tree, index2, ret);
+
+	/*
+	 * It is dangerous if log_commit is changed before we set
+	 * ->log_ret of log ctx. Because the readers may not get
+	 *  the return value.
+	 */
+	smp_wmb();
+
 	atomic_set(&log_root_tree->log_commit[index2], 0);
 	smp_mb();
 	if (waitqueue_active(&log_root_tree->log_commit_wait[index2]))
 		wake_up(&log_root_tree->log_commit_wait[index2]);
 out:
+	/* See above. */
+	btrfs_remove_all_log_ctxs(root, index1, ret);
+
+	/* See above. */
+	smp_wmb();
 	atomic_set(&root->log_commit[index1], 0);
+
 	smp_mb();
 	if (waitqueue_active(&root->log_commit_wait[index1]))
 		wake_up(&root->log_commit_wait[index1]);
@@ -4089,7 +4145,8 @@ out:
  */
 static int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
 			    	  struct btrfs_root *root, struct inode *inode,
-			    	  struct dentry *parent, int exists_only)
+			    	  struct dentry *parent, int exists_only,
+				  struct btrfs_log_ctx *ctx)
 {
 	int inode_only = exists_only ? LOG_INODE_EXISTS : LOG_INODE_ALL;
 	struct super_block *sb;
@@ -4126,7 +4183,7 @@ static int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
 		goto end_no_trans;
 	}
 
-	ret = start_log_trans(trans, root);
+	ret = start_log_trans(trans, root, ctx);
 	if (ret)
 		goto end_no_trans;
 
@@ -4176,6 +4233,9 @@ end_trans:
 		root->fs_info->last_trans_log_full_commit = trans->transid;
 		ret = 1;
 	}
+
+	if (ret)
+		btrfs_remove_log_ctx(root, ctx);
 	btrfs_end_log_trans(root);
 end_no_trans:
 	return ret;
@@ -4188,12 +4248,14 @@ end_no_trans:
  * data on disk.
  */
 int btrfs_log_dentry_safe(struct btrfs_trans_handle *trans,
-			  struct btrfs_root *root, struct dentry *dentry)
+			  struct btrfs_root *root, struct dentry *dentry,
+			  struct btrfs_log_ctx *ctx)
 {
 	struct dentry *parent = dget_parent(dentry);
 	int ret;
 
-	ret = btrfs_log_inode_parent(trans, root, dentry->d_inode, parent, 0);
+	ret = btrfs_log_inode_parent(trans, root, dentry->d_inode, parent,
+				     0, ctx);
 	dput(parent);
 
 	return ret;
@@ -4430,6 +4492,6 @@ int btrfs_log_new_name(struct btrfs_trans_handle *trans,
 		    root->fs_info->last_trans_committed))
 		return 0;
 
-	return btrfs_log_inode_parent(trans, root, inode, parent, 1);
+	return btrfs_log_inode_parent(trans, root, inode, parent, 1, NULL);
 }
 
diff --git a/fs/btrfs/tree-log.h b/fs/btrfs/tree-log.h
index 1d4ae0d..59c1edb 100644
--- a/fs/btrfs/tree-log.h
+++ b/fs/btrfs/tree-log.h
@@ -22,14 +22,26 @@
 /* return value for btrfs_log_dentry_safe that means we don't need to log it at all */
 #define BTRFS_NO_LOG_SYNC 256
 
+struct btrfs_log_ctx {
+	int log_ret;
+	struct list_head list;
+};
+
+static inline void btrfs_init_log_ctx(struct btrfs_log_ctx *ctx)
+{
+	ctx->log_ret = 0;
+	INIT_LIST_HEAD(&ctx->list);
+}
+
 int btrfs_sync_log(struct btrfs_trans_handle *trans,
-		   struct btrfs_root *root);
+		   struct btrfs_root *root, struct btrfs_log_ctx *ctx);
 int btrfs_free_log(struct btrfs_trans_handle *trans, struct btrfs_root *root);
 int btrfs_free_log_root_tree(struct btrfs_trans_handle *trans,
 			     struct btrfs_fs_info *fs_info);
 int btrfs_recover_log_trees(struct btrfs_root *tree_root);
 int btrfs_log_dentry_safe(struct btrfs_trans_handle *trans,
-			  struct btrfs_root *root, struct dentry *dentry);
+			  struct btrfs_root *root, struct dentry *dentry,
+			  struct btrfs_log_ctx *ctx);
 int btrfs_del_dir_entries_in_log(struct btrfs_trans_handle *trans,
 				 struct btrfs_root *root,
 				 const char *name, int name_len,
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 9/9] Btrfs: just wait or commit our own log sub-transaction
  2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
                   ` (6 preceding siblings ...)
  2014-02-20 10:08 ` [PATCH 8/9] Btrfs: fix skipped error handle when log sync failed Miao Xie
@ 2014-02-20 10:08 ` Miao Xie
  7 siblings, 0 replies; 17+ messages in thread
From: Miao Xie @ 2014-02-20 10:08 UTC (permalink / raw)
  To: linux-btrfs

We might commit the log sub-transaction which didn't contain the metadata we
logged. It was because we didn't record the log transid and just select
the current log sub-transaction to commit, but the right one might be
committed by the other task already. Actually, we needn't do anything
and it is safe that we go back directly in this case.

This patch improves the log sync by the above idea. We record the transid
of the log sub-transaction in which we log the metadata, and the transid
of the log sub-transaction we have committed. If the committed transid
is >= the transid we record when logging the metadata, we just go back.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h    |  3 +++
 fs/btrfs/disk-io.c  |  2 ++
 fs/btrfs/tree-log.c | 63 ++++++++++++++++++++++++++++++++++-------------------
 fs/btrfs/tree-log.h |  2 ++
 4 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1bfce32..dc72c1f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1744,6 +1744,9 @@ struct btrfs_root {
 	atomic_t log_commit[2];
 	atomic_t log_batch;
 	int log_transid;
+	/* No matter the commit succeeds or not*/
+	int log_transid_committed;
+	/* Just be updated when the commit succeeds. */
 	int last_log_commit;
 	pid_t log_start_pid;
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f85f83b..6699483 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1206,6 +1206,7 @@ static void __setup_root(u32 nodesize, u32 leafsize, u32 sectorsize,
 	atomic_set(&root->orphan_inodes, 0);
 	atomic_set(&root->refs, 1);
 	root->log_transid = 0;
+	root->log_transid_committed = -1;
 	root->last_log_commit = 0;
 	if (fs_info)
 		extent_io_tree_init(&root->dirty_log_pages,
@@ -1419,6 +1420,7 @@ int btrfs_add_log_tree(struct btrfs_trans_handle *trans,
 	WARN_ON(root->log_root);
 	root->log_root = log_root;
 	root->log_transid = 0;
+	root->log_transid_committed = -1;
 	root->last_log_commit = 0;
 	return 0;
 }
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 3289eb5..ffee158 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -162,6 +162,7 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 		if (ctx) {
 			index = root->log_transid % 2;
 			list_add_tail(&ctx->list, &root->log_ctxs[index]);
+			ctx->log_transid = root->log_transid;
 		}
 		mutex_unlock(&root->log_mutex);
 		return 0;
@@ -187,6 +188,7 @@ static int start_log_trans(struct btrfs_trans_handle *trans,
 	if (ctx) {
 		index = root->log_transid % 2;
 		list_add_tail(&ctx->list, &root->log_ctxs[index]);
+		ctx->log_transid = root->log_transid;
 	}
 out:
 	mutex_unlock(&root->log_mutex);
@@ -2393,13 +2395,13 @@ static void wait_log_commit(struct btrfs_trans_handle *trans,
 				&wait, TASK_UNINTERRUPTIBLE);
 		mutex_unlock(&root->log_mutex);
 
-		if (root->log_transid < transid + 2 &&
+		if (root->log_transid_committed < transid &&
 		    atomic_read(&root->log_commit[index]))
 			schedule();
 
 		finish_wait(&root->log_commit_wait[index], &wait);
 		mutex_lock(&root->log_mutex);
-	} while (root->log_transid < transid + 2 &&
+	} while (root->log_transid_committed < transid &&
 		 atomic_read(&root->log_commit[index]));
 }
 
@@ -2476,18 +2478,24 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	struct blk_plug plug;
 
 	mutex_lock(&root->log_mutex);
-	log_transid = root->log_transid;
-	index1 = root->log_transid % 2;
+	log_transid = ctx->log_transid;
+	if (root->log_transid_committed >= log_transid) {
+		mutex_unlock(&root->log_mutex);
+		return ctx->log_ret;
+	}
+
+	index1 = log_transid % 2;
 	if (atomic_read(&root->log_commit[index1])) {
-		wait_log_commit(trans, root, root->log_transid);
+		wait_log_commit(trans, root, log_transid);
 		mutex_unlock(&root->log_mutex);
 		return ctx->log_ret;
 	}
+	ASSERT(log_transid == root->log_transid);
 	atomic_set(&root->log_commit[index1], 1);
 
 	/* wait for previous tree log sync to complete */
 	if (atomic_read(&root->log_commit[(index1 + 1) % 2]))
-		wait_log_commit(trans, root, root->log_transid - 1);
+		wait_log_commit(trans, root, log_transid - 1);
 
 	while (1) {
 		int batch = atomic_read(&root->log_batch);
@@ -2544,9 +2552,16 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	 */
 	mutex_unlock(&root->log_mutex);
 
+	btrfs_init_log_ctx(&root_log_ctx);
+
 	mutex_lock(&log_root_tree->log_mutex);
 	atomic_inc(&log_root_tree->log_batch);
 	atomic_inc(&log_root_tree->log_writers);
+
+	index2 = log_root_tree->log_transid % 2;
+	list_add_tail(&root_log_ctx.list, &log_root_tree->log_ctxs[index2]);
+	root_log_ctx.log_transid = log_root_tree->log_transid;
+
 	mutex_unlock(&log_root_tree->log_mutex);
 
 	ret = update_log_root(trans, log);
@@ -2559,6 +2574,9 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 	}
 
 	if (ret) {
+		if (!list_empty(&root_log_ctx.list))
+			list_del_init(&root_log_ctx.list);
+
 		blk_finish_plug(&plug);
 		ACCESS_ONCE(root->fs_info->last_trans_log_full_commit) =
 								trans->transid;
@@ -2574,26 +2592,29 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 
-	index2 = log_root_tree->log_transid % 2;
-
-	btrfs_init_log_ctx(&root_log_ctx);
-	list_add_tail(&root_log_ctx.list, &log_root_tree->log_ctxs[index2]);
+	if (log_root_tree->log_transid_committed >= root_log_ctx.log_transid) {
+		mutex_unlock(&log_root_tree->log_mutex);
+		ret = root_log_ctx.log_ret;
+		goto out;
+	}
 
+	index2 = root_log_ctx.log_transid % 2;
 	if (atomic_read(&log_root_tree->log_commit[index2])) {
 		blk_finish_plug(&plug);
 		btrfs_wait_marked_extents(log, &log->dirty_log_pages, mark);
 		wait_log_commit(trans, log_root_tree,
-				log_root_tree->log_transid);
+				root_log_ctx.log_transid);
 		btrfs_free_logged_extents(log, log_transid);
 		mutex_unlock(&log_root_tree->log_mutex);
 		ret = root_log_ctx.log_ret;
 		goto out;
 	}
+	ASSERT(root_log_ctx.log_transid == log_root_tree->log_transid);
 	atomic_set(&log_root_tree->log_commit[index2], 1);
 
 	if (atomic_read(&log_root_tree->log_commit[(index2 + 1) % 2])) {
 		wait_log_commit(trans, log_root_tree,
-				log_root_tree->log_transid - 1);
+				root_log_ctx.log_transid - 1);
 	}
 
 	wait_for_writer(trans, log_root_tree);
@@ -2665,26 +2686,22 @@ out_wake_log_root:
 	 */
 	btrfs_remove_all_log_ctxs(log_root_tree, index2, ret);
 
-	/*
-	 * It is dangerous if log_commit is changed before we set
-	 * ->log_ret of log ctx. Because the readers may not get
-	 *  the return value.
-	 */
-	smp_wmb();
-
+	mutex_lock(&log_root_tree->log_mutex);
+	log_root_tree->log_transid_committed++;
 	atomic_set(&log_root_tree->log_commit[index2], 0);
-	smp_mb();
+	mutex_unlock(&log_root_tree->log_mutex);
+
 	if (waitqueue_active(&log_root_tree->log_commit_wait[index2]))
 		wake_up(&log_root_tree->log_commit_wait[index2]);
 out:
 	/* See above. */
 	btrfs_remove_all_log_ctxs(root, index1, ret);
 
-	/* See above. */
-	smp_wmb();
+	mutex_lock(&root->log_mutex);
+	root->log_transid_committed++;
 	atomic_set(&root->log_commit[index1], 0);
+	mutex_unlock(&root->log_mutex);
 
-	smp_mb();
 	if (waitqueue_active(&root->log_commit_wait[index1]))
 		wake_up(&root->log_commit_wait[index1]);
 	return ret;
diff --git a/fs/btrfs/tree-log.h b/fs/btrfs/tree-log.h
index 59c1edb..91b145f 100644
--- a/fs/btrfs/tree-log.h
+++ b/fs/btrfs/tree-log.h
@@ -24,12 +24,14 @@
 
 struct btrfs_log_ctx {
 	int log_ret;
+	int log_transid;
 	struct list_head list;
 };
 
 static inline void btrfs_init_log_ctx(struct btrfs_log_ctx *ctx)
 {
 	ctx->log_ret = 0;
+	ctx->log_transid = 0;
 	INIT_LIST_HEAD(&ctx->list);
 }
 
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-03-10  1:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-20 10:08 [PATCH 1/9] Btrfs: use ACCESS_ONCE to prevent the optimize accesses to ->last_trans_log_full_commit Miao Xie
2014-02-20 10:08 ` [PATCH 2/9] Btrfs: fix the skipped transaction commit during the file sync Miao Xie
2014-02-20 10:08 ` [PATCH 3/9] Btrfs: don't start the log transaction if the log tree init fails Miao Xie
2014-02-20 10:08 ` [PATCH 4/9] Btrfs: use bitfield instead of integer data type for the some variants in btrfs_root Miao Xie
2014-02-22  0:23   ` David Sterba
2014-02-26  9:10     ` Miao Xie
2014-02-27 17:02       ` David Sterba
2014-03-07 23:54   ` Josef Bacik
2014-03-08  0:00   ` Josef Bacik
2014-03-10  1:16     ` Miao Xie
2014-02-20 10:08 ` [PATCH 5/9] Btrfs: remove unnecessary memory barrier in btrfs_sync_log() Miao Xie
2014-02-20 10:08 ` [PATCH 6/9] Btrfs: use signed integer instead of unsigned long integer for log transid Miao Xie
2014-02-20 10:08 ` [PATCH 7/9] Btrfs: stop joining the log transaction if sync log fails Miao Xie
2014-02-22  0:27   ` David Sterba
2014-02-26  8:51     ` Miao Xie
2014-02-20 10:08 ` [PATCH 8/9] Btrfs: fix skipped error handle when log sync failed Miao Xie
2014-02-20 10:08 ` [PATCH 9/9] Btrfs: just wait or commit our own log sub-transaction Miao Xie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.