Bug 146296 - [zfs] [patch] fix deadlock during zfs receive (onnv 9299)
Summary: [zfs] [patch] fix deadlock during zfs receive (onnv 9299)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.0-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Martin Matuska
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-04 11:50 UTC by Martin Matuska
Modified: 2010-05-11 08:28 UTC (History)
1 user (show)

See Also:

Attachments
file.diff (2.21 KB, patch)
2010-05-04 11:50 UTC, Martin Matuska 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Matuska freebsd_committer freebsd_triage 2010-05-04 11:50:01 UTC 
I have encountered a hanging zfs receive during receiving many incremental
streams.

This problem has been described in OpenSolaris mailing lists,
it matches my symptoms and affects our ZFS port.

OpenSolaris Bug IDs:
	6783818 Incremental stream receive panics system
	6826836 Deadlock possible in dmu_object_reclaim()

Mailing list discussion:
http://mail.opensolaris.org/pipermail/storage-discuss/2009-June/006171.html

Fixed in onnv revision: 9299:8809e849f63e
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-05-04 16:58:45 UTC 
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 2 dfilter service freebsd_committer freebsd_triage 2010-05-04 18:30:27 UTC 
Author: mm
Date: Tue May  4 17:30:07 2010
New Revision: 207624
URL: http://svn.freebsd.org/changeset/base/207624

Log:
  Fix deadlock during zfs receive.
  
  OpenSolaris onnv revision:	9299:8809e849f63e
  
  PR:		kern/146296
  Submitted by:	myself
  Approved by:	pjd, delphij (mentor)
  Obtained from:	OpenSolaris (Bug ID 6783818, 6826836)
  MFC after:	1 week

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May  4 17:12:36 2010	(r207623)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May  4 17:30:07 2010	(r207624)
@@ -464,15 +464,15 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t
 	ASSERT(db->db_buf == NULL);
 
 	if (db->db_blkid == DB_BONUS_BLKID) {
-		int bonuslen = dn->dn_bonuslen;
+		int bonuslen = MIN(dn->dn_bonuslen, dn->dn_phys->dn_bonuslen);
 
 		ASSERT3U(bonuslen, <=, db->db.db_size);
 		db->db.db_data = zio_buf_alloc(DN_MAX_BONUSLEN);
 		arc_space_consume(DN_MAX_BONUSLEN);
 		if (bonuslen < DN_MAX_BONUSLEN)
 			bzero(db->db.db_data, DN_MAX_BONUSLEN);
-		bcopy(DN_BONUS(dn->dn_phys), db->db.db_data,
-		    bonuslen);
+		if (bonuslen)
+			bcopy(DN_BONUS(dn->dn_phys), db->db.db_data, bonuslen);
 		dbuf_update_data(db);
 		db->db_state = DB_CACHED;
 		mutex_exit(&db->db_mtx);

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May  4 17:12:36 2010	(r207623)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May  4 17:30:07 2010	(r207624)
@@ -128,15 +128,6 @@ dmu_object_reclaim(objset_t *os, uint64_
 		return (0);
 	}
 
-	tx = dmu_tx_create(os);
-	dmu_tx_hold_bonus(tx, object);
-	err = dmu_tx_assign(tx, TXG_WAIT);
-	if (err) {
-		dmu_tx_abort(tx);
-		dnode_rele(dn, FTAG);
-		return (err);
-	}
-
 	nblkptr = 1 + ((DN_MAX_BONUSLEN - bonuslen) >> SPA_BLKPTRSHIFT);
 
 	/*
@@ -144,16 +135,27 @@ dmu_object_reclaim(objset_t *os, uint64_
 	 * be a new file instance.   We must clear out the previous file
 	 * contents before we can change this type of metadata in the dnode.
 	 */
-	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize)
-		dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize) {
+		err = dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+		if (err)
+			goto out;
+	}
+
+	tx = dmu_tx_create(os);
+	dmu_tx_hold_bonus(tx, object);
+	err = dmu_tx_assign(tx, TXG_WAIT);
+	if (err) {
+		dmu_tx_abort(tx);
+		goto out;
+	}
 
 	dnode_reallocate(dn, ot, blocksize, bonustype, bonuslen, tx);
 
 	dmu_tx_commit(tx);
-
+out:
 	dnode_rele(dn, FTAG);
 
-	return (0);
+	return (err);
 }
 
 int
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 3 Martin Matuska freebsd_committer freebsd_triage 2010-05-04 22:08:22 UTC 
State Changed
From-To: open->patched

Scheduled for MFC
Comment 4 Martin Matuska freebsd_committer freebsd_triage 2010-05-04 22:09:08 UTC 
Responsible Changed
From-To: freebsd-fs->mm

Take PR.
Comment 5 dfilter service freebsd_committer freebsd_triage 2010-05-11 08:02:39 UTC 
Author: mm
Date: Tue May 11 07:02:29 2010
New Revision: 207906
URL: http://svn.freebsd.org/changeset/base/207906

Log:
  MFC r207624:
  
  Fix deadlock during zfs receive.
  
  OpenSolaris onnv revision:	9299:8809e849f63e
  
  PR:		kern/146296
  Approved by:	pjd, delphij (mentor)
  Obtained from:	OpenSolaris (Bug ID 6783818, 6826836)

Modified:
  stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
  stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)
  stable/8/sys/geom/sched/   (props changed)

Modified: stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
==============================================================================
--- stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May 11 01:29:18 2010	(r207905)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May 11 07:02:29 2010	(r207906)
@@ -464,15 +464,15 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t
 	ASSERT(db->db_buf == NULL);
 
 	if (db->db_blkid == DB_BONUS_BLKID) {
-		int bonuslen = dn->dn_bonuslen;
+		int bonuslen = MIN(dn->dn_bonuslen, dn->dn_phys->dn_bonuslen);
 
 		ASSERT3U(bonuslen, <=, db->db.db_size);
 		db->db.db_data = zio_buf_alloc(DN_MAX_BONUSLEN);
 		arc_space_consume(DN_MAX_BONUSLEN);
 		if (bonuslen < DN_MAX_BONUSLEN)
 			bzero(db->db.db_data, DN_MAX_BONUSLEN);
-		bcopy(DN_BONUS(dn->dn_phys), db->db.db_data,
-		    bonuslen);
+		if (bonuslen)
+			bcopy(DN_BONUS(dn->dn_phys), db->db.db_data, bonuslen);
 		dbuf_update_data(db);
 		db->db_state = DB_CACHED;
 		mutex_exit(&db->db_mtx);

Modified: stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c
==============================================================================
--- stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May 11 01:29:18 2010	(r207905)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May 11 07:02:29 2010	(r207906)
@@ -128,15 +128,6 @@ dmu_object_reclaim(objset_t *os, uint64_
 		return (0);
 	}
 
-	tx = dmu_tx_create(os);
-	dmu_tx_hold_bonus(tx, object);
-	err = dmu_tx_assign(tx, TXG_WAIT);
-	if (err) {
-		dmu_tx_abort(tx);
-		dnode_rele(dn, FTAG);
-		return (err);
-	}
-
 	nblkptr = 1 + ((DN_MAX_BONUSLEN - bonuslen) >> SPA_BLKPTRSHIFT);
 
 	/*
@@ -144,16 +135,27 @@ dmu_object_reclaim(objset_t *os, uint64_
 	 * be a new file instance.   We must clear out the previous file
 	 * contents before we can change this type of metadata in the dnode.
 	 */
-	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize)
-		dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize) {
+		err = dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+		if (err)
+			goto out;
+	}
+
+	tx = dmu_tx_create(os);
+	dmu_tx_hold_bonus(tx, object);
+	err = dmu_tx_assign(tx, TXG_WAIT);
+	if (err) {
+		dmu_tx_abort(tx);
+		goto out;
+	}
 
 	dnode_reallocate(dn, ot, blocksize, bonustype, bonuslen, tx);
 
 	dmu_tx_commit(tx);
