• kaixuxia's avatar
    xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT · 6fb102dd
    kaixuxia authored
    commit bc56ad8c upstream.
    
    When performing rename operation with RENAME_WHITEOUT flag, we will
    hold AGF lock to allocate or free extents in manipulating the dirents
    firstly, and then doing the xfs_iunlink_remove() call last to hold
    AGI lock to modify the tmpfile info, so we the lock order AGI->AGF.
    
    The big problem here is that we have an ordering constraint on AGF
    and AGI locking - inode allocation locks the AGI, then can allocate
    a new extent for new inodes, locking the AGF after the AGI. Hence
    the ordering that is imposed by other parts of the code is AGI before
    AGF. So we get an ABBA deadlock between the AGI and AGF here.
    
    Process A:
    Call trace:
     ? __schedule+0x2bd/0x620
     schedule+0x33/0x90
     schedule_timeout+0x17d/0x290
     __down_common+0xef/0x125
     ? xfs_buf_find+0x215/0x6c0 [xfs]
     down+0x3b/0x50
     xfs_buf_lock+0x34/0xf0 [xfs]
     xfs_buf_find+0x215/0x6c0 [xfs]
     xfs_buf_get_map+0x37/0x230 [xfs]
     xfs_buf_read_map+0x29/0x190 [xfs]
     xfs_trans_read_buf_map+0x13d/0x520 [xfs]
     xfs_read_agf+0xa6/0x180 [xfs]
     ? schedule_timeout+0x17d/0x290
     xfs_alloc_read_agf+0x52/0x1f0 [xfs]
     xfs_alloc_fix_freelist+0x432/0x590 [xfs]
     ? down+0x3b/0x50
     ? xfs_buf_lock+0x34/0xf0 [xfs]
     ? xfs_buf_find+0x215/0x6c0 [xfs]
     xfs_alloc_vextent+0x301/0x6c0 [xfs]
     xfs_ialloc_ag_alloc+0x182/0x700 [xfs]
     ? _xfs_trans_bjoin+0x72/0xf0 [xfs]
     xfs_dialloc+0x116/0x290 [xfs]
     xfs_ialloc+0x6d/0x5e0 [xfs]
     ? xfs_log_reserve+0x165/0x280 [xfs]
     xfs_dir_ialloc+0x8c/0x240 [xfs]
     xfs_create+0x35a/0x610 [xfs]
     xfs_generic_create+0x1f1/0x2f0 [xfs]
     ...
    
    Process B:
    Call trace:
     ? __schedule+0x2bd/0x620
     ? xfs_bmapi_allocate+0x245/0x380 [xfs]
     schedule+0x33/0x90
     schedule_timeout+0x17d/0x290
     ? xfs_buf_find+0x1fd/0x6c0 [xfs]
     __down_common+0xef/0x125
     ? xfs_buf_get_map+0x37/0x230 [xfs]
     ? xfs_buf_find+0x215/0x6c0 [xfs]
     down+0x3b/0x50
     xfs_buf_lock+0x34/0xf0 [xfs]
     xfs_buf_find+0x215/0x6c0 [xfs]
     xfs_buf_get_map+0x37/0x230 [xfs]
     xfs_buf_read_map+0x29/0x190 [xfs]
     xfs_trans_read_buf_map+0x13d/0x520 [xfs]
     xfs_read_agi+0xa8/0x160 [xfs]
     xfs_iunlink_remove+0x6f/0x2a0 [xfs]
     ? current_time+0x46/0x80
     ? xfs_trans_ichgtime+0x39/0xb0 [xfs]
     xfs_rename+0x57a/0xae0 [xfs]
     xfs_vn_rename+0xe4/0x150 [xfs]
     ...
    
    In this patch we move the xfs_iunlink_remove() call to
    before acquiring the AGF lock to preserve correct AGI/AGF locking
    order.
    
    [Minor massage required due to upstream change making xfs_bumplink() a
    void function where as in the 4.19.y tree the return value is checked,
    even though it is always zero. Only change was to the last code block
    removed by the patch. Functionally equivalent to upstream.]
    Signed-off-by: default avatarkaixuxia <kaixuxia@tencent.com>
    Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
    Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: default avatarSuraj Jitindar Singh <surajjs@amazon.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    6fb102dd
xfs_inode.c 97.5 KB