6347134 zfs_zaccess() is killing ZFS stat() performance

6348240 zfs set aclinherit error message incorrect

7 files changed, 30 insertions, 15 deletions

diff --git a/usr/src/common/acl/acl_common.c b/usr/src/common/acl/acl_common.c
index ebe4f060b5..a5615b3801 100644
--- a/usr/src/common/acl/acl_common.c
+++ b/usr/src/common/acl/acl_common.c
@@ -120,9 +120,11 @@ ace_trivial(ace_t *acep, int aclcnt)
 		/*
 		 * Special check for some special bits
 		 *
-		 * Don't allow anybody to deny reading an ACL
+		 * Don't allow anybody to deny reading basic
+		 * attributes or a files ACL.
 		 */
-		if ((acep[i].a_access_mask & ACE_READ_ACL) &&
+		if ((acep[i].a_access_mask &
+		    (ACE_READ_ACL|ACE_READ_ATTRIBUTES)) &&
 		    (acep[i].a_type == ACE_ACCESS_DENIED_ACE_TYPE))
 			return (1);
 
diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
index 06e942fcf8..f88057c39d 100644
--- a/usr/src/lib/libzfs/common/libzfs_dataset.c
+++ b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -687,7 +687,7 @@ zfs_prop_validate(zfs_prop_t prop, const char *value, uint64_t *intval)
 			if (acl_inherit_table[i].name == NULL) {
 				zfs_error(dgettext(TEXT_DOMAIN,
 				    "bad %s value '%s': must be 'discard', "
-				    "'noallow', 'groupmask' or 'passthrough'"),
+				    "'noallow', 'secure' or 'passthrough'"),
 				    propname, value);
 				return (-1);
 			}
diff --git a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
index 0050316eb5..2ea27493f9 100644
--- a/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
+++ b/usr/src/uts/common/fs/zfs/sys/zfs_acl.h
@@ -50,7 +50,7 @@ typedef struct zfs_znode_acl {
 	uint64_t	z_acl_extern_obj;	  /* ext acl pieces */
 	uint32_t	z_acl_count;		  /* Number of ACEs */
 	uint16_t	z_acl_version;		  /* acl version */
-	uint16_t	z_acl_state;		  /* goop */
+	uint16_t	z_acl_pad;		  /* pad */
 	ace_t		z_ace_data[ACE_SLOT_CNT]; /* 6 standard ACEs */
 } zfs_znode_acl_t;
 
diff --git a/usr/src/uts/common/fs/zfs/sys/zfs_znode.h b/usr/src/uts/common/fs/zfs/sys/zfs_znode.h
index 77c4a5fb16..f9331be00a 100644
--- a/usr/src/uts/common/fs/zfs/sys/zfs_znode.h
+++ b/usr/src/uts/common/fs/zfs/sys/zfs_znode.h
@@ -48,6 +48,7 @@ extern "C" {
  */
 #define	ZFS_XATTR	0x1		/* is an extended attribute */
 #define	ZFS_INHERIT_ACE	0x2		/* ace has inheritable ACEs */
+#define	ZFS_ACL_TRIVIAL 0x4		/* files ACL is trivial */
 
 #define	MASTER_NODE_OBJ	1
 
diff --git a/usr/src/uts/common/fs/zfs/zfs_acl.c b/usr/src/uts/common/fs/zfs/zfs_acl.c
index 97d1956579..2f38923f88 100644
--- a/usr/src/uts/common/fs/zfs/zfs_acl.c
+++ b/usr/src/uts/common/fs/zfs/zfs_acl.c
@@ -433,10 +433,13 @@ zfs_aclset_common(znode_t *zp, zfs_acl_t *aclp, dmu_tx_t *tx, int *ihp)
 		    aclp->z_acl_count * sizeof (ace_t));
 		zacl->z_acl_count = aclp->z_acl_count;
 	}
-	if (inherit)
+
+	zp->z_phys->zp_flags &= ~(ZFS_ACL_TRIVIAL|ZFS_INHERIT_ACE);
+	if (inherit) {
 		zp->z_phys->zp_flags |= ZFS_INHERIT_ACE;
-	else
-		zp->z_phys->zp_flags &= ~ZFS_INHERIT_ACE;
+	} else if (ace_trivial(zacl->z_ace_data, zacl->z_acl_count) == 0) {
+		zp->z_phys->zp_flags |= ZFS_ACL_TRIVIAL;
+	}
 
 	zphys->zp_mode = zfs_mode_compute(zp, aclp);
 	zfs_time_stamper_locked(zp, STATE_CHANGED, tx);
