Re: [PATCH 1/2] [PATCH] ext4: Add inode to the orphan list during block allocation failure

[Theodore Tso <tytso@mit.edu>]

On Tue, Mar 31, 2009 at 02:59:25PM +0530, Aneesh Kumar K.V wrote:
> We should add inode to the orphan list in the same transaction
> as block allocation. This ensures that if we crash after a failed
> block allocation and before we do a vmtruncate we don't leak block
> (ie block marked as used in bitmap but not claimed by the inode).

How likely is this going to happen?  If it's a failure in the block
allocation, we will have really end up with blocks outside i_size?
And in that case, I'm missing how this ends up being a leaked block,
since presumably the block is still being referenced by the inode.  Am
I missing something here?

I guess it can happen if do_journal_get_write_access() returns an
error, which could potentially happen if there is a ENOMEM error
coming from the jbd2 layer.  But that brings up another potential
problem.  It's possible for vmtruncate() to fail; if ext4_truncate()
fails too early (particularly in ext4_ext_truncate, due to a memory
error in a jbd2 routines), it's possible for it to return without
calling ext4_orphan_del() --- and stray inodes on the orphan list will
cause the kernel to panic on umount.

I think this can be fixed by making sure that ext4_truncate() and
ext4_ext_truncate() calls ext4_orphan_del() in *all* of their error
paths.  That *should* the problem, since at the moment, it doesn't
look vmtruncate() will return without calling inode->i_op->truncate().
But could you double check this carefully?

Thanks,

						- Ted