On Jul 13, 2007 02:05 -0700, Andrew Morton wrote: > On Tue, 10 Jul 2007 16:32:47 -0700 Andrew Morton <[EMAIL PROTECTED]> wrote: > > > > + brelse(bh); > > > + up_write(&EXT4_I(inode)->xattr_sem); > > > + return error; > > > +} > > > + > > > > We're doing GFP_KERNEL memory allocations while holding xattr_sem. This > > can cause the VM to reenter the filesystem, perhaps taking i_mutex and/or > > i_truncate_sem and/or journal_start() (I forget whether this still > > happens). Have we checked whether this can occur and if so, whether we are > > OK from a lock ranking POV? Bear in mind that journalled-data mode is more > > complex in this regard. > > I notice that everyone carefully avoided addressing this ;) > > Oh well, hopefully people are testing with lockdep enabled. As long > as the fs is put under extreme memory pressure, most bugs should be reported.
I have no objection to changing these to GFP_NOFS or GFP_ATOMIC, because the number of times this function is called is really quite small (only for existing inodes when the size of the fixed fields in the inode is increasing) and the buffers are freed immediately so this won't put any undue strain on the atomic memory pools. That said, there is also a GFP_KERNEL allocations in ext3_xattr_block_set() under xattr_sem, so the same problem would exist there. I also just noticed that "buffer" and "b_entry_name" are leaked in ext4_expand_extra_isize() if the while loop is run more than one time (again a relatively rare event). Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