@@ -750,6 +753,8 @@ zfs_acl_chmod(znode_t *zp, uint64_t mode, zfs_acl_t *aclp,
 	int 		entry_type;
 	int 		reuse_deny;
 	int 		need_canonical_six = 1;
+	int		inherit = 0;
+	int		iflags;
 
 	ASSERT(MUTEX_HELD(&zp->z_acl_lock));
 	ASSERT(MUTEX_HELD(&zp->z_lock));
@@ -758,10 +763,13 @@ zfs_acl_chmod(znode_t *zp, uint64_t mode, zfs_acl_t *aclp,
 	while (i < aclp->z_acl_count) {
 		acep = aclp->z_acl;
 		entry_type = (acep[i].a_flags & 0xf040);
+		iflags = (acep[i].a_flags & ALL_INHERIT);
 
 		if ((acep[i].a_type != ALLOW && acep[i].a_type != DENY) ||
-		    (acep[i].a_flags & ACE_INHERIT_ONLY_ACE)) {
+		    (iflags & ACE_INHERIT_ONLY_ACE)) {
 			i++;
+			if (iflags)
+				inherit = 1;
 			continue;
 		}
 
@@ -774,11 +782,12 @@ zfs_acl_chmod(znode_t *zp, uint64_t mode, zfs_acl_t *aclp,
 		/*
 		 * Need to split ace into two?
 		 */
-		if ((acep[i].a_flags & (ACE_FILE_INHERIT_ACE|
+		if ((iflags & (ACE_FILE_INHERIT_ACE|
 		    ACE_DIRECTORY_INHERIT_ACE)) &&
-		    (!(acep[i].a_flags & ACE_INHERIT_ONLY_ACE))) {
+		    (!(iflags & ACE_INHERIT_ONLY_ACE))) {
 			zfs_acl_split_ace(aclp, i);
 			i++;
+			inherit = 1;
 			continue;
 		}
 
@@ -851,7 +860,7 @@ zfs_acl_chmod(znode_t *zp, uint64_t mode, zfs_acl_t *aclp,
 	zfs_acl_fixup_canonical_six(aclp, mode);
 
 	zp->z_phys->zp_mode = mode;
-	error = zfs_aclset_common(zp, aclp, tx, NULL);
+	error = zfs_aclset_common(zp, aclp, tx, &inherit);
 	return (error);
 }
 
diff --git a/usr/src/uts/common/fs/zfs/zfs_byteswap.c b/usr/src/uts/common/fs/zfs/zfs_byteswap.c
index e1e857aa44..c8450d488b 100644
--- a/usr/src/uts/common/fs/zfs/zfs_byteswap.c
+++ b/usr/src/uts/common/fs/zfs/zfs_byteswap.c
@@ -94,6 +94,6 @@ zfs_znode_byteswap(void *buf, size_t size)
 	zp->zp_acl.z_acl_extern_obj = BSWAP_64(zp->zp_acl.z_acl_extern_obj);
 	zp->zp_acl.z_acl_count = BSWAP_32(zp->zp_acl.z_acl_count);
 	zp->zp_acl.z_acl_version = BSWAP_16(zp->zp_acl.z_acl_version);
-	zp->zp_acl.z_acl_state = BSWAP_16(zp->zp_acl.z_acl_state);
+	zp->zp_acl.z_acl_pad = BSWAP_16(zp->zp_acl.z_acl_pad);
 	zfs_ace_byteswap(&zp->zp_acl.z_ace_data[0], ACE_SLOT_CNT);
 }
diff --git a/usr/src/uts/common/fs/zfs/zfs_vnops.c b/usr/src/uts/common/fs/zfs/zfs_vnops.c
index af06948b79..980dc552f7 100644
--- a/usr/src/uts/common/fs/zfs/zfs_vnops.c
+++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c
@@ -1851,10 +1851,13 @@ zfs_getattr(vnode_t *vp, vattr_t *vap, int flags, cred_t *cr)
 	ZFS_TIME_DECODE(&vap->va_ctime, pzp->zp_ctime);
 
 	/*
-	 * Owner should be allowed to always read_attributes
+	 * If ACL is trivial don't bother looking for ACE_READ_ATTRIBUTES.
+	 * Also, if we are the owner don't bother, since owner should
+	 * always be allowed to read basic attributes of file.
 	 */
-	if (error = zfs_zaccess(zp, ACE_READ_ATTRIBUTES, cr)) {
-		if (zp->z_phys->zp_uid != crgetuid(cr)) {
+	if (!(zp->z_phys->zp_flags & ZFS_ACL_TRIVIAL) &&
+	    (zp->z_phys->zp_uid != crgetuid(cr))) {
+		if (error = zfs_zaccess(zp, ACE_READ_ATTRIBUTES, cr)) {
 			mutex_exit(&zp->z_lock);
 			ZFS_EXIT(zfsvfs);
 			return (error);