On 5/9/2011 7:42 AM, Jan Kara wrote:
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
Hi all,
Oh, I understand the problem now. Sorry for the late response, I had to
stop and dig around with this one for a bit. Would people prefer to add
the new code before the do_split like this:
+ ext4_handle_dirty_metadata(handle, dir, frame->bh);
+ ext4_handle_dirty_metadata(handle, dir, bh);
+
de = do_split(handle,dir, &bh, frame, &hinfo, &retval);
if (!de) {
/*
@@ -1421,8 +1425,6 @@ static int make_indexed_dir(handle_t *handle,
struct dentry *dentry,
* with corrupted filesystem.
*/
ext4_mark_inode_dirty(handle, dir);
- ext4_handle_dirty_metadata(handle, dir, frame->bh);
- ext4_handle_dirty_metadata(handle, dir, bh);
dx_release(frames);
return retval;
}
I've tested both patches and they both seem to resolve the null pointer.
The only other solution that comes to mind would be to add flags to
the do_split to skip the brelse or to do the mark dirty before the
brelse as you suggest.
Allison Henderson
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
