Bug 141355 - [zfs] [patch] zfs recv can fail with E2BIG
Summary: [zfs] [patch] zfs recv can fail with E2BIG
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: Xin LI
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-10 21:50 UTC by Martin Matuska
Modified: 2010-01-03 03:10 UTC (History)
0 users

See Also:


Attachments
sys.patch (8.37 KB, patch)
2009-12-11 09:08 UTC, Martin Matuska
no flags Details | Diff
dmu_send.c.diff (1.22 KB, patch)
2009-12-13 16:42 UTC, Martin Matuska
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Matuska freebsd_committer freebsd_triage 2009-12-10 21:50:03 UTC
There is a bug in ZFS send/receive that was already fixed in OpenSolaris snv_111

References:
Opensolaris Bug ID: 6801979
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6801979

Fix: 

This bug was fixed in snv_111, right after the dirtying snapshot problem:
http://dlc.sun.com/osol/on/downloads/b111/on-changelog-b111.html

Issues Resolved:
BUG/RFE:6801979 zfs recv can fail with E2BIG
Files Changed: (revision 8986:45c289aff7c9)
update:usr/src/uts/common/fs/zfs/dmu_object.c
update:usr/src/uts/common/fs/zfs/dmu_send.c
update:usr/src/uts/common/fs/zfs/dnode.c
update:usr/src/uts/common/fs/zfs/sys/dmu.h

Should be easy to implement.
How-To-Repeat: This may happen during ZFS send/receive (managed to reproduce it):
internal error: Argument list too long
Abort (core dumped)
warning: cannot send 'pool/home@s2': Broken pipe
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2009-12-10 22:51:00 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 2 Martin Matuska freebsd_committer freebsd_triage 2009-12-11 09:08:43 UTC
I have created a patch from OpenSolaris mercurial for head.

Diffs used:

hg clone ssh://anon@hg.opensolaris.org/hg/onnv/onnv-gate
hg diff -r 7837 -r 7994 onnv-gate/usr/src/uts/common/fs/zfs/dmu_send.c
hg diff -c 8986 onnv-gate

Patch is easily backportable to RELENG_8. I am testing it in RELENG_8.
Comment 3 Martin Matuska freebsd_committer freebsd_triage 2009-12-13 16:42:06 UTC
Your patch does not include the changes in dmu_send.c between revisions
7837 and 7994, that seems to be in direct relation with this bugfix:

hg clone ssh://anon@hg.opensolaris.org/hg/onnv/onnv-gate
hg diff -r 7837 -r 7994 onnv-gate/usr/src/uts/common/fs/zfs/dmu_send.c


I am now running for some days with applied patch of dmu_send.c
7837-7994 and total changeset of 8989 and have no panics - on the
contrary, everything works well.

The opensolaris diff output of dmu_send.c between revisions 7837 and
7994 is attached.
Comment 4 Borja Marcos 2009-12-16 09:54:09 UTC
I reported a panic with Pawel's patch for this issue, and I'm afraid I =
cannot reproduce it now.=20

As I had been doing lots of tests in order to reproduce a deadlock =
situation, maybe the virtual machine's ZFS pool had some damage, I =
cannot find out I'm afraid.

I will try this alternative patch and see how it goes.

Sorry for the maybe silly panic report.




Borja.
Comment 5 Xin LI freebsd_committer freebsd_triage 2009-12-19 11:54:04 UTC
State Changed
From-To: feedback->patched

Patch was applied as revision 200726 (requested by mm@). 

MFC reminder. 


Comment 6 Xin LI freebsd_committer freebsd_triage 2009-12-19 11:54:04 UTC
Responsible Changed
From-To: pjd->delphij

All bugs are mine (you touch it and you buy it :)
Comment 7 Xin LI freebsd_committer freebsd_triage 2010-01-03 03:05:36 UTC
State Changed
From-To: patched->closed

Patch applied against 8-STABLE, thanks for submitting!
Comment 8 dfilter service freebsd_committer freebsd_triage 2010-01-03 03:05:40 UTC
Author: delphij
Date: Sun Jan  3 03:05:30 2010
New Revision: 201411
URL: http://svn.freebsd.org/changeset/base/201411

Log:
  MFC r200726:
  
  Apply fix for Solaris bug 6801979: zfs recv can fail with E2BIG
  (onnv revision 8986)
  
  PR:		kern/141355
  Requested by:	mm
  Submitted by:	pjd
  Obtained from:	OpenSolaris

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_send.c
  stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
  stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h
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)

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	Sun Jan  3 02:58:43 2010	(r201410)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_object.c	Sun Jan  3 03:05:30 2010	(r201411)
@@ -19,12 +19,10 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <sys/dmu.h>
 #include <sys/dmu_objset.h>
 #include <sys/dmu_tx.h>
