Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jul 2014 08:36:56 +0000 (UTC)
From:      Xin LI <delphij@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r268086 - in head: cddl/contrib/opensolaris/cmd/ztest sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201407010836.s618auuC042698@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: delphij
Date: Tue Jul  1 08:36:56 2014
New Revision: 268086
URL: http://svnweb.freebsd.org/changeset/base/268086

Log:
  MFV r267570:
  
  4756 metaslab_group_preload() could deadlock
  
  illumos/illumos-gate@30beaff42d8240ebf5386e8b7a14e3d137a1631f
  
  MFC after:	2 weeks

Modified:
  head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/metaslab.c
Directory Properties:
  head/cddl/contrib/opensolaris/   (props changed)
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/ztest/ztest.c	Tue Jul  1 08:28:49 2014	(r268085)
+++ head/cddl/contrib/opensolaris/cmd/ztest/ztest.c	Tue Jul  1 08:36:56 2014	(r268086)
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2013 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2014 by Delphix. All rights reserved.
  * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2012 Martin Matuska <mm@FreeBSD.org>.  All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
@@ -187,6 +187,7 @@ static const ztest_shared_opts_t ztest_o
 extern uint64_t metaslab_gang_bang;
 extern uint64_t metaslab_df_alloc_threshold;
 extern uint64_t zfs_deadman_synctime_ms;
+extern int metaslab_preload_limit;
 
 static ztest_shared_opts_t *ztest_shared_opts;
 static ztest_shared_opts_t ztest_opts;
@@ -5596,6 +5597,7 @@ ztest_run(ztest_shared_t *zs)
 	kernel_init(FREAD | FWRITE);
 	VERIFY0(spa_open(ztest_opts.zo_pool, &spa, FTAG));
 	spa->spa_debug = B_TRUE;
+	metaslab_preload_limit = ztest_random(20) + 1;
 	ztest_spa = spa;
 
 	VERIFY0(dmu_objset_own(ztest_opts.zo_pool,

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/metaslab.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/metaslab.c	Tue Jul  1 08:28:49 2014	(r268085)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/metaslab.c	Tue Jul  1 08:36:56 2014	(r268086)
@@ -1284,6 +1284,8 @@ metaslab_preload(void *arg)
 	metaslab_t *msp = arg;
 	spa_t *spa = msp->ms_group->mg_vd->vdev_spa;
 
+	ASSERT(!MUTEX_HELD(&msp->ms_group->mg_lock));
+
 	mutex_enter(&msp->ms_lock);
 	metaslab_load_wait(msp);
 	if (!msp->ms_loaded)
@@ -1308,19 +1310,36 @@ metaslab_group_preload(metaslab_group_t 
 		taskq_wait(mg->mg_taskq);
 		return;
 	}
-	mutex_enter(&mg->mg_lock);
 
+	mutex_enter(&mg->mg_lock);
 	/*
-	 * Prefetch the next potential metaslabs
+	 * Load the next potential metaslabs
 	 */
-	for (msp = avl_first(t); msp != NULL; msp = AVL_NEXT(t, msp)) {
+	msp = avl_first(t);
+	while (msp != NULL) {
+		metaslab_t *msp_next = AVL_NEXT(t, msp);
 
 		/* If we have reached our preload limit then we're done */
 		if (++m > metaslab_preload_limit)
 			break;
 
+		/*
+		 * We must drop the metaslab group lock here to preserve
+		 * lock ordering with the ms_lock (when grabbing both
+		 * the mg_lock and the ms_lock, the ms_lock must be taken
+		 * first).  As a result, it is possible that the ordering
+		 * of the metaslabs within the avl tree may change before
+		 * we reacquire the lock. The metaslab cannot be removed from
+		 * the tree while we're in syncing context so it is safe to
+		 * drop the mg_lock here. If the metaslabs are reordered
+		 * nothing will break -- we just may end up loading a
+		 * less than optimal one.
+		 */
+		mutex_exit(&mg->mg_lock);
 		VERIFY(taskq_dispatch(mg->mg_taskq, metaslab_preload,
 		    msp, TQ_SLEEP) != 0);
+		mutex_enter(&mg->mg_lock);
+		msp = msp_next;
 	}
 	mutex_exit(&mg->mg_lock);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407010836.s618auuC042698>