Commit e21e5744 authored by Larry Chen's avatar Larry Chen Committed by Linus Torvalds
Browse files

ocfs2: fix deadlock caused by ocfs2_defrag_extent() 

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: default avatarLarry Chen <lchen@suse.com>
Reviewed-by: default avatarChangwei 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: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 08be37b7

fs/ocfs2/move_extents.c

+26 −21
Original line number Diff line number Diff line
@@ -157,18 +157,14 @@ out: 
}



/*

 * 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)

{
@@ -193,13 +189,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);
@@ -259,9 +248,9 @@ static int ocfs2_defrag_extent(struct ocfs2_move_extents_context *context, 
		}

	}



	ret = ocfs2_lock_allocators_move_extents(inode, &context->et, *len, 1,

	ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,

						*len, 1,

						&context->meta_ac,

						 &context->data_ac,

						extra_blocks, &credits);

	if (ret) {

		mlog_errno(ret);
@@ -285,6 +274,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);
@@ -617,9 +621,10 @@ static int ocfs2_move_extent(struct ocfs2_move_extents_context *context, 
		}

	}



	ret = ocfs2_lock_allocators_move_extents(inode, &context->et, len, 1,

	ret = ocfs2_lock_meta_allocator_move_extents(inode, &context->et,

						len, 1,

						&context->meta_ac,

						 NULL, extra_blocks, &credits);

						extra_blocks, &credits);

	if (ret) {

		mlog_errno(ret);

		goto out;

CRA Git | Maintained and supported by SUSTech CRA and CCSE