@@ -108,19 +106,51 @@ dmu_object_claim(objset_t *os, uint64_t 
 
 int
 dmu_object_reclaim(objset_t *os, uint64_t object, dmu_object_type_t ot,
-    int blocksize, dmu_object_type_t bonustype, int bonuslen, dmu_tx_t *tx)
+    int blocksize, dmu_object_type_t bonustype, int bonuslen)
 {
 	dnode_t *dn;
+	dmu_tx_t *tx;
+	int nblkptr;
 	int err;
 
-	if (object == DMU_META_DNODE_OBJECT && !dmu_tx_private_ok(tx))
+	if (object == DMU_META_DNODE_OBJECT)
 		return (EBADF);
 
 	err = dnode_hold_impl(os->os, object, DNODE_MUST_BE_ALLOCATED,
 	    FTAG, &dn);
 	if (err)
 		return (err);
+
+	if (dn->dn_type == ot && dn->dn_datablksz == blocksize &&
+	    dn->dn_bonustype == bonustype && dn->dn_bonuslen == bonuslen) {
+		/* nothing is changing, this is a noop */
+		dnode_rele(dn, FTAG);
+		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);
+
+	/*
+	 * If we are losing blkptrs or changing the block size this must
+	 * 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);
+
 	dnode_reallocate(dn, ot, blocksize, bonustype, bonuslen, tx);
+
+	dmu_tx_commit(tx);
+
 	dnode_rele(dn, FTAG);
 
 	return (0);

Modified: stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c
==============================================================================
--- stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c	Sun Jan  3 02:58:43 2010	(r201410)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c	Sun Jan  3 03:05:30 2010	(r201411)
@@ -829,11 +829,6 @@ restore_object(struct restorearg *ra, ob
 	int err;
 	dmu_tx_t *tx;
 
-	err = dmu_object_info(os, drro->drr_object, NULL);
-
-	if (err != 0 && err != ENOENT)
-		return (EINVAL);
-
 	if (drro->drr_type == DMU_OT_NONE ||
 	    drro->drr_type >= DMU_OT_NUMTYPES ||
 	    drro->drr_bonustype >= DMU_OT_NUMTYPES ||
@@ -846,12 +841,15 @@ restore_object(struct restorearg *ra, ob
 		return (EINVAL);
 	}
 
-	tx = dmu_tx_create(os);
+	err = dmu_object_info(os, drro->drr_object, NULL);
+
+	if (err != 0 && err != ENOENT)
+		return (EINVAL);
 
 	if (err == ENOENT) {
 		/* currently free, want to be allocated */
+		tx = dmu_tx_create(os);
 		dmu_tx_hold_bonus(tx, DMU_NEW_OBJECT);
-		dmu_tx_hold_write(tx, DMU_NEW_OBJECT, 0, 1);
 		err = dmu_tx_assign(tx, TXG_WAIT);
 		if (err) {
 			dmu_tx_abort(tx);
@@ -860,28 +858,23 @@ restore_object(struct restorearg *ra, ob
 		err = dmu_object_claim(os, drro->drr_object,
 		    drro->drr_type, drro->drr_blksz,
 		    drro->drr_bonustype, drro->drr_bonuslen, tx);
+		dmu_tx_commit(tx);
 	} else {
 		/* currently allocated, want to be allocated */
-		dmu_tx_hold_bonus(tx, drro->drr_object);
-		/*
-		 * We may change blocksize and delete old content,
-		 * so need to hold_write and hold_free.
-		 */
-		dmu_tx_hold_write(tx, drro->drr_object, 0, 1);
-		dmu_tx_hold_free(tx, drro->drr_object, 0, DMU_OBJECT_END);
-		err = dmu_tx_assign(tx, TXG_WAIT);
-		if (err) {
-			dmu_tx_abort(tx);
-			return (err);
-		}
 
 		err = dmu_object_reclaim(os, drro->drr_object,
 		    drro->drr_type, drro->drr_blksz,
-		    drro->drr_bonustype, drro->drr_bonuslen, tx);
+		    drro->drr_bonustype, drro->drr_bonuslen);
 	}
-	if (err) {
-		dmu_tx_commit(tx);
+	if (err)
 		return (EINVAL);
+
+	tx = dmu_tx_create(os);
+	dmu_tx_hold_bonus(tx, drro->drr_object);
+	err = dmu_tx_assign(tx, TXG_WAIT);
+	if (err) {
+		dmu_tx_abort(tx);
+		return (err);
 	}
 
 	dmu_object_set_checksum(os, drro->drr_object, drro->drr_checksum, tx);

Modified: stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
==============================================================================
--- stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Sun Jan  3 02:58:43 2010	(r201410)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Sun Jan  3 03:05:30 2010	(r201411)
@@ -415,8 +415,7 @@ void
 dnode_reallocate(dnode_t *dn, dmu_object_type_t ot, int blocksize,
     dmu_object_type_t bonustype, int bonuslen, dmu_tx_t *tx)
 {
-	int i, nblkptr;
-	dmu_buf_impl_t *db = NULL;
+	int nblkptr;
 
 	ASSERT3U(blocksize, >=, SPA_MINBLOCKSIZE);
 	ASSERT3U(blocksize, <=, SPA_MAXBLOCKSIZE);
@@ -428,42 +427,25 @@ dnode_reallocate(dnode_t *dn, dmu_object
 	ASSERT3U(bonustype, <, DMU_OT_NUMTYPES);
 	ASSERT3U(bonuslen, <=, DN_MAX_BONUSLEN);
 
-	for (i = 0; i < TXG_SIZE; i++)
-		ASSERT(!list_link_active(&dn->dn_dirty_link[i]));
-
 	/* clean up any unreferenced dbufs */
 	dnode_evict_dbufs(dn);
-	ASSERT3P(list_head(&dn->dn_dbufs), ==, NULL);
-
-	/*
-	 * XXX I should really have a generation number to tell if we
-	 * need to do this...
-	 */
-	if (blocksize != dn->dn_datablksz ||
-	    dn->dn_bonustype != bonustype || dn->dn_bonuslen != bonuslen) {
-		/* free all old data */
-		dnode_free_range(dn, 0, -1ULL, tx);
-	}
-
-	nblkptr = 1 + ((DN_MAX_BONUSLEN - bonuslen) >> SPA_BLKPTRSHIFT);
 
-	/* change blocksize */
 	rw_enter(&dn->dn_struct_rwlock, RW_WRITER);
-	if (blocksize != dn->dn_datablksz &&
-	    (!BP_IS_HOLE(&dn->dn_phys->dn_blkptr[0]) ||
-	    list_head(&dn->dn_dbufs) != NULL)) {
-		db = dbuf_hold(dn, 0, FTAG);
-		dbuf_new_size(db, blocksize, tx);
-	}
-	dnode_setdblksz(dn, blocksize);
 	dnode_setdirty(dn, tx);
-	dn->dn_next_bonuslen[tx->tx_txg&TXG_MASK] = bonuslen;
-	dn->dn_next_blksz[tx->tx_txg&TXG_MASK] = blocksize;
+	if (dn->dn_datablksz != blocksize) {
+		/* change blocksize */
+		ASSERT(dn->dn_maxblkid == 0 &&
+		    (BP_IS_HOLE(&dn->dn_phys->dn_blkptr[0]) ||
+		    dnode_block_freed(dn, 0)));
+		dnode_setdblksz(dn, blocksize);
+		dn->dn_next_blksz[tx->tx_txg&TXG_MASK] = blocksize;
+	}
+	if (dn->dn_bonuslen != bonuslen)
+		dn->dn_next_bonuslen[tx->tx_txg&TXG_MASK] = bonuslen;
+	nblkptr = 1 + ((DN_MAX_BONUSLEN - bonuslen) >> SPA_BLKPTRSHIFT);
 	if (dn->dn_nblkptr != nblkptr)
 		dn->dn_next_nblkptr[tx->tx_txg&TXG_MASK] = nblkptr;
 	rw_exit(&dn->dn_struct_rwlock);
-	if (db)
-		dbuf_rele(db, FTAG);
 
 	/* change type */
 	dn->dn_type = ot;
@@ -1187,11 +1169,6 @@ dnode_block_freed(dnode_t *dn, uint64_t 
 	if (dn->dn_free_txg)
 		return (TRUE);
 
-	/*
-	 * If dn_datablkshift is not set, then there's only a single
-	 * block, in which case there will never be a free range so it
-	 * won't matter.
-	 */
 	range_tofind.fr_blkid = blkid;
 	mutex_enter(&dn->dn_mtx);
 	for (i = 0; i < TXG_SIZE; i++) {

Modified: stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h
==============================================================================
--- stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h	Sun Jan  3 02:58:43 2010	(r201410)
+++ stable/8/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h	Sun Jan  3 03:05:30 2010	(r201411)
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -237,7 +237,7 @@ uint64_t dmu_object_alloc(objset_t *os, 
 int dmu_object_claim(objset_t *os, uint64_t object, dmu_object_type_t ot,
     int blocksize, dmu_object_type_t bonus_type, int bonus_len, dmu_tx_t *tx);
 int dmu_object_reclaim(objset_t *os, uint64_t object, dmu_object_type_t ot,
-    int blocksize, dmu_object_type_t bonustype, int bonuslen, dmu_tx_t *tx);
+    int blocksize, dmu_object_type_t bonustype, int bonuslen);
 
 /*
  * Free an object from this objset.
_______________________________________________
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 9 Pawel Jakub Dawidek freebsd_committer freebsd_triage 2014-06-01 07:12:08 UTC
State Changed
From-To: open->feedback

Is thispatch like that one: 

http://people.freebsd.org/~pjd/patches/zfs_recv_E2BIG.patch 

There was a report that it was causing a panic. 


Comment 10 Pawel Jakub Dawidek freebsd_committer freebsd_triage 2014-06-01 07:12:08 UTC
Responsible Changed
From-To: freebsd-fs->pjd

I'll take this one.