lkml.org 
[lkml]   [2014]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v7 07/22] Replace the XIP page fault handler with the DAX page fault handler
    On Tue 29-07-14 17:23:33, Matthew Wilcox wrote:
    > On Tue, Jul 29, 2014 at 11:04:57PM +0200, Jan Kara wrote:
    > > > Path 1:
    > > >
    > > > ext4_fallocate ->
    > > > ext4_punch_hole ->
    > > > ext4_inode_attach_jinode() -> ... ->
    > > > lock_map_acquire(&handle->h_lockdep_map);
    > > > truncate_pagecache_range() ->
    > > > unmap_mapping_range() ->
    > > > mutex_lock(&mapping->i_mmap_mutex);
    > > This is strange. I don't see how ext4_inode_attach_jinode() can ever lead
    > > to lock_map_acquire(&handle->h_lockdep_map). Can you post a full trace for
    > > this?
    >
    > Unfortunately, lockdep finds the inversion in the other order, so I
    > have the backtraces of this path hitting the i_mmap_mutex while already
    > holding jbd_mutex:
    I see the problem now. How about an attached patch? Do you see other
    lockdep warnings with it?

    Honza
    >
    > ======================================================
    > [ INFO: possible circular locking dependency detected ]
    > 3.16.0-rc6+ #91 Tainted: G W
    > -------------------------------------------------------
    > fstest/31836 is trying to acquire lock:
    > (jbd2_handle){+.+.+.}, at: [<ffffffffa00f5333>] start_this_handle+0x193/0x630 [jbd2]
    >
    > but task is already holding lock:
    > (&mapping->i_mmap_mutex){+.+...}, at: [<ffffffff8124c0a0>] do_dax_fault+0x4e0/0x640
    >
    > which lock already depends on the new lock.
    >
    >
    > the existing dependency chain (in reverse order) is:
    >
    > -> #1 (&mapping->i_mmap_mutex){+.+...}:
    > [<ffffffff810cfa22>] lock_acquire+0xb2/0x1f0
    > [<ffffffff815cad15>] mutex_lock_nested+0x75/0x420
    > [<ffffffff811acf4b>] unmap_mapping_range+0x6b/0x180
    > [<ffffffff811901ba>] truncate_pagecache_range+0x4a/0x60
    > [<ffffffffa020af41>] ext4_punch_hole+0x4d1/0x530 [ext4]
    > [<ffffffffa0235356>] ext4_fallocate+0x156/0xb70 [ext4]
    > [<ffffffff811f3c19>] do_fallocate+0x119/0x1b0
    > [<ffffffff811f3cf3>] SyS_fallocate+0x43/0x70
    > [<ffffffff815cf8a9>] system_call_fastpath+0x16/0x1b
    >
    > -> #0 (jbd2_handle){+.+.+.}:
    > [<ffffffff810ce9e1>] __lock_acquire+0x1d01/0x1eb0
    > [<ffffffff810cfa22>] lock_acquire+0xb2/0x1f0
    > [<ffffffffa00f538e>] start_this_handle+0x1ee/0x630 [jbd2]
    > [<ffffffffa00f5c04>] jbd2__journal_start+0xd4/0x260 [jbd2]
    > [<ffffffffa0235f6d>] __ext4_journal_start_sb+0x6d/0x190 [ext4]
    > [<ffffffffa0206fca>] _ext4_get_block+0x16a/0x1c0 [ext4]
    > [<ffffffffa0207036>] ext4_get_block+0x16/0x20 [ext4]
    > [<ffffffff8124c199>] do_dax_fault+0x5d9/0x640
    > [<ffffffff8124c23f>] dax_fault+0x3f/0x90
    > [<ffffffffa01ff975>] ext4_dax_fault+0x15/0x20 [ext4]
    > [<ffffffff811ab6d1>] __do_fault+0x41/0xd0
    > [<ffffffff811ae7f5>] do_shared_fault.isra.56+0x35/0x220
    > [<ffffffff811af983>] handle_mm_fault+0x303/0xf70
    > [<ffffffff81062d2c>] __do_page_fault+0x1ec/0x5b0
    > [<ffffffff81063112>] do_page_fault+0x22/0x30
    > [<ffffffff815d18b8>] page_fault+0x28/0x30
    >
    > other info that might help us debug this:
    >
    > Possible unsafe locking scenario:
    >
    > CPU0 CPU1
    > ---- ----
    > lock(&mapping->i_mmap_mutex);
    > lock(jbd2_handle);
    > lock(&mapping->i_mmap_mutex);
    > lock(jbd2_handle);
    >
    > *** DEADLOCK ***
    >
    > 3 locks held by fstest/31836:
    > #0: (&mm->mmap_sem){++++++}, at: [<ffffffff81062cc2>] __do_page_fault+0x182/0x5b0
    > #1: (sb_pagefaults){++++..}, at: [<ffffffff8124c27a>] dax_fault+0x7a/0x90
    > #2: (&mapping->i_mmap_mutex){+.+...}, at: [<ffffffff8124c0a0>] do_dax_fault+0x4e0/0x640
    >
    > stack backtrace:
    > CPU: 6 PID: 31836 Comm: fstest Tainted: G W 3.16.0-rc6+ #91
    > Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Q87M-D2H, BIOS F6 08/03/2013
    > ffffffff825e63e0 ffff8800a0fc78c0 ffffffff815c6bc3 ffffffff825e63e0
    > ffff8800a0fc7900 ffffffff815c4e59 ffff8800a0fc7970 ffff8800a88f4a50
    > ffff8800a88f4af8 ffff8800a88f5280 0000000000000003 ffff8800a88f5248
    > Call Trace:
    > [<ffffffff815c6bc3>] dump_stack+0x4d/0x66
    > [<ffffffff815c4e59>] print_circular_bug+0x201/0x20f
    > [<ffffffff810ce9e1>] __lock_acquire+0x1d01/0x1eb0
    > [<ffffffff81023b00>] ? cyc2ns_read_end+0x20/0x20
    > [<ffffffff810cfa22>] lock_acquire+0xb2/0x1f0
    > [<ffffffffa00f5333>] ? start_this_handle+0x193/0x630 [jbd2]
    > [<ffffffffa00f538e>] start_this_handle+0x1ee/0x630 [jbd2]
    > [<ffffffffa00f5333>] ? start_this_handle+0x193/0x630 [jbd2]
    > [<ffffffffa00f5020>] ? new_handle+0x20/0x60 [jbd2]
    > [<ffffffffa00f5c04>] jbd2__journal_start+0xd4/0x260 [jbd2]
    > [<ffffffffa0206fca>] ? _ext4_get_block+0x16a/0x1c0 [ext4]
    > [<ffffffffa0235f6d>] __ext4_journal_start_sb+0x6d/0x190 [ext4]
    > [<ffffffffa0206fca>] _ext4_get_block+0x16a/0x1c0 [ext4]
    > [<ffffffffa0207036>] ext4_get_block+0x16/0x20 [ext4]
    > [<ffffffff8124c199>] do_dax_fault+0x5d9/0x640
    > [<ffffffffa0207020>] ? _ext4_get_block+0x1c0/0x1c0 [ext4]
    > [<ffffffffa0207020>] ? _ext4_get_block+0x1c0/0x1c0 [ext4]
    > [<ffffffff8124c23f>] dax_fault+0x3f/0x90
    > [<ffffffffa01ff975>] ext4_dax_fault+0x15/0x20 [ext4]
    > [<ffffffff811ab6d1>] __do_fault+0x41/0xd0
    > [<ffffffff811ae7f5>] do_shared_fault.isra.56+0x35/0x220
    > [<ffffffff811af983>] handle_mm_fault+0x303/0xf70
    > [<ffffffff810ca676>] ? __lock_is_held+0x56/0x80
    > [<ffffffff81062d2c>] __do_page_fault+0x1ec/0x5b0
    > [<ffffffff8119dc3c>] ? vm_mmap_pgoff+0x9c/0xc0
    > [<ffffffff810c80cf>] ? up_write+0x1f/0x40
    > [<ffffffff8119dc3c>] ? vm_mmap_pgoff+0x9c/0xc0
    > [<ffffffff8133e1ea>] ? trace_hardirqs_off_thunk+0x3a/0x3c
    > [<ffffffff81063112>] do_page_fault+0x22/0x30
    > [<ffffffff815d18b8>] page_fault+0x28/0x30
    >
    --
    Jan Kara <jack@suse.cz>
    SUSE Labs, CR
    From c01c905cf3c4c6304a5ea9836389d9cf0d575884 Mon Sep 17 00:00:00 2001
    From: Jan Kara <jack@suse.cz>
    Date: Wed, 30 Jul 2014 11:49:07 +0200
    Subject: [PATCH] ext4: Avoid lock inversion between i_mmap_mutex and
    transaction start

    When DAX is enabled, it uses i_mmap_mutex as a protection against
    truncate during page fault. This inevitably forces i_mmap_mutex to rank
    outside of a transaction start and thus we have to avoid calling
    pagecache purging operations when transaction is started.

    Signed-off-by: Jan Kara <jack@suse.cz>
    ---
    fs/ext4/inode.c | 14 ++++++++++----
    1 file changed, 10 insertions(+), 4 deletions(-)

    diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
    index 8a064734e6eb..494a8645d63e 100644
    --- a/fs/ext4/inode.c
    +++ b/fs/ext4/inode.c
    @@ -3631,13 +3631,19 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
    if (IS_SYNC(inode))
    ext4_handle_sync(handle);

    - /* Now release the pages again to reduce race window */
    + inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
    + ext4_mark_inode_dirty(handle, inode);
    + ext4_journal_stop(handle);
    +
    + /*
    + * Now release the pages again to reduce race window. This has to happen
    + * outside of a transaction to avoid lock inversion on i_mmap_mutex
    + * when DAX is enabled.
    + */
    if (last_block_offset > first_block_offset)
    truncate_pagecache_range(inode, first_block_offset,
    last_block_offset);
    -
    - inode->i_mtime = inode->i_ctime = ext4_current_time(inode);
    - ext4_mark_inode_dirty(handle, inode);
    + goto out_dio;
    out_stop:
    ext4_journal_stop(handle);
    out_dio:
    --
    1.8.1.4
    \
     
     \ /
      Last update: 2014-07-30 12:41    [W:5.162 / U:0.236 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site