lkml.org 
[lkml]   [2018]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 3.18 22/26] ocfs2: fix deadlock caused by ocfs2_defrag_extent()
    Date
    From: Larry Chen <lchen@suse.com>

    [ Upstream commit e21e57445a64598b29a6f629688f9b9a39e7242a ]

    ocfs2_defrag_extent may fall into deadlock.

    ocfs2_ioctl_move_extents
    ocfs2_ioctl_move_extents
    ocfs2_move_extents
    ocfs2_defrag_extent
    ocfs2_lock_allocators_move_extents

    ocfs2_reserve_clusters
    inode_lock GLOBAL_BITMAP_SYSTEM_INODE

    __ocfs2_flush_truncate_log
    inode_lock GLOBAL_BITMAP_SYSTEM_INODE

    As backtrace shows above, ocfs2_reserve_clusters() will call inode_lock
    against the global bitmap if local allocator has not sufficient cluters.
    Once global bitmap could meet the demand, ocfs2_reserve_cluster will
    return success with global bitmap locked.

    After ocfs2_reserve_cluster(), if truncate log is full,
    __ocfs2_flush_truncate_log() will definitely fall into deadlock because
    it needs to inode_lock global bitmap, which has already been locked.

    To fix this bug, we could remove from
    ocfs2_lock_allocators_move_extents() the code which intends to lock
    global allocator, and put the removed code after
    __ocfs2_flush_truncate_log().

    ocfs2_lock_allocators_move_extents() is referred by 2 places, one is
    here, the other does not need the data allocator context, which means
    this patch does not affect the caller so far.

    Link: http://lkml.kernel.org/r/20181101071422.14470-1-lchen@suse.com
    Signed-off-by: Larry Chen <lchen@suse.com>
    Reviewed-by: Changwei Ge <ge.changwei@h3c.com>
    Cc: Mark Fasheh <mark@fasheh.com>
    Cc: Joel Becker <jlbec@evilplan.org>
    Cc: Junxiao Bi <junxiao.bi@oracle.com>
    Cc: Joseph Qi <jiangqi903@gmail.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    fs/ocfs2/move_extents.c | 47 +++++++++++++++++++++++------------------
    1 file changed, 26 insertions(+), 21 deletions(-)

    diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
    index 74caffeeee1d..51b7ab554208 100644
    --- a/fs/ocfs2/move_extents.c
    +++ b/fs/ocfs2/move_extents.c
    @@ -158,18 +158,14 @@ static int __ocfs2_move_extent(handle_t *handle,
    }

    /*
    - * lock allocators, and reserving appropriate number of bits for
    - * meta blocks and data clusters.
    - *
    - * in some cases, we don't need to reserve clusters, just let data_ac
    - * be NULL.
    + * lock allocator, and reserve appropriate number of bits for
    + * meta blocks.
    */
    -static int ocfs2_lock_allocators_move_extents(struct inode *inode,
    +static int ocfs2_lock_meta_allocator_move_extents(struct inode *inode,
    struct ocfs2_extent_tree *et,
    u32 clusters_to_move,
    u32 extents_to_split,
    struct ocfs2_alloc_context **meta_ac,
    - struct ocfs2_alloc_context **data_ac,
    int extra_blocks,
    int *credits)
    {
    @@ -194,13 +190,6 @@ static int ocfs2_lock_allocators_move_extents(struct inode *inode,
    goto out;
    }

    - if (data_ac) {
    - ret = ocfs2_reserve_clusters(osb, clusters_to_move, data_ac);
    - if (ret) {
    - mlog_errno(ret);
    - goto out;
    - }
    - }

    *credits += ocfs2_calc_extend_credits(osb->sb, et->et_root_el);

    @@ -262,10 +251,10 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
    }
    }

    - ret = ocfs2_lock_allocators_move_extents(inode, &context->et, *len, 1,
    - &context->meta_ac,
    - &context->data_ac,
    - extra_blocks, &credits);
    + ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,
    + *len, 1,
    + &context->meta_ac,
    + extra_blocks, &credits);
    if (ret) {
    mlog_errno(ret);
    goto out;
    @@ -288,6 +277,21 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context,
    }
    }

    + /*
    + * Make sure ocfs2_reserve_cluster is called after
    + * __ocfs2_flush_truncate_log, otherwise, dead lock may happen.
    + *
    + * If ocfs2_reserve_cluster is called
    + * before __ocfs2_flush_truncate_log, dead lock on global bitmap
    + * may happen.
    + *
    + */
    + ret = ocfs2_reserve_clusters(osb, *len, &context->data_ac);
    + if (ret) {
    + mlog_errno(ret);
    + goto out_unlock_mutex;
    + }
    +
    handle = ocfs2_start_trans(osb, credits);
    if (IS_ERR(handle)) {
    ret = PTR_ERR(handle);
    @@ -608,9 +612,10 @@ static int ocfs2_move_extent(struct ocfs2_move_extents_context *context,
    }
    }

    - ret = ocfs2_lock_allocators_move_extents(inode, &context->et, len, 1,
    - &context->meta_ac,
    - NULL, extra_blocks, &credits);
    + ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,
    + len, 1,
    + &context->meta_ac,
    + extra_blocks, &credits);
    if (ret) {
    mlog_errno(ret);
    goto out;
    --
    2.17.1
    \
     
     \ /
      Last update: 2018-12-05 10:57    [W:3.767 / U:0.388 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site