Jan Schmidt | 14 Jun 2012 19:16

[PATCH 0/6] Btrfs: tree mod log fixes for 3.5-rc3

Hi Chris,

This are my current fixes for the tree mod log. With these patches,
backref walking looks really stable now. My qgroup tests are getting
really close to succeeding :-)

Everything's pretty small and non-invasive, so it should be fine for the
upcomming rc.

You can also pull from

	git://git.jan-o-sch.net/btrfs-unstable for-chris

Thanks,
Jan

Jan Schmidt (6):
  Btrfs: remove call to btrfs_header_nritems with no effect
  Btrfs: remove obsolete btrfs_next_leaf call from
    __resolve_indirect_ref
  Btrfs: use btrfs_read_lock_root_node in get_old_root
  Btrfs: fix return value for __tree_mod_log_oldest_root
  Btrfs: add btrfs_next_old_leaf
  Btrfs: fix race in tree mod log addition

 fs/btrfs/backref.c |   17 +++-------
 fs/btrfs/ctree.c   |   86 ++++++++++++++++++++++++++++++++++++++--------------
 fs/btrfs/ctree.h   |    2 +
 3 files changed, 70 insertions(+), 35 deletions(-)

(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 1/6] Btrfs: remove call to btrfs_header_nritems with no effect

This is a leftover from cleanup patch 559af821. Before the cleanup,
btrfs_header_nritems was called inside an if condition. As it has no side
effects we need to preserve here, it should simply be dropped.

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/ctree.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index d7a96cf..836e4e0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
 <at>  <at>  -1650,8 +1650,6  <at>  <at>  static noinline int balance_level(struct btrfs_trans_handle *trans,
 	    BTRFS_NODEPTRS_PER_BLOCK(root) / 4)
 		return 0;

-	btrfs_header_nritems(mid);
-
 	left = read_node_slot(root, parent, pslot - 1);
 	if (left) {
 		btrfs_tree_lock(left);
 <at>  <at>  -1681,7 +1679,6  <at>  <at>  static noinline int balance_level(struct btrfs_trans_handle *trans,
 		wret = push_node_left(trans, root, left, mid, 1);
 		if (wret < 0)
 			ret = wret;
-		btrfs_header_nritems(mid);
 	}

 	/*
(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 4/6] Btrfs: fix return value for __tree_mod_log_oldest_root

In __tree_mod_log_oldest_root() we must return the found operation even if
it's not a ROOT_REPLACE operation. Otherwise, the caller assumes that there
are no operations to be rewinded and returns immediately.

The code in the caller is modified to improve readability.

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/ctree.c |   33 ++++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 2cde7b0..50d7c99 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
 <at>  <at>  -1023,6 +1023,10  <at>  <at>  __tree_mod_log_oldest_root(struct btrfs_fs_info *fs_info,
 		looped = 1;
 	}

+	/* if there's no old root to return, return what we found instead */
+	if (!found)
+		found = tm;
+
 	return found;
 }

 <at>  <at>  -1155,18 +1159,24  <at>  <at>  get_old_root(struct btrfs_root *root, u64 time_seq)
 {
 	struct tree_mod_elem *tm;
 	struct extent_buffer *eb;
(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 6/6] Btrfs: fix race in tree mod log addition

When adding to the tree modification log, we grab two locks at different
stages. We must not drop the outer lock until we're done with section
protected by the inner lock. This moves the unlock call for the outer lock
to the appropriate position.

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/ctree.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cb76b2a..04b06bc 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
 <at>  <at>  -467,6 +467,15  <at>  <at>  static inline int tree_mod_dont_log(struct btrfs_fs_info *fs_info,
 	return 0;
 }

+/*
+ * This allocates memory and gets a tree modification sequence number when
+ * needed.
+ *
+ * Returns 0 when no sequence number is needed, < 0 on error.
+ * Returns 1 when a sequence number was added. In this case,
+ * fs_info->tree_mod_seq_lock was acquired and must be released by the caller
+ * after inserting into the rb tree.
+ */
 static inline int tree_mod_alloc(struct btrfs_fs_info *fs_info, gfp_t flags,
 				 struct tree_mod_elem **tm_ret)
 {
(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 3/6] Btrfs: use btrfs_read_lock_root_node in get_old_root

get_old_root could race with root node updates because we weren't locking
the node early enough. Use btrfs_read_lock_root_node to grab the root locked
in the very beginning and release the lock as soon as possible (just like
btrfs_search_slot does).

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/ctree.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 836e4e0..2cde7b0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
 <at>  <at>  -1143,6 +1143,13  <at>  <at>  tree_mod_log_rewind(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
 	return eb_rewin;
 }

+/*
+ * get_old_root() rewinds the state of  <at> root's root node to the given  <at> time_seq
+ * value. If there are no changes, the current root->root_node is returned. If
+ * anything changed in between, there's a fresh buffer allocated on which the
+ * rewind operations are done. In any case, the returned buffer is read locked.
+ * Returns NULL on error (with no locks held).
+ */
 static inline struct extent_buffer *
 get_old_root(struct btrfs_root *root, u64 time_seq)
 {
 <at>  <at>  -1151,6 +1158,7  <at>  <at>  get_old_root(struct btrfs_root *root, u64 time_seq)
 	struct tree_mod_root *old_root;
(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 5/6] Btrfs: add btrfs_next_old_leaf

To make sense of the tree mod log, the backref walker not only needs
btrfs_search_old_slot, but it also called btrfs_next_leaf, which in turn was
calling btrfs_search_slot. This obviously didn't give the correct result.

This commit adds btrfs_next_old_leaf, a drop-in replacement for
btrfs_next_leaf with a time_seq parameter. If it is zero, it behaves exactly
like btrfs_next_leaf. If it is non-zero, it will use btrfs_search_old_slot
with this time_seq parameter.

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/backref.c |    7 ++++---
 fs/btrfs/ctree.c   |   11 ++++++++++-
 fs/btrfs/ctree.h   |    2 ++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 579131d..8f7d123 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
 <at>  <at>  -179,7 +179,8  <at>  <at>  static int __add_prelim_ref(struct list_head *head, u64 root_id,

 static int add_all_parents(struct btrfs_root *root, struct btrfs_path *path,
 				struct ulist *parents, int level,
-				struct btrfs_key *key, u64 wanted_disk_byte,
+				struct btrfs_key *key, u64 time_seq,
+				u64 wanted_disk_byte,
 				const u64 *extent_item_pos)
 {
 	int ret;
(Continue reading)

Jan Schmidt | 14 Jun 2012 19:16

[PATCH 2/6] Btrfs: remove obsolete btrfs_next_leaf call from __resolve_indirect_ref

When resolving indirect refs, we used to call btrfs_next_leaf in case we
didn't find an exact match. While we should find exact matches most of the
time, in case we don't, we must continue searching. Treating those matches
differently depending on the level we're searching doesn't make sense.

Even worse, we might end up searching for a key larger than the largest, in
which case there is no next_leaf and subsequent jobs would fail. This commit
drops the bogous lines.

Signed-off-by: Jan Schmidt <list.btrfs <at> jan-o-sch.net>
---
 fs/btrfs/backref.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 3f75895..579131d 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
 <at>  <at>  -294,16 +294,8  <at>  <at>  static int __resolve_indirect_ref(struct btrfs_fs_info *fs_info,
 		goto out;
 	}

-	if (level == 0) {
-		if (ret == 1 && path->slots[0] >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret)
-				goto out;
-			eb = path->nodes[0];
-		}
-
(Continue reading)


Gmane