[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-ext4
Subject:    Re: 2.6.33-rc1: kernel BUG at fs/ext4/inode.c:1063 (sparc)
From:       tytso () mit ! edu
Date:       2009-12-30 17:48:49
Message-ID: 20091230174849.GN4429 () thunk ! org
[Download RAW message or body]

On Wed, Dec 30, 2009 at 04:18:09PM +0300, Dmitry Monakhov wrote:
> 
> IMHO we may drop i_allocated_meta_block in ext4_release_file()
> But while looking in to this function i've found another question
> about locking
> static int ext4_release_file(struct inode *inode, struct file *filp)
> {
> 	if (EXT4_I(inode)->i_state & EXT4_STATE_DA_ALLOC_CLOSE) {
> 		ext4_alloc_da_blocks(inode);
> 		EXT4_I(inode)->i_state &= ~EXT4_STATE_DA_ALLOC_CLOSE;
> <<< Seems what i_state modification must being protected by i_mutex,
>  but currently caller don't have to hold it.

(I'm answering this in a separate message since it really is a
separate question).

Yeah, that looks like a problem --- and it exists in more than just
this one place.  Unfortunately using i_mutex to protect updates to
i_state is a bit heavyweight.  What I'm thinking about doing is
converting all of the references the i_state flags to use set_bit,
clear_bit, and test_bit, since this will allow us to safely and
cleanly set/clear/test individual bits.

A quick audit of ext3 seems to show this is potentially a problem with
ext3 as well (specifically, in fs/ext3/xattr.c's use of
EXT3_STATE_XATTR).

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]