On Wed, Jul 6, 2011 at 3:54 PM, Andreas Dilger <adilger@xxxxxxxxx> wrote: > On 2011-01-16, at 11:05 AM, Manish Katiyar wrote: >> On Sun, Jan 16, 2011 at 8:20 AM, Andreas Dilger <adilger@xxxxxxxxx> wrote: >>> Why not just put the iput() at failed_mount4() instead of spread around the code? >> >> Thanks Andreas, Here is the updated patch. > > We are hitting this same problem due to ENOMEM on allocating some large > filesystem structures for 128TB filesystems. However, when we were going > to add this patch to our patch series (until vendor kernels include it), > Yu Jian (one of our developers) noticed a problem with the patch. > > In the error path, the patch is doing: > > failed_mount4: > iput(root); > sb->s_root = NULL; > > but in fact sb->s_root is a dentry allocated by d_alloc_root(), so the > above code is freeing the inode, but still leaking the dentry. This > is of course a lot better than before (no oops), but still isn't correct. > > The original problem appears to have been inadvertently fixed with commit > 8aefcd557d26d0023a36f9ec5afbf55e59f8f26b, because ext4_clear_inode() now > checks "if (EXT4_I(inode)->jinode)" before deferencing EXT4_SB() and the > now-NULL s_fs_info. jinode should be NULL during mount, because it has > never been opened. I haven't confirmed this theory yet, however. Manish, > can you please give this a try with your fault-injection testing? > > It looks like we could revert 32a9bb57d7c1fd04ae0f72b8f671501f000a0e9f > (this patch, leaving the two explicit iput() in place in case of a bad > root inode or dentry) and leave the VFS to clean up the root dentry. Hi Andreas, I reverted my original patch in latest Ted's tree and retried. ext4_clear_inode() is fixed, but we still panic in #14 <signal handler called> #15 ext4_sync_fs (sb=0x17d6ee00, wait=0) at fs/ext4/super.c:4191 #16 0x080d5214 in __sync_filesystem (sb=0x17d6ee00, wait=0) at fs/sync.c:49 #17 0x080d5277 in sync_filesystem (sb=0x17d6ee00) at fs/sync.c:74 #18 0x080bc188 in generic_shutdown_super (sb=0x17d6ee00) at fs/super.c:282 #19 0x080bc236 in kill_block_super (sb=0x17d6ee00) at fs/super.c:856 #20 0x080bc3fe in deactivate_locked_super (s=0x17d6ee00) at fs/super.c:183 #21 0x080bc96b in mount_bdev (fs_type=0x8264974, flags=0, dev_name=0x17d74ed0 "/dev/loop0", data=0x0, fill_super=0x81236f1 <ext4_fill_super>) at fs/super.c:831 #22 0x0811fc36 in ext4_mount (fs_type=0x8264974, flags=0, dev_name=0x17d74ed0 "/dev/loop0", data=0x0) at fs/ext4/super.c:4820 (gdb) f 15 #15 ext4_sync_fs (sb=0x17d6ee00, wait=0) at fs/ext4/super.c:4191 4191 flush_workqueue(sbi->dio_unwritten_wq); (gdb) p sbi $1 = (struct ext4_sb_info *) 0x0 -- Thanks - Manish -- 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
