On Tue, 12 Mar 2013, fanchaoting wrote:
> Date: Tue, 12 Mar 2013 13:06:37 +0800
> From: fanchaoting <fanchaoting@xxxxxxxxxxxxxx>
> To: jack@xxxxxxx
> Cc: tyhicks@xxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx,
> wangshilong1991@xxxxxxxxx
> Subject: [PATCH] Ext2: do not mark_inode_dirty to avoid BUG_ON
>
> From: Wang Shilong <wangsl-fnst@xxxxxxxxxxxxxx>
>
> commit 8e3dffc651cb668e1ff4d8b89cc1c3dde7540d3b leads into
> a regression that casue BUG_ON when unlinking inode.
Hi,
it seems to be that we do need to mark the inode dirty, because
we're changing inode->i_blocks from within
dquot_free_block_nodirty().
However looking at the code we usually call mark_inode_dirty(inode)
after we call ext2_free_blocks() except when we're about to remove
the inode so it seems that having that call within ext2_free_blocks()
is not necessary.
However I am not sure about the error path in ext2_alloc_branch()
which does not dirty the inode after calling ext2_free_blocks().
However presumably since we're just undoing the changes we might
have done and not actually allocating, or freeing any space for
real, dirtying the inode might not be necessary. Can you confirm
that ?
Thanks!
-Lukas
>
> Reported-by: tyhicks@xxxxxxxxxxxxx
> Signed-off-by: Wang Shilong <wangsl-fnst@xxxxxxxxxxxxxx>
> ---
> fs/ext2/balloc.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
> index 9f9992b..06d82fc 100644
> --- a/fs/ext2/balloc.c
> +++ b/fs/ext2/balloc.c
> @@ -562,7 +562,6 @@ error_return:
> if (freed) {
> percpu_counter_add(&sbi->s_freeblocks_counter, freed);
> dquot_free_block_nodirty(inode, freed);
> - mark_inode_dirty(inode);
> }
> }
>
> -- 1.7.7.6
>
>
>
>
> --
> 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
>
--
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
