Btrfs: stop using GFP_ATOMIC for the tree mod log allocations
Previously we held the tree mod lock when adding stuff because we use it to
check and see if we truly do want to track tree modifications. This is
admirable, but GFP_ATOMIC in a critical area that is going to get hit pretty
hard and often is not nice. So instead do our basic checks to see if we don't
need to track modifications, and if those pass then do our allocation, and then
when we go to insert the new modification check if we still care, and if we
don't just free up our mod and return. Otherwise we're good to go and we can
carry on. Thanks,
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
Signed-off-by: Chris Mason <chris.mason@fusionio.com>
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index ed50460..0d5c686 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -484,8 +484,27 @@
struct rb_node **new;
struct rb_node *parent = NULL;
struct tree_mod_elem *cur;
+ int ret = 0;
- BUG_ON(!tm || !tm->seq);
+ BUG_ON(!tm);
+
+ tree_mod_log_write_lock(fs_info);
+ if (list_empty(&fs_info->tree_mod_seq_list)) {
+ tree_mod_log_write_unlock(fs_info);
+ /*
+ * Ok we no longer care about logging modifications, free up tm
+ * and return 0. Any callers shouldn't be using tm after
+ * calling tree_mod_log_insert, but if they do we can just
+ * change this to return a special error code to let the callers
+ * do their own thing.
+ */
+ kfree(tm);
+ return 0;
+ }
+
+ spin_lock(&fs_info->tree_mod_seq_lock);
+ tm->seq = btrfs_inc_tree_mod_seq_minor(fs_info);
+ spin_unlock(&fs_info->tree_mod_seq_lock);
tm_root = &fs_info->tree_mod_log;
new = &tm_root->rb_node;
@@ -501,14 +520,17 @@
else if (cur->seq > tm->seq)
new = &((*new)->rb_right);
else {
+ ret = -EEXIST;
kfree(tm);
- return -EEXIST;
+ goto out;
}
}
rb_link_node(&tm->node, parent, new);
rb_insert_color(&tm->node, tm_root);
- return 0;
+out:
+ tree_mod_log_write_unlock(fs_info);
+ return ret;
}
/*
@@ -524,57 +546,19 @@
return 1;
if (eb && btrfs_header_level(eb) == 0)
return 1;
-
- tree_mod_log_write_lock(fs_info);
- if (list_empty(&fs_info->tree_mod_seq_list)) {
- /*
- * someone emptied the list while we were waiting for the lock.
- * we must not add to the list when no blocker exists.
- */
- tree_mod_log_write_unlock(fs_info);
- return 1;
- }
-
return 0;
}
-/*
- * This allocates memory and gets a tree modification sequence number.
- *
- * Returns <0 on error.
- * Returns >0 (the added sequence number) on success.
- */
-static inline int tree_mod_alloc(struct btrfs_fs_info *fs_info, gfp_t flags,
- struct tree_mod_elem **tm_ret)
-{
- struct tree_mod_elem *tm;
-
- /*
- * once we switch from spin locks to something different, we should
- * honor the flags parameter here.
- */
- tm = *tm_ret = kzalloc(sizeof(*tm), GFP_ATOMIC);
- if (!tm)
- return -ENOMEM;
-
- spin_lock(&fs_info->tree_mod_seq_lock);
- tm->seq = btrfs_inc_tree_mod_seq_minor(fs_info);
- spin_unlock(&fs_info->tree_mod_seq_lock);
-
- return tm->seq;
-}
-
static inline int
__tree_mod_log_insert_key(struct btrfs_fs_info *fs_info,
struct extent_buffer *eb, int slot,
enum mod_log_op op, gfp_t flags)
{
- int ret;
struct tree_mod_elem *tm;
- ret = tree_mod_alloc(fs_info, flags, &tm);
- if (ret < 0)
- return ret;
+ tm = kzalloc(sizeof(*tm), flags);
+ if (!tm)
+ return -ENOMEM;
tm->index = eb->start >> PAGE_CACHE_SHIFT;
if (op != MOD_LOG_KEY_ADD) {
@@ -589,34 +573,14 @@
}
static noinline int
-tree_mod_log_insert_key_mask(struct btrfs_fs_info *fs_info,
- struct extent_buffer *eb, int slot,
- enum mod_log_op op, gfp_t flags)
+tree_mod_log_insert_key(struct btrfs_fs_info *fs_info,
+ struct extent_buffer *eb, int slot,
+ enum mod_log_op op, gfp_t flags)
{
- int ret;
-
if (tree_mod_dont_log(fs_info, eb))
return 0;
- ret = __tree_mod_log_insert_key(fs_info, eb, slot, op, flags);
-
- tree_mod_log_write_unlock(fs_info);
- return ret;
-}
-
-static noinline int
-tree_mod_log_insert_key(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
- int slot, enum mod_log_op op)
-{
- return tree_mod_log_insert_key_mask(fs_info, eb, slot, op, GFP_NOFS);
-}
-
-static noinline int
-tree_mod_log_insert_key_locked(struct btrfs_fs_info *fs_info,
- struct extent_buffer *eb, int slot,
- enum mod_log_op op)
-{
- return __tree_mod_log_insert_key(fs_info, eb, slot, op, GFP_NOFS);
+ return __tree_mod_log_insert_key(fs_info, eb, slot, op, flags);
}
static noinline int
@@ -637,14 +601,14 @@
* buffer, i.e. dst_slot < src_slot.
*/
for (i = 0; i + dst_slot < src_slot && i < nr_items; i++) {
- ret = tree_mod_log_insert_key_locked(fs_info, eb, i + dst_slot,
- MOD_LOG_KEY_REMOVE_WHILE_MOVING);
+ ret = __tree_mod_log_insert_key(fs_info, eb, i + dst_slot,
+ MOD_LOG_KEY_REMOVE_WHILE_MOVING, GFP_NOFS);
BUG_ON(ret < 0);
}
- ret = tree_mod_alloc(fs_info, flags, &tm);
- if (ret < 0)
- goto out;
+ tm = kzalloc(sizeof(*tm), flags);
+ if (!tm)
+ return -ENOMEM;
tm->index = eb->start >> PAGE_CACHE_SHIFT;
tm->slot = src_slot;
@@ -652,10 +616,7 @@
tm->move.nr_items = nr_items;
tm->op = MOD_LOG_MOVE_KEYS;
- ret = __tree_mod_log_insert(fs_info, tm);
-out:
- tree_mod_log_write_unlock(fs_info);
- return ret;
+ return __tree_mod_log_insert(fs_info, tm);
}
static inline void
@@ -670,8 +631,8 @@
nritems = btrfs_header_nritems(eb);
for (i = nritems - 1; i >= 0; i--) {
- ret = tree_mod_log_insert_key_locked(fs_info, eb, i,
- MOD_LOG_KEY_REMOVE_WHILE_FREEING);
+ ret = __tree_mod_log_insert_key(fs_info, eb, i,
+ MOD_LOG_KEY_REMOVE_WHILE_FREEING, GFP_NOFS);
BUG_ON(ret < 0);
}
}
@@ -683,7 +644,6 @@
int log_removal)
{
struct tree_mod_elem *tm;
- int ret;
if (tree_mod_dont_log(fs_info, NULL))
return 0;
@@ -691,9 +651,9 @@
if (log_removal)
__tree_mod_log_free_eb(fs_info, old_root);
- ret = tree_mod_alloc(fs_info, flags, &tm);
- if (ret < 0)
- goto out;
+ tm = kzalloc(sizeof(*tm), flags);
+ if (!tm)
+ return -ENOMEM;
tm->index = new_root->start >> PAGE_CACHE_SHIFT;
tm->old_root.logical = old_root->start;
@@ -701,10 +661,7 @@
tm->generation = btrfs_header_generation(old_root);
tm->op = MOD_LOG_ROOT_REPLACE;
- ret = __tree_mod_log_insert(fs_info, tm);
-out:
- tree_mod_log_write_unlock(fs_info);
- return ret;
+ return __tree_mod_log_insert(fs_info, tm);
}
static struct tree_mod_elem *
@@ -784,23 +741,20 @@
if (tree_mod_dont_log(fs_info, NULL))
return;
- if (btrfs_header_level(dst) == 0 && btrfs_header_level(src) == 0) {
- tree_mod_log_write_unlock(fs_info);
+ if (btrfs_header_level(dst) == 0 && btrfs_header_level(src) == 0)
return;
- }
for (i = 0; i < nr_items; i++) {
- ret = tree_mod_log_insert_key_locked(fs_info, src,
+ ret = __tree_mod_log_insert_key(fs_info, src,
i + src_offset,
- MOD_LOG_KEY_REMOVE);
+ MOD_LOG_KEY_REMOVE, GFP_NOFS);
BUG_ON(ret < 0);
- ret = tree_mod_log_insert_key_locked(fs_info, dst,
+ ret = __tree_mod_log_insert_key(fs_info, dst,
i + dst_offset,
- MOD_LOG_KEY_ADD);
+ MOD_LOG_KEY_ADD,
+ GFP_NOFS);
BUG_ON(ret < 0);
}
-
- tree_mod_log_write_unlock(fs_info);
}
static inline void
@@ -819,9 +773,9 @@
{
int ret;
- ret = tree_mod_log_insert_key_mask(fs_info, eb, slot,
- MOD_LOG_KEY_REPLACE,
- atomic ? GFP_ATOMIC : GFP_NOFS);
+ ret = __tree_mod_log_insert_key(fs_info, eb, slot,
+ MOD_LOG_KEY_REPLACE,
+ atomic ? GFP_ATOMIC : GFP_NOFS);
BUG_ON(ret < 0);
}
@@ -830,10 +784,7 @@
{
if (tree_mod_dont_log(fs_info, eb))
return;
-
__tree_mod_log_free_eb(fs_info, eb);
-
- tree_mod_log_write_unlock(fs_info);
}
static noinline void
@@ -1083,7 +1034,7 @@
WARN_ON(trans->transid != btrfs_header_generation(parent));
tree_mod_log_insert_key(root->fs_info, parent, parent_slot,
- MOD_LOG_KEY_REPLACE);
+ MOD_LOG_KEY_REPLACE, GFP_NOFS);
btrfs_set_node_blockptr(parent, parent_slot,
cow->start);
btrfs_set_node_ptr_generation(parent, parent_slot,
@@ -3208,7 +3159,7 @@
}
if (level) {
ret = tree_mod_log_insert_key(root->fs_info, lower, slot,
- MOD_LOG_KEY_ADD);
+ MOD_LOG_KEY_ADD, GFP_NOFS);
BUG_ON(ret < 0);
}
btrfs_set_node_key(lower, key, slot);
@@ -4642,7 +4593,7 @@
(nritems - slot - 1));
} else if (level) {
ret = tree_mod_log_insert_key(root->fs_info, parent, slot,
- MOD_LOG_KEY_REMOVE);
+ MOD_LOG_KEY_REMOVE, GFP_NOFS);
BUG_ON(ret < 0);
}