On 02/10/2012 04:31 AM, Martin Wilck wrote:
> I don't want to bother, but could someone have a look at this
> patch please?
You have to bother, to get attention ;)
> Martin
>
> Processes hang forever on a sync-mounted ext2 file system that
> is mounted with the ext4 module (default in Fedora 16).
>
> I can reproduce this reliably by mounting an ext2 partition with
> "-o sync" and opening a new file an that partition with vim. vim
> will hang in "D" state forever.
>
> I am attaching a small patch here that solves this issue for me.
> In the sync mounted case, ext4_handle_dirty_metadata() may call
> sync_dirty_buffer(), which can't be called with buffer lock held.
>
> My knowledge of locking in ext4 is insufficient to judge if this
> patch is correct, but it may serve as starting point for others.
This looks an awful lot like:
b2f49033d80c952a0ffc2d5647bc1a0b8a09c1b3 [PATCH] fix deadlock in ext2
from 2006! :)
But I'm also eyeballing:
8a2bfdcbfa441d8b0e5cb9c9a7f45f77f80da465 [PATCH] ext[34]: EA block
reference count racing fix
to be sure it doesn't regress anything there.
And I'd really like to know why this only happens on ext2-with-ext4
(ext4 nojournal seems ok... hm, actually, I can't repro it on
ext2 either...)
Ahah. Ok, makes sense, it has to be mkfs'd with a 128-byte inode, so
that selinux xattrs will go into an external block. Now I can reproduce
it on ext2. But still not on ext4, with 128-byte-inodes
and nojournal. Odd. I'd like to sort that out.
The patch also unlocks prior to mb_cache_entry_release(), and I think
that might put some of the raciness back?
-Eric
> Signed-off-by: Martin.Wilck@xxxxxxxxxxxxxx
> ---
> fs/ext4/xattr.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 93a00d8..eb771ea 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -487,8 +487,10 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
> ext4_free_blocks(handle, inode, bh, 0, 1,
> EXT4_FREE_BLOCKS_METADATA |
> EXT4_FREE_BLOCKS_FORGET);
> + unlock_buffer(bh);
> } else {
> le32_add_cpu(&BHDR(bh)->h_refcount, -1);
> + unlock_buffer(bh);
> error = ext4_handle_dirty_metadata(handle, inode, bh);
> if (IS_SYNC(inode))
> ext4_handle_sync(handle);
> @@ -498,7 +500,6 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
> if (ce)
> mb_cache_entry_release(ce);
> }
> - unlock_buffer(bh);
> out:
> ext4_std_error(inode->i_sb, error);
> return;
--
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
