On 3/31/11 9:36 AM, Yongqiang Yang wrote:

On Thu, Mar 31, 2011 at 10:04 PM, Eric Sandeen [off-list ref] wrote:

quoted

On 3/31/11 3:37 AM, Yongqiang Yang wrote:

quoted

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.

If you have a testcase to test it with, sure.  plus a patch would help me know for sure what you propose :)

Sorry for breaking it (if I did!)  But holding a mutex and returning to userspace was pretty bad, too :(

Thanks,
-Eric

quoted

commit 6b0310fbf087ad6e9e3b8392adca97cd77184084
Author: Eric Sandeen [off-list ref]
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 [off-list ref]
   Signed-off-by: "Theodore Ts'o" [off-list ref]