Mercurial > illumos > illumos-gate

changeset 12620:12fcd99a642d

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6957974 assertion failed: 0 == sa_lookup(zp->z_sa_hdl, SA_ZPL_ZNODE_ACL(zp->z_zfsvfs) 6960907 zfs_rezget() has uninitialized z_mode variable
author Mark Shellenbaum <Mark.Shellenbaum@Oracle.COM>
date Mon, 14 Jun 2010 10:04:10 -0600
parents 60877912fc2d
children 67c30502d363
files usr/src/uts/common/fs/zfs/sa.c usr/src/uts/common/fs/zfs/sys/zfs_acl.h usr/src/uts/common/fs/zfs/zfs_acl.c usr/src/uts/common/fs/zfs/zfs_dir.c usr/src/uts/common/fs/zfs/zfs_ioctl.c usr/src/uts/common/fs/zfs/zfs_sa.c usr/src/uts/common/fs/zfs/zfs_vnops.c usr/src/uts/common/fs/zfs/zfs_znode.c
diffstat 8 files changed, 173 insertions(+), 70 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/fs/zfs/sa.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/sa.c	Mon Jun 14 10:04:10 2010 -0600
@@ -713,7 +713,7 @@
 			length = attr_desc[i].sa_length;
 
 		if (buf_space < length) {  /* switch to spill buffer */
-			ASSERT(bonustype != DMU_OT_ZNODE);
+			VERIFY(bonustype == DMU_OT_SA);
 			if (buftype == SA_BONUS && !sa->sa_force_spill) {
 				sa_find_layout(hdl->sa_os, hash, attrs_start,
 				    lot_count, tx, &lot);
@@ -747,6 +747,14 @@
 	}
 
 	sa_find_layout(hdl->sa_os, hash, attrs_start, lot_count, tx, &lot);
+
+	/*
+	 * Verify that old znodes always have layout number 0.
+	 * Must be DMU_OT_SA for arbitrary layouts
+	 */
+	VERIFY((bonustype == DMU_OT_ZNODE && lot->lot_num == 0) ||
+	    (bonustype == DMU_OT_SA && lot->lot_num > 1));
+
 	if (bonustype == DMU_OT_SA) {
 		SA_SET_HDR(sahdr, lot->lot_num,
 		    buftype == SA_BONUS ? hdrsize : spillhdrsize);
--- a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h	Mon Jun 14 10:04:10 2010 -0600
@@ -185,10 +185,6 @@
 	struct zfs_fuid_info 	*z_fuidp;	/* for tracking fuids for log */
 } zfs_acl_ids_t;
 
-#define	ZFS_EXTERNAL_ACL(zp) \
-	(zp->z_is_sa ? 0 : zfs_external_acl(zp))
-#define	ZNODE_ACL_VERSION(zp) \
-	(zp->z_is_sa ? ZFS_ACL_VERSION_FUID : zfs_znode_acl_version(zp))
 /*
  * Property values for acl_mode and acl_inherit.
  *
--- a/usr/src/uts/common/fs/zfs/zfs_acl.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_acl.c	Mon Jun 14 10:04:10 2010 -0600
@@ -327,19 +327,35 @@
  * an external ACL and what version of ACL previously existed on the
  * file.  Would really be nice to not need this, sigh.
  */
-
 uint64_t
 zfs_external_acl(znode_t *zp)
 {
 	zfs_acl_phys_t acl_phys;
+	int error;
 
 	if (zp->z_is_sa)
 		return (0);
 
-	VERIFY(0 == sa_lookup(zp->z_sa_hdl, SA_ZPL_ZNODE_ACL(zp->z_zfsvfs),
-	    &acl_phys, sizeof (acl_phys)));
-
-	return (acl_phys.z_acl_extern_obj);
+	/*
+	 * Need to deal with a potential
+	 * race where zfs_sa_upgrade could cause
+	 * z_isa_sa to change.
+	 *
+	 * If the lookup fails then the state of z_is_sa should have
+	 * changed.
+	 */
+
+	if ((error = sa_lookup(zp->z_sa_hdl, SA_ZPL_ZNODE_ACL(zp->z_zfsvfs),
+	    &acl_phys, sizeof (acl_phys))) == 0)
+		return (acl_phys.z_acl_extern_obj);
+	else {
+		/*
+		 * after upgrade the SA_ZPL_ZNODE_ACL should have been
+		 * removed
+		 */
+		VERIFY(zp->z_is_sa && error == ENOENT);
+		return (0);
+	}
 }
 
 /*
@@ -357,6 +373,7 @@
 	int size;
 	int error;
 
+	ASSERT(MUTEX_HELD(&zp->z_acl_lock));
 	if (zp->z_is_sa) {
 		if ((error = sa_size(zp->z_sa_hdl, SA_ZPL_DACL_ACES(zfsvfs),
 		    &size)) != 0)
@@ -387,13 +404,31 @@
 {
 	zfs_acl_phys_t acl_phys;
 
-	if (zp->z_is_sa) {
+	if (zp->z_is_sa)
 		return (ZFS_ACL_VERSION_FUID);
-	} else {
-		VERIFY(0 == sa_lookup(zp->z_sa_hdl,
+	else {
+		int error;
+
+		/*
+		 * Need to deal with a potential
+		 * race where zfs_sa_upgrade could cause
+		 * z_isa_sa to change.
+		 *
+		 * If the lookup fails then the state of z_is_sa should have
+		 * changed.
+		 */
+		if ((error = sa_lookup(zp->z_sa_hdl,
 		    SA_ZPL_ZNODE_ACL(zp->z_zfsvfs),
-		    &acl_phys, sizeof (acl_phys)));
-		return (acl_phys.z_acl_version);
+		    &acl_phys, sizeof (acl_phys))) == 0)
+			return (acl_phys.z_acl_version);
+		else {
+			/*
+			 * After upgrade SA_ZPL_ZNODE_ACL should have
+			 * been removed.
+			 */
+			VERIFY(zp->z_is_sa && error == ENOENT);
+			return (ZFS_ACL_VERSION_FUID);
+		}
 	}
 }
 
@@ -1024,7 +1059,8 @@
  * create a new acl and leave any cached acl in place.
  */
 static int
-zfs_acl_node_read(znode_t *zp, zfs_acl_t **aclpp, boolean_t will_modify)
+zfs_acl_node_read(znode_t *zp, boolean_t have_lock, zfs_acl_t **aclpp,
+    boolean_t will_modify)
 {
 	zfs_acl_t	*aclp;
 	int		aclsize;
@@ -1033,6 +1069,7 @@
 	zfs_acl_phys_t	znode_acl;
 	int		version;
 	int		error;
+	boolean_t	drop_lock = B_FALSE;
 
 	ASSERT(MUTEX_HELD(&zp->z_acl_lock));
 
@@ -1041,11 +1078,23 @@
 		return (0);
 	}
 
-	version = ZNODE_ACL_VERSION(zp);
+	/*
+	 * close race where znode could be upgrade while trying to
+	 * read the znode attributes.
+	 *
+	 * But this could only happen if the file isn't already an SA
+	 * znode
+	 */
+	if (!zp->z_is_sa && !have_lock) {
+		mutex_enter(&zp->z_lock);
+		drop_lock = B_TRUE;
+	}
+	version = zfs_znode_acl_version(zp);
 
 	if ((error = zfs_acl_znode_info(zp, &aclsize,
-	    &acl_count, &znode_acl)) != 0)
-		return (error);
+	    &acl_count, &znode_acl)) != 0) {
+		goto done;
+	}
 
 	aclp = zfs_acl_alloc(version);
 
@@ -1076,7 +1125,7 @@
 		/* convert checksum errors into IO errors */
 		if (error == ECKSUM)
 			error = EIO;
-		return (error);
+		goto done;
 	}
 
 	list_insert_head(&aclp->z_acl, aclnode);
@@ -1084,7 +1133,10 @@
 	*aclpp = aclp;
 	if (!will_modify)
 		zp->z_acl_cached = aclp;
-	return (0);
+done:
+	if (drop_lock)
+		mutex_exit(&zp->z_lock);
+	return (error);
 }
 
 /*ARGSUSED*/
@@ -1134,14 +1186,14 @@
 	zfs_acl_t *aclp;
 	uint64_t fuid, fgid;
 
+	ASSERT(MUTEX_HELD(&zp->z_lock));
+	ASSERT(MUTEX_HELD(&zp->z_acl_lock));
 	if ((error = zfs_acl_get_owner_fuids(zp, &fuid, &fgid)) != 0)
 		return (error);
 
-	mutex_enter(&zp->z_acl_lock);
-	if ((error = zfs_acl_node_read(zp, &aclp, B_FALSE)) == 0)
+	if ((error = zfs_acl_node_read(zp, B_TRUE, &aclp, B_FALSE)) == 0)
 		zp->z_mode = zfs_mode_compute(zp->z_mode, aclp,
 		    &zp->z_pflags, fuid, fgid);
-	mutex_exit(&zp->z_acl_lock);
 	return (error);
 }
 
@@ -1485,13 +1537,13 @@
 int
 zfs_acl_chmod_setattr(znode_t *zp, zfs_acl_t **aclp, uint64_t mode)
 {
+	mutex_enter(&zp->z_acl_lock);
 	mutex_enter(&zp->z_lock);
-	mutex_enter(&zp->z_acl_lock);
 	*aclp = zfs_acl_alloc(zfs_acl_version_zp(zp));
 	(*aclp)->z_hints = zp->z_pflags & V4_ACL_WIDE_FLAGS;
 	zfs_acl_chmod(zp->z_zfsvfs, mode, *aclp);
+	mutex_exit(&zp->z_lock);
 	mutex_exit(&zp->z_acl_lock);
-	mutex_exit(&zp->z_lock);
 	ASSERT(*aclp);
 	return (0);
 }
@@ -1746,15 +1798,15 @@
 	}
 
 	if (acl_ids->z_aclp == NULL) {
+		mutex_enter(&dzp->z_acl_lock);
 		mutex_enter(&dzp->z_lock);
 		if (!(flag & IS_ROOT_NODE) && (ZTOV(dzp)->v_type == VDIR &&
 		    (dzp->z_pflags & ZFS_INHERIT_ACE)) &&
 		    !(dzp->z_pflags & ZFS_XATTR)) {
-			mutex_enter(&dzp->z_acl_lock);
-			VERIFY(0 == zfs_acl_node_read(dzp, &paclp, B_FALSE));
+			VERIFY(0 == zfs_acl_node_read(dzp, B_TRUE,
+			    &paclp, B_FALSE));
 			acl_ids->z_aclp = zfs_acl_inherit(zfsvfs,
 			    vap->va_type, paclp, acl_ids->z_mode, &need_chmod);
-			mutex_exit(&dzp->z_acl_lock);
 			inherited = B_TRUE;
 		} else {
 			acl_ids->z_aclp =
@@ -1762,6 +1814,7 @@
 			acl_ids->z_aclp->z_hints |= ZFS_ACL_TRIVIAL;
 		}
 		mutex_exit(&dzp->z_lock);
+		mutex_exit(&dzp->z_acl_lock);
 		if (need_chmod) {
 			acl_ids->z_aclp->z_hints |= (vap->va_type == VDIR) ?
 			    ZFS_ACL_AUTO_INHERIT : 0;
@@ -1824,7 +1877,7 @@
 
 	mutex_enter(&zp->z_acl_lock);
 
-	error = zfs_acl_node_read(zp, &aclp, B_FALSE);
+	error = zfs_acl_node_read(zp, B_FALSE, &aclp, B_FALSE);
 	if (error != 0) {
 		mutex_exit(&zp->z_acl_lock);
 		return (error);
@@ -1970,6 +2023,7 @@
 	zfs_acl_t	*aclp;
 	zfs_fuid_info_t	*fuidp = NULL;
 	boolean_t	fuid_dirtied;
+	uint64_t	acl_obj;
 
 	if (mask == 0)
 		return (ENOSYS);
@@ -1994,8 +2048,8 @@
 		    (zp->z_pflags & V4_ACL_WIDE_FLAGS);
 	}
 top:
+	mutex_enter(&zp->z_acl_lock);
 	mutex_enter(&zp->z_lock);
-	mutex_enter(&zp->z_acl_lock);
 
 	tx = dmu_tx_create(zfsvfs->z_os);
 
@@ -2010,14 +2064,13 @@
 	 * upgrading then take out necessary DMU holds
 	 */
 
-	if (ZFS_EXTERNAL_ACL(zp)) {
+	if ((acl_obj = zfs_external_acl(zp)) != 0) {
 		if (zfsvfs->z_version <= ZPL_VERSION_SA &&
-		    ZNODE_ACL_VERSION(zp) <= ZFS_ACL_VERSION_INITIAL) {
-			dmu_tx_hold_free(tx, ZFS_EXTERNAL_ACL(zp), 0,
+		    zfs_znode_acl_version(zp) <= ZFS_ACL_VERSION_INITIAL) {
+			dmu_tx_hold_free(tx, acl_obj, 0,
 			    DMU_OBJECT_END);
 		} else {
-			dmu_tx_hold_write(tx, ZFS_EXTERNAL_ACL(zp),
-			    0, aclp->z_acl_bytes);
+			dmu_tx_hold_write(tx, acl_obj, 0, aclp->z_acl_bytes);
 		}
 	} else if (!zp->z_is_sa && aclp->z_acl_bytes > ZFS_ACE_SPACE) {
 		dmu_tx_hold_write(tx, DMU_NEW_OBJECT, 0, aclp->z_acl_bytes);
@@ -2052,8 +2105,8 @@
 		zfs_fuid_info_free(fuidp);
 	dmu_tx_commit(tx);
 done:
+	mutex_exit(&zp->z_lock);
 	mutex_exit(&zp->z_acl_lock);
-	mutex_exit(&zp->z_lock);
 
 	return (error);
 }
@@ -2141,7 +2194,7 @@
 
 	mutex_enter(&zp->z_acl_lock);
 
-	error = zfs_acl_node_read(zp, &aclp, B_FALSE);
+	error = zfs_acl_node_read(zp, B_FALSE, &aclp, B_FALSE);
 	if (error != 0) {
 		mutex_exit(&zp->z_acl_lock);
 		return (error);
--- a/usr/src/uts/common/fs/zfs/zfs_dir.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_dir.c	Mon Jun 14 10:04:10 2010 -0600
@@ -630,7 +630,7 @@
 		ASSERT(error == 0);
 	}
 
-	acl_obj = ZFS_EXTERNAL_ACL(zp);
+	acl_obj = zfs_external_acl(zp);
 
 	/*
 	 * Set up the final transaction.
--- a/usr/src/uts/common/fs/zfs/zfs_ioctl.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_ioctl.c	Mon Jun 14 10:04:10 2010 -0600
@@ -1002,14 +1002,15 @@
  * case its z_vfs will be NULL, and it will be opened as the owner.
  */
 static int
-zfsvfs_hold(const char *name, void *tag, zfsvfs_t **zfvp)
+zfsvfs_hold(const char *name, void *tag, zfsvfs_t **zfvp, boolean_t writer)
 {
 	int error = 0;
 
 	if (getzfsvfs(name, zfvp) != 0)
 		error = zfsvfs_create(name, zfvp);
 	if (error == 0) {
-		rrw_enter(&(*zfvp)->z_teardown_lock, RW_READER, tag);
+		rrw_enter(&(*zfvp)->z_teardown_lock, (writer) ? RW_WRITER :
+		    RW_READER, tag);
 		if ((*zfvp)->z_unmounted) {
 			/*
 			 * XXX we could probably try again, since the unmounting
@@ -1906,7 +1907,7 @@
 	rid = valary[1];
 	quota = valary[2];
 
-	err = zfsvfs_hold(dsname, FTAG, &zfsvfs);
+	err = zfsvfs_hold(dsname, FTAG, &zfsvfs, B_FALSE);
 	if (err == 0) {
 		err = zfs_set_userquota(zfsvfs, type, domain, rid, quota);
 		zfsvfs_rele(zfsvfs, FTAG);
@@ -1971,7 +1972,7 @@
 	{
 		zfsvfs_t *zfsvfs;
 
-		if ((err = zfsvfs_hold(dsname, FTAG, &zfsvfs)) != 0)
+		if ((err = zfsvfs_hold(dsname, FTAG, &zfsvfs, B_TRUE)) != 0)
 			break;
 
 		err = zfs_set_version(zfsvfs, intval);
@@ -3806,7 +3807,7 @@
 	if (zc->zc_objset_type >= ZFS_NUM_USERQUOTA_PROPS)
 		return (EINVAL);
 
-	error = zfsvfs_hold(zc->zc_name, FTAG, &zfsvfs);
+	error = zfsvfs_hold(zc->zc_name, FTAG, &zfsvfs, B_FALSE);
 	if (error)
 		return (error);
 
@@ -3837,7 +3838,7 @@
 	if (bufsize <= 0)
 		return (ENOMEM);
 
-	int error = zfsvfs_hold(zc->zc_name, FTAG, &zfsvfs);
+	int error = zfsvfs_hold(zc->zc_name, FTAG, &zfsvfs, B_FALSE);
 	if (error)
 		return (error);
 
--- a/usr/src/uts/common/fs/zfs/zfs_sa.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_sa.c	Mon Jun 14 10:04:10 2010 -0600
@@ -125,6 +125,7 @@
 	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
 	xoptattr_t *xoap;
 
+	ASSERT(MUTEX_HELD(&zp->z_lock));
 	VERIFY((xoap = xva_getxoptattr(xvap)) != NULL);
 	if (zp->z_is_sa) {
 		if (sa_lookup(zp->z_sa_hdl, SA_ZPL_SCANSTAMP(zfsvfs),
@@ -158,6 +159,7 @@
 	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
 	xoptattr_t *xoap;
 
+	ASSERT(MUTEX_HELD(&zp->z_lock));
 	VERIFY((xoap = xva_getxoptattr(xvap)) != NULL);
 	if (zp->z_is_sa)
 		VERIFY(0 == sa_update(zp->z_sa_hdl, SA_ZPL_SCANSTAMP(zfsvfs),
@@ -204,6 +206,7 @@
 	uint64_t crtime[2], mtime[2], ctime[2];
 	zfs_acl_phys_t znode_acl;
 	char scanstamp[AV_SCANSTAMP_SZ];
+	boolean_t drop_lock = B_FALSE;
 
 	/*
 	 * No upgrade if ACL isn't cached
@@ -214,6 +217,22 @@
 	if (zp->z_acl_cached == NULL || ZTOV(zp)->v_type == VLNK)
 		return;
 
+	/*
+	 * If the z_lock is held and we aren't the owner
+	 * the just return since we don't want to deadlock
+	 * trying to update the status of z_is_sa.  This
+	 * file can then be upgraded at a later time.
+	 *
+	 * Otherwise, we know we are doing the
+	 * sa_update() that caused us to enter this function.
+	 */
+	if (mutex_owner(&zp->z_lock) != curthread) {
+		if (mutex_tryenter(&zp->z_lock) == 0)
+			return;
+		else
+			drop_lock = B_TRUE;
+	}
+
 	/* First do a bulk query of the attributes that aren't cached */
 	SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_MTIME(zfsvfs), NULL, &mtime, 16);
 	SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_CTIME(zfsvfs), NULL, &ctime, 16);
@@ -228,7 +247,7 @@
 	    &znode_acl, 88);
 
 	if (sa_bulk_lookup_locked(hdl, bulk, count) != 0)
-		return;
+		goto done;
 
 
 	/*
@@ -269,9 +288,10 @@
 	locate.cb_aclp = zp->z_acl_cached;
 	SA_ADD_BULK_ATTR(sa_attrs, count, SA_ZPL_DACL_ACES(zfsvfs),
 	    zfs_acl_data_locator, &locate, zp->z_acl_cached->z_acl_bytes);
+
 	if (xattr)
-		SA_ADD_BULK_ATTR(sa_attrs, count, SA_ZPL_RDEV(zfsvfs),
-		    NULL, &rdev, 8);
+		SA_ADD_BULK_ATTR(sa_attrs, count, SA_ZPL_XATTR(zfsvfs),
+		    NULL, &xattr, 8);
 
 	/* if scanstamp then add scanstamp */
 
@@ -291,6 +311,9 @@
 		    znode_acl.z_acl_extern_obj, tx));
 
 	zp->z_is_sa = B_TRUE;
+done:
+	if (drop_lock)
+		mutex_exit(&zp->z_lock);
 }
 
 void
@@ -299,12 +322,11 @@
 	if (!zp->z_zfsvfs->z_use_sa || zp->z_is_sa)
 		return;
 
-	ASSERT(!zp->z_is_sa);
 
 	dmu_tx_hold_sa(tx, zp->z_sa_hdl, B_TRUE);
 
-	if (ZFS_EXTERNAL_ACL(zp)) {
-		dmu_tx_hold_free(tx, ZFS_EXTERNAL_ACL(zp), 0,
+	if (zfs_external_acl(zp)) {
+		dmu_tx_hold_free(tx, zfs_external_acl(zp), 0,
 		    DMU_OBJECT_END);
 	}
 }
--- a/usr/src/uts/common/fs/zfs/zfs_vnops.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c	Mon Jun 14 10:04:10 2010 -0600
@@ -1619,9 +1619,10 @@
 		dmu_tx_hold_sa(tx, xzp->z_sa_hdl, B_FALSE);
 	}
 
-	/* are there any additional acls */
-	if ((acl_obj = ZFS_EXTERNAL_ACL(zp)) != 0 && may_delete_now)
+	mutex_enter(&zp->z_lock);
+	if ((acl_obj = zfs_external_acl(zp)) != 0 && may_delete_now)
 		dmu_tx_hold_free(tx, acl_obj, 0, DMU_OBJECT_END);
+	mutex_exit(&zp->z_lock);
 
 	/* charge as an update -- would be nice not to charge at all */
 	dmu_tx_hold_zap(tx, zfsvfs->z_unlinkedobj, FALSE, NULL);
@@ -1654,13 +1655,18 @@
 
 	if (unlinked) {
 
+		/*
+		 * Hold z_lock so that we can make sure that the ACL obj
+		 * hasn't changed.  Could have been deleted due to
+		 * zfs_sa_upgrade().
+		 */
+		mutex_enter(&zp->z_lock);
 		mutex_enter(&vp->v_lock);
-
 		(void) sa_lookup(zp->z_sa_hdl, SA_ZPL_XATTR(zfsvfs),
 		    &xattr_obj_unlinked, sizeof (xattr_obj_unlinked));
 		delete_now = may_delete_now && !toobig &&
 		    vp->v_count == 1 && !vn_has_cached_data(vp) &&
-		    xattr_obj == xattr_obj_unlinked && ZFS_EXTERNAL_ACL(zp) ==
+		    xattr_obj == xattr_obj_unlinked && zfs_external_acl(zp) ==
 		    acl_obj;
 		mutex_exit(&vp->v_lock);
 	}
@@ -1676,6 +1682,7 @@
 			ASSERT3U(error,  ==,  0);
 			mutex_exit(&xzp->z_lock);
 			zfs_unlinked_add(xzp, tx);
+
 			if (zp->z_is_sa)
 				error = sa_remove(zp->z_sa_hdl,
 				    SA_ZPL_XATTR(zfsvfs), tx);
@@ -1685,7 +1692,6 @@
 				    sizeof (uint64_t), tx);
 			ASSERT3U(error, ==, 0);
 		}
-		mutex_enter(&zp->z_lock);
 		mutex_enter(&vp->v_lock);
 		vp->v_count--;
 		ASSERT3U(vp->v_count, ==, 0);
@@ -1693,6 +1699,7 @@
 		mutex_exit(&zp->z_lock);
 		zfs_znode_delete(zp, tx);
 	} else if (unlinked) {
+		mutex_exit(&zp->z_lock);
 		zfs_unlinked_add(zp, tx);
 	}
 
@@ -2912,32 +2919,34 @@
 
 	if (mask & AT_MODE) {
 		uint64_t pmode = zp->z_mode;
+		uint64_t acl_obj;
 		new_mode = (pmode & S_IFMT) | (vap->va_mode & ~S_IFMT);
 
 		if (err = zfs_acl_chmod_setattr(zp, &aclp, new_mode))
 			goto out;
 
-		if (!zp->z_is_sa && ZFS_EXTERNAL_ACL(zp)) {
+		mutex_enter(&zp->z_lock);
+		if (!zp->z_is_sa && ((acl_obj = zfs_external_acl(zp)) != 0)) {
 			/*
 			 * Are we upgrading ACL from old V0 format
 			 * to V1 format?
 			 */
 			if (zfsvfs->z_version <= ZPL_VERSION_FUID &&
-			    ZNODE_ACL_VERSION(zp) ==
+			    zfs_znode_acl_version(zp) ==
 			    ZFS_ACL_VERSION_INITIAL) {
-				dmu_tx_hold_free(tx,
-				    ZFS_EXTERNAL_ACL(zp), 0,
+				dmu_tx_hold_free(tx, acl_obj, 0,
 				    DMU_OBJECT_END);
 				dmu_tx_hold_write(tx, DMU_NEW_OBJECT,
 				    0, aclp->z_acl_bytes);
 			} else {
-				dmu_tx_hold_write(tx, ZFS_EXTERNAL_ACL(zp), 0,
+				dmu_tx_hold_write(tx, acl_obj, 0,
 				    aclp->z_acl_bytes);
 			}
 		} else if (!zp->z_is_sa && aclp->z_acl_bytes > ZFS_ACE_SPACE) {
 			dmu_tx_hold_write(tx, DMU_NEW_OBJECT,
 			    0, aclp->z_acl_bytes);
 		}
+		mutex_exit(&zp->z_lock);
 		dmu_tx_hold_sa(tx, zp->z_sa_hdl, B_TRUE);
 	} else {
 		if ((mask & AT_XVATTR) &&
@@ -2973,12 +2982,17 @@
 	 * updated as a side-effect of calling this function.
 	 */
 
+
+	if (mask & (AT_UID|AT_GID|AT_MODE))
+		mutex_enter(&zp->z_acl_lock);
 	mutex_enter(&zp->z_lock);
 
 	SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_FLAGS(zfsvfs), NULL,
 	    &zp->z_pflags, sizeof (zp->z_pflags));
 
 	if (attrzp) {
+		if (mask & (AT_UID|AT_GID|AT_MODE))
+			mutex_enter(&attrzp->z_acl_lock);
 		mutex_enter(&attrzp->z_lock);
 		SA_ADD_BULK_ATTR(xattr_bulk, xattr_count,
 		    SA_ZPL_FLAGS(zfsvfs), NULL, &attrzp->z_pflags,
@@ -3026,7 +3040,6 @@
 	}
 
 	if (mask & AT_MODE) {
-		mutex_enter(&zp->z_acl_lock);
 		SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_MODE(zfsvfs), NULL,
 		    &new_mode, sizeof (new_mode));
 		zp->z_mode = new_mode;
@@ -3035,11 +3048,8 @@
 		ASSERT3U(err, ==, 0);
 		zp->z_acl_cached = aclp;
 		aclp = NULL;
-		mutex_exit(&zp->z_acl_lock);
 	}
 
-	if (attrzp)
-		mutex_exit(&attrzp->z_lock);
 
 	if (mask & AT_ATIME) {
 		ZFS_TIME_ENCODE(&vap->va_atime, zp->z_atime);
@@ -3118,7 +3128,14 @@
 		zfs_log_setattr(zilog, tx, TX_SETATTR, zp, vap, mask, fuidp);
 
 	mutex_exit(&zp->z_lock);
-
+	if (mask & (AT_UID|AT_GID|AT_MODE))
+		mutex_exit(&zp->z_acl_lock);
+
+	if (attrzp) {
+		if (mask & (AT_UID|AT_GID|AT_MODE))
+			mutex_exit(&attrzp->z_acl_lock);
+		mutex_exit(&attrzp->z_lock);
+	}
 out:
 	if (err == 0 && attrzp) {
 		err2 = sa_bulk_update(attrzp->z_sa_hdl, xattr_bulk,
@@ -3724,11 +3741,13 @@
 	if (fuid_dirtied)
 		zfs_fuid_sync(zfsvfs, tx);
 
+	mutex_enter(&zp->z_lock);
 	if (zp->z_is_sa)
 		error = sa_update(zp->z_sa_hdl, SA_ZPL_SYMLINK(zfsvfs),
 		    link, len, tx);
 	else
 		zfs_sa_symlink(zp, link, len, tx);
+	mutex_exit(&zp->z_lock);
 
 	zp->z_size = len;
 	(void) sa_update(zp->z_sa_hdl, SA_ZPL_SIZE(zfsvfs),
@@ -3785,11 +3804,13 @@
 	ZFS_ENTER(zfsvfs);
 	ZFS_VERIFY_ZP(zp);
 
+	mutex_enter(&zp->z_lock);
 	if (zp->z_is_sa)
 		error = sa_lookup_uio(zp->z_sa_hdl,
 		    SA_ZPL_SYMLINK(zfsvfs), uio);
 	else
 		error = zfs_sa_readlink(zp, uio);
+	mutex_exit(&zp->z_lock);
 
 	ZFS_ACCESSTIME_STAMP(zfsvfs, zp);
 
--- a/usr/src/uts/common/fs/zfs/zfs_znode.c	Mon Jun 14 07:12:23 2010 -0700
+++ b/usr/src/uts/common/fs/zfs/zfs_znode.c	Mon Jun 14 10:04:10 2010 -0600
@@ -1226,14 +1226,14 @@
 	SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_MODE(zfsvfs), NULL,
 	    &mode, sizeof (mode));
 
-	zp->z_mode = mode;
-
 	if (sa_bulk_lookup(zp->z_sa_hdl, bulk, count)) {
 		zfs_znode_dmu_fini(zp);
 		ZFS_OBJ_HOLD_EXIT(zfsvfs, obj_num);
 		return (EIO);
 	}
 
+	zp->z_mode = mode;
+
 	if (gen != zp->z_gen) {
 		zfs_znode_dmu_fini(zp);
 		ZFS_OBJ_HOLD_EXIT(zfsvfs, obj_num);
@@ -1256,11 +1256,13 @@
 	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
 	objset_t *os = zfsvfs->z_os;
 	uint64_t obj = zp->z_id;
-	uint64_t acl_obj = ZFS_EXTERNAL_ACL(zp);
+	uint64_t acl_obj = zfs_external_acl(zp);
 
 	ZFS_OBJ_HOLD_ENTER(zfsvfs, obj);
-	if (acl_obj)
+	if (acl_obj) {
+		VERIFY(!zp->z_is_sa);
 		VERIFY(0 == dmu_object_free(os, acl_obj, tx));
+	}
 	VERIFY(0 == dmu_object_free(os, obj, tx));
 	zfs_znode_dmu_fini(zp);
 	ZFS_OBJ_HOLD_EXIT(zfsvfs, obj);