On Mon 09-05-11 10:22:37, Ted Tso wrote:
> On Mon, May 09, 2011 at 04:05:37PM +0200, Jan Kara wrote:
> > Yes. ext4_append() can return ENOSPC and passed bh will get set to NULL
> > without being marked dirty.
>
> Ah, so the right fix then is to add to make the cleanup code like this:
>
> ext4_mark_inode_dirty(handle, dir);
> ext4_handle_dirty_metadata(handle, dir, frame->bh);
> + ext4_handle_dirty_metadata(handle, dir, bh2);
> + if (bh)
> + ext4_handle_dirty_metadata(handle, dir, bh);
> dx_release(frames);
> return retval;
>
> Agreed?
Not quite. make_indexed_dir() does frame->bh = bh and bh = bh2 before
calling do_split(). So bh2 is not really carrying a valid buffer reference
at this point - even more so because do_split() does brelse() on the passed
bh so it need not be around when are at this point. The code is a real
mess. But for example attached patch will work because both callers of
do_split() do brelse() anyway.
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
>From 59729e0ab18a763ba36616a1025ce606a8721f1c Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@xxxxxxx>
Date: Mon, 9 May 2011 16:39:34 +0200
Subject: [PATCH] ext4: Stop releasing passed bh in do_split()
make_indexed_dir() needs to do error recovery on the passed bh when do_split()
fails. So do not release it early in do_split().
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
fs/ext4/namei.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3c7a06e..1cddab9 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1165,11 +1165,8 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
int err = 0, i;
bh2 = ext4_append (handle, dir, &newblock, &err);
- if (!(bh2)) {
- brelse(*bh);
- *bh = NULL;
+ if (!bh2)
goto errout;
- }
BUFFER_TRACE(*bh, "get_write_access");
err = ext4_journal_get_write_access(handle, *bh);
@@ -1235,9 +1232,7 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
return de;
journal_error:
- brelse(*bh);
brelse(bh2);
- *bh = NULL;
ext4_std_error(dir->i_sb, err);
errout:
*error = err;
--
1.7.1