-
+out:
 	dnode_rele(dn, FTAG);
 
-	return (0);
+	return (err);
 }
 
 int
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 dfilter service freebsd_committer freebsd_triage 2010-05-11 08:08:03 UTC 
Author: mm
Date: Tue May 11 07:07:44 2010
New Revision: 207907
URL: http://svn.freebsd.org/changeset/base/207907

Log:
  MFC r207624:
  
  Fix deadlock during zfs receive.
  
  OpenSolaris onnv revision:	9299:8809e849f63e
  
  PR:		kern/146296
  Approved by:	pjd, delphij (mentor)
  Obtained from:	OpenSolaris (Bug ID 6783818, 6826836)

Modified:
  stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
  stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
==============================================================================
--- stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May 11 07:02:29 2010	(r207906)
+++ stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Tue May 11 07:07:44 2010	(r207907)
@@ -464,15 +464,15 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t
 	ASSERT(db->db_buf == NULL);
 
 	if (db->db_blkid == DB_BONUS_BLKID) {
-		int bonuslen = dn->dn_bonuslen;
+		int bonuslen = MIN(dn->dn_bonuslen, dn->dn_phys->dn_bonuslen);
 
 		ASSERT3U(bonuslen, <=, db->db.db_size);
 		db->db.db_data = zio_buf_alloc(DN_MAX_BONUSLEN);
 		arc_space_consume(DN_MAX_BONUSLEN);
 		if (bonuslen < DN_MAX_BONUSLEN)
 			bzero(db->db.db_data, DN_MAX_BONUSLEN);
-		bcopy(DN_BONUS(dn->dn_phys), db->db.db_data,
-		    bonuslen);
+		if (bonuslen)
+			bcopy(DN_BONUS(dn->dn_phys), db->db.db_data, bonuslen);
 		dbuf_update_data(db);
 		db->db_state = DB_CACHED;
 		mutex_exit(&db->db_mtx);

Modified: stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c
==============================================================================
--- stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May 11 07:02:29 2010	(r207906)
+++ stable/7/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Tue May 11 07:07:44 2010	(r207907)
@@ -128,15 +128,6 @@ dmu_object_reclaim(objset_t *os, uint64_
 		return (0);
 	}
 
-	tx = dmu_tx_create(os);
-	dmu_tx_hold_bonus(tx, object);
-	err = dmu_tx_assign(tx, TXG_WAIT);
-	if (err) {
-		dmu_tx_abort(tx);
-		dnode_rele(dn, FTAG);
-		return (err);
-	}
-
 	nblkptr = 1 + ((DN_MAX_BONUSLEN - bonuslen) >> SPA_BLKPTRSHIFT);
 
 	/*
@@ -144,16 +135,27 @@ dmu_object_reclaim(objset_t *os, uint64_
 	 * be a new file instance.   We must clear out the previous file
 	 * contents before we can change this type of metadata in the dnode.
 	 */
-	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize)
-		dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+	if (dn->dn_nblkptr > nblkptr || dn->dn_datablksz != blocksize) {
+		err = dmu_free_long_range(os, object, 0, DMU_OBJECT_END);
+		if (err)
+			goto out;
+	}
+
+	tx = dmu_tx_create(os);
+	dmu_tx_hold_bonus(tx, object);
+	err = dmu_tx_assign(tx, TXG_WAIT);
+	if (err) {
+		dmu_tx_abort(tx);
+		goto out;
+	}
 
 	dnode_reallocate(dn, ot, blocksize, bonustype, bonuslen, tx);
 
 	dmu_tx_commit(tx);
-
+out:
 	dnode_rele(dn, FTAG);
 
-	return (0);
+	return (err);
 }
 
 int
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 Martin Matuska freebsd_committer freebsd_triage 2010-05-11 08:28:43 UTC 
State Changed
From-To: patched->closed

Committed. Thanks!