On Thu, Mar 31, 2011 at 10:04 PM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote: > On 3/31/11 3:37 AM, Yongqiang Yang wrote: > >> in ext3, ext3_freeze() prevents journal from being updated by >> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by >> unlock_journal_updates(). >> >> in ext4, however, before ext4_freeze() returns, it unlock journal, and >> ext4 prevents journal from being updated by s_frozen. s_frozen is in >> an upper layer, so it is out control of ext4 and deadlock is easy to >> happen. >> >> Could someone explain why ext4 does like above but not follow ext3? >> >> Yongqiang. > > That was me, I think ... Thank you, Eric. I think ext4_journal_start() should check if current thread has an active handle before vfs_check_frozen(), if so, current handle will be returned. Thus, we can avoid deadlocks. Do you agree with me? If I am right, I will send a patch. > > commit 6b0310fbf087ad6e9e3b8392adca97cd77184084 > Author: Eric Sandeen <sandeen@xxxxxxxxxx> > Date: Sun May 16 02:00:00 2010 -0400 > > ext4: don't return to userspace after freezing the fs with a mutex held > > ext4_freeze() used jbd2_journal_lock_updates() which takes > the j_barrier mutex, and then returns to userspace. The > kernel does not like this: > > ================================================ > [ BUG: lock held when returning to user space! ] > ------------------------------------------------ > lvcreate/1075 is leaving the kernel with locks still held! > 1 lock held by lvcreate/1075: > #0: (&journal->j_barrier){+.+...}, at: [<ffffffff811c6214>] > jbd2_journal_lock_updates+0xe1/0xf0 > > Use vfs_check_frozen() added to ext4_journal_start_sb() and > ext4_force_commit() instead. > > Addresses-Red-Hat-Bugzilla: #568503 > > > Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> > Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx> > -- Best Wishes Yongqiang Yang -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html