* [PATCH next/mmotm] ext4: fix cache_es after merge_left
@ 2012-09-27 20:31 Hugh Dickins
2012-09-27 20:39 ` Theodore Ts'o
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2012-09-27 20:31 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Zheng Liu, Yongqiang Yang, Allison Henderson, Lukas Czerner,
Andrew Morton, linux-ext4, linux-kernel
Kernel build with CONFIG_DEBUG_SLAB or CONFIG_SLUB_DEBUG slub_debug=FPZ
gives me kernel BUG at fs/ext4/extents_status.c:142! That's the
BUG_ON(es->start + es->len < es->start) in extent_status_end() called
from ext4_es_insert_extent(). tree->cache_es has been freed and poisoned.
This comes from when ext4_es_try_to_merge_left() merges es into leftward
es1, but ext4_es_insert_extent()'s out then updates cache_es to the freed
extent_status. ext4_es_try_to_merge_right() does not pose a problem.
Change ext4_es_try_to_merge_left() to return whichever extent_status
should be recorded in tree->cache_es. Remove cache_es update from
both of them, leaving that to ext4_es_insert_extent()'s out label.
Signed-off-by: Hugh Dickins <hughd@google.com>
---
fs/ext4/extents_status.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
--- mmotm/fs/ext4/extents_status.c 2012-09-26 10:15:29.340071552 -0700
+++ linux/fs/ext4/extents_status.c 2012-09-27 11:52:59.284937056 -0700
@@ -244,24 +244,24 @@ static void ext4_es_free_extent(struct e
kmem_cache_free(ext4_es_cachep, es);
}
-static void ext4_es_try_to_merge_left(struct ext4_es_tree *tree,
- struct extent_status *es)
+static struct extent_status *
+ext4_es_try_to_merge_left(struct ext4_es_tree *tree, struct extent_status *es)
{
struct extent_status *es1;
struct rb_node *node;
node = rb_prev(&es->rb_node);
if (!node)
- return;
+ return es;
es1 = rb_entry(node, struct extent_status, rb_node);
if (es->start == extent_status_end(es1) + 1) {
es1->len += es->len;
rb_erase(&es->rb_node, &tree->root);
- if (es == tree->cache_es)
- tree->cache_es = es1;
ext4_es_free_extent(es);
+ es = es1; /* Caller will update tree->cache_es to this */
}
+ return es;
}
static void ext4_es_try_to_merge_right(struct ext4_es_tree *tree,
@@ -278,9 +278,8 @@ static void ext4_es_try_to_merge_right(s
if (es1->start == extent_status_end(es) + 1) {
es->len += es1->len;
rb_erase(node, &tree->root);
- if (es1 == tree->cache_es)
- tree->cache_es = es;
ext4_es_free_extent(es1);
+ /* Caller will update tree->cache_es to es */
}
}
@@ -318,7 +317,7 @@ int ext4_es_insert_extent(struct inode *
es_debug("cached by [%u/%u)\n", es->start, es->len);
es->start = offset;
es->len += len;
- ext4_es_try_to_merge_left(tree, es);
+ es = ext4_es_try_to_merge_left(tree, es);
goto out;
} else if (es && in_range(offset, es->start, es->len)) {
es_debug("cached by [%u/%u)\n", es->start, es->len);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH next/mmotm] ext4: fix cache_es after merge_left
2012-09-27 20:31 [PATCH next/mmotm] ext4: fix cache_es after merge_left Hugh Dickins
@ 2012-09-27 20:39 ` Theodore Ts'o
2012-09-27 22:53 ` Zheng Liu
0 siblings, 1 reply; 3+ messages in thread
From: Theodore Ts'o @ 2012-09-27 20:39 UTC (permalink / raw)
To: Hugh Dickins
Cc: Zheng Liu, Yongqiang Yang, Allison Henderson, Lukas Czerner,
Andrew Morton, linux-ext4, linux-kernel
On Thu, Sep 27, 2012 at 01:31:28PM -0700, Hugh Dickins wrote:
> Kernel build with CONFIG_DEBUG_SLAB or CONFIG_SLUB_DEBUG slub_debug=FPZ
> gives me kernel BUG at fs/ext4/extents_status.c:142! That's the
> BUG_ON(es->start + es->len < es->start) in extent_status_end() called
> from ext4_es_insert_extent(). tree->cache_es has been freed and poisoned.
>
> This comes from when ext4_es_try_to_merge_left() merges es into leftward
> es1, but ext4_es_insert_extent()'s out then updates cache_es to the freed
> extent_status. ext4_es_try_to_merge_right() does not pose a problem.
>
> Change ext4_es_try_to_merge_left() to return whichever extent_status
> should be recorded in tree->cache_es. Remove cache_es update from
> both of them, leaving that to ext4_es_insert_extent()'s out label.
>
> Signed-off-by: Hugh Dickins <hughd@google.com>
Hugh, thanks for finding this bug!
Zheng, you were going to send me an updated patch series; can you take
care of merging this patch into your patch series (and crediting Hugh
with a Signed-off-by as appropriate)?
Thanks!!
- Ted
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH next/mmotm] ext4: fix cache_es after merge_left
2012-09-27 20:39 ` Theodore Ts'o
@ 2012-09-27 22:53 ` Zheng Liu
0 siblings, 0 replies; 3+ messages in thread
From: Zheng Liu @ 2012-09-27 22:53 UTC (permalink / raw)
To: Theodore Ts'o, Hugh Dickins, Zheng Liu, Yongqiang Yang,
Allison Henderson, Lukas Czerner, Andrew Morton, linux-ext4,
linux-kernel
On Thu, Sep 27, 2012 at 04:39:20PM -0400, Theodore Ts'o wrote:
> On Thu, Sep 27, 2012 at 01:31:28PM -0700, Hugh Dickins wrote:
> > Kernel build with CONFIG_DEBUG_SLAB or CONFIG_SLUB_DEBUG slub_debug=FPZ
> > gives me kernel BUG at fs/ext4/extents_status.c:142! That's the
> > BUG_ON(es->start + es->len < es->start) in extent_status_end() called
> > from ext4_es_insert_extent(). tree->cache_es has been freed and poisoned.
> >
> > This comes from when ext4_es_try_to_merge_left() merges es into leftward
> > es1, but ext4_es_insert_extent()'s out then updates cache_es to the freed
> > extent_status. ext4_es_try_to_merge_right() does not pose a problem.
> >
> > Change ext4_es_try_to_merge_left() to return whichever extent_status
> > should be recorded in tree->cache_es. Remove cache_es update from
> > both of them, leaving that to ext4_es_insert_extent()'s out label.
> >
> > Signed-off-by: Hugh Dickins <hughd@google.com>
>
> Hugh, thanks for finding this bug!
>
> Zheng, you were going to send me an updated patch series; can you take
> care of merging this patch into your patch series (and crediting Hugh
> with a Signed-off-by as appropriate)?
Yeah, thanks for fixing it, although I have fixed this problem in my new
patch set. I will add a Signed-off-by into the patch.
Regards,
Zheng
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-27 22:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27 20:31 [PATCH next/mmotm] ext4: fix cache_es after merge_left Hugh Dickins
2012-09-27 20:39 ` Theodore Ts'o
2012-09-27 22:53 ` Zheng Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).