Liu Bo
2012-Oct-18 07:32 UTC
[PATCH] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node''s nritems
Key MOD_LOG_KEY_REMOVE_WHILE_MOVING means that we''re doing memmove inside an extent buffer node, and the node''s number of items remains unchanged, so we don''t need to increment node''s number of items during rewinding, otherwise we may get an node larger than leafsize and cause general protection errors later. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/ctree.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index b334362..2858c2f 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1142,13 +1142,13 @@ __tree_mod_log_rewind(struct extent_buffer *eb, u64 time_seq, switch (tm->op) { case MOD_LOG_KEY_REMOVE_WHILE_FREEING: BUG_ON(tm->slot < n); - case MOD_LOG_KEY_REMOVE_WHILE_MOVING: case MOD_LOG_KEY_REMOVE: + n++; + case MOD_LOG_KEY_REMOVE_WHILE_MOVING: btrfs_set_node_key(eb, &tm->key, tm->slot); btrfs_set_node_blockptr(eb, tm->slot, tm->blockptr); btrfs_set_node_ptr_generation(eb, tm->slot, tm->generation); - n++; break; case MOD_LOG_KEY_REPLACE: BUG_ON(tm->slot >= n); -- 1.7.7.6 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Schmidt
2012-Oct-19 05:13 UTC
Re: [PATCH] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node''s nritems
On Thu, October 18, 2012 at 09:32 (+0200), Liu Bo wrote:> Key MOD_LOG_KEY_REMOVE_WHILE_MOVING means that we''re doing memmove inside > an extent buffer node, and the node''s number of items remains unchanged, > so we don''t need to increment node''s number of items during rewinding, > otherwise we may get an node larger than leafsize and cause general protection > errors later.This patch triggers the following bug on when running fsmark on a quota enabled file system: 1153 case MOD_LOG_KEY_REPLACE: 1154 BUG_ON(tm->slot >= n); MOD_LOG_KEY_REMOVE_WHILE_MOVING elements are added by tree_mod_log_insert_move when dst_slot < src_slot, thus we''re reducing the number of elements in the buffer, thus replaying adds those elements. -Jan -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2012-Oct-19 05:30 UTC
Re: [PATCH] Btrfs: MOD_LOG_KEY_REMOVE_WHILE_MOVING never change node''s nritems
On 10/19/2012 01:13 PM, Jan Schmidt wrote:> On Thu, October 18, 2012 at 09:32 (+0200), Liu Bo wrote: >> Key MOD_LOG_KEY_REMOVE_WHILE_MOVING means that we''re doing memmove inside >> an extent buffer node, and the node''s number of items remains unchanged, >> so we don''t need to increment node''s number of items during rewinding, >> otherwise we may get an node larger than leafsize and cause general protection >> errors later. > > This patch triggers the following bug on when running fsmark on a quota enabled > file system: > > 1153 case MOD_LOG_KEY_REPLACE: > 1154 BUG_ON(tm->slot >= n); > > MOD_LOG_KEY_REMOVE_WHILE_MOVING elements are added by tree_mod_log_insert_move > when dst_slot < src_slot, thus we''re reducing the number of elements in the > buffer, thus replaying adds those elements. >Hi Jan, I doubt it. tree_mod_log_insert_move() has only one caller tree_mod_log_eb_move(), and tree_mod_log_eb_move() has 4 callers: - push_node_left() - balance_node_right() - insert_ptr() - del_ptr() Among the 4 callers, only del_ptr() modifies nritems without inserting other keys, like MOD_LOG_KEY_ADD, etc. So IMO memmove does not modify the nritems: | - - - 3 4 5 | => | 3 4 5 - - - | (nritems remain unchanged) It is other operations (MOD_LOG_KEY_ADD, MOD_LOG_KEY_REMOVE, MOD_LOG_KEY_REMOVE_WHILE_FREEING) that modify the nritems. I think the following patch can help us: diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 6d183f6..cdd5995 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -4722,7 +4722,8 @@ static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root, btrfs_node_key_ptr_offset(slot + 1), sizeof(struct btrfs_key_ptr) * (nritems - slot - 1)); - } else if (tree_mod_log && level) { + } + if (tree_mod_log && level) { ret = tree_mod_log_insert_key(root->fs_info, parent, slot, MOD_LOG_KEY_REMOVE); BUG_ON(ret < 0); thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html