PERFORCE change 148425 for review
Edward Tomasz Napierala
trasz at FreeBSD.org
Mon Aug 25 19:16:56 UTC 2008
http://perforce.freebsd.org/chv.cgi?CH=148425
Change 148425 by trasz at trasz_traszkan on 2008/08/25 19:16:10
Fix a performance problem, visible e.g. with untarring the ports tree.
Change the return type of acl_nfs4_sync_acl_from_mode
and acl_nfs4_compute_inherited_acl to void, as they cannot return
an error.
Affected files ...
.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#28 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/sys/acl.h#24 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_acl.c#12 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#16 edit
Differences ...
==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#28 (text+ko) ====
@@ -324,7 +324,7 @@
return (&(aclp->acl_entry[entry_index + 1]));
}
-int
+void
acl_nfs4_sync_acl_from_mode(struct acl *aclp, mode_t mode, int file_owner_id)
{
int i, meets, must_append;
@@ -335,6 +335,10 @@
const int WRITE = 02;
const int EXEC = 01;
+ KASSERT(aclp->acl_cnt >= 0, ("aclp->acl_cnt >= 0"));
+ KASSERT(aclp->acl_cnt <= ACL_MAX_ENTRIES,
+ ("aclp->acl_cnt <= ACL_MAX_ENTRIES"));
+
/*
* NFSv4 Minor Version 1, draft-ietf-nfsv4-minorversion1-03.txt
*
@@ -600,8 +604,8 @@
}
if (must_append) {
- if (aclp->acl_cnt + 6 >= ACL_MAX_ENTRIES)
- return (EPERM);
+ KASSERT(aclp->acl_cnt + 6 <= ACL_MAX_ENTRIES,
+ ("aclp->acl_cnt <= ACL_MAX_ENTRIES"));
a1 = _acl_append(aclp, ACL_USER_OBJ, 0, ACL_EXTENDED_DENY);
a2 = _acl_append(aclp, ACL_USER_OBJ, ACL_WRITE_ACL |
@@ -661,10 +665,6 @@
a6->ae_perm |= ACL_EXECUTE;
else
a5->ae_perm |= ACL_EXECUTE;
-
- KASSERT(aclp->acl_cnt >= 6, ("acl->acl_cnt >= 6"));
-
- return (0);
}
void
@@ -674,6 +674,10 @@
mode_t old_mode = *_mode, mode = 0, seen = 0;
const struct acl_entry *entry;
+ KASSERT(aclp->acl_cnt > 0, ("aclp->acl_cnt > 0"));
+ KASSERT(aclp->acl_cnt <= ACL_MAX_ENTRIES,
+ ("aclp->acl_cnt <= ACL_MAX_ENTRIES"));
+
/*
* NFSv4 Minor Version 1, draft-ietf-nfsv4-minorversion1-03.txt
*
@@ -800,16 +804,19 @@
*_mode = mode | (old_mode & ACL_PRESERVE_MASK);
}
-int
+void
acl_nfs4_compute_inherited_acl(const struct acl *parent_aclp,
struct acl *child_aclp, mode_t mode, int file_owner_id,
int is_directory)
{
- int i, error, flags;
+ int i, flags;
const struct acl_entry *parent_entry;
struct acl_entry *entry, *copy;
KASSERT(child_aclp->acl_cnt == 0, ("child_aclp->acl_cnt == 0"));
+ KASSERT(parent_aclp->acl_cnt > 0, ("parent_aclp->acl_cnt > 0"));
+ KASSERT(parent_aclp->acl_cnt <= ACL_MAX_ENTRIES,
+ ("parent_aclp->acl_cnt <= ACL_MAX_ENTRIES"));
/*
* NFSv4 Minor Version 1, draft-ietf-nfsv4-minorversion1-03.txt
@@ -955,9 +962,31 @@
* in Section 2.16.6.3, using the mode that is to be used for file
* creation.
*/
- error = acl_nfs4_sync_acl_from_mode(child_aclp, mode, file_owner_id);
+ acl_nfs4_sync_acl_from_mode(child_aclp, mode, file_owner_id);
+}
+
+static int
+_acls_are_equal(const struct acl *a, const struct acl *b)
+{
+ int i;
+ const struct acl_entry *entrya, *entryb;
+
+ if (a->acl_cnt != b->acl_cnt)
+ return (0);
+
+ for (i = 0; i < b->acl_cnt; i++) {
+ entrya = &(a->acl_entry[i]);
+ entryb = &(b->acl_entry[i]);
+
+ if (entrya->ae_tag != entryb->ae_tag ||
+ entrya->ae_id != entryb->ae_id ||
+ entrya->ae_perm != entryb->ae_perm ||
+ entrya->ae_extended != entryb->ae_extended ||
+ entrya->ae_flags != entryb->ae_flags)
+ return (0);
+ }
- return (error);
+ return (1);
}
/*
@@ -965,9 +994,32 @@
* that stores ACL contents.
*/
int
-acl_nfs4_is_trivial(const struct acl *aclp)
+acl_nfs4_is_trivial(const struct acl *aclp, int file_owner_id)
{
- return (0);
+ int trivial;
+ mode_t tmpmode = 0;
+ struct acl *tmpaclp;
+
+ if (aclp->acl_cnt != 6)
+ return (0);
+
+ /*
+ * Compute the mode from the ACL, then compute new ACL from that mode.
+ * If the ACLs are identical, then the ACL is trivial.
+ *
+ * XXX: I guess there is a faster way to do this. However, even
+ * this slow implementation significantly speeds things up
+ * for files that don't have any extended ACL entries - it's
+ * critical for performance to not use EA when they are not
+ * needed.
+ */
+ tmpaclp = acl_alloc();
+ acl_nfs4_sync_mode_from_acl(&tmpmode, aclp);
+ acl_nfs4_sync_acl_from_mode(tmpaclp, tmpmode, file_owner_id);
+ trivial = _acls_are_equal(aclp, tmpaclp);
+ acl_free(tmpaclp);
+
+ return (trivial);
}
int
==== //depot/projects/soc2008/trasz_nfs4acl/sys/sys/acl.h#24 (text+ko) ====
@@ -270,12 +270,13 @@
mode_t acl_posix1e_newfilemode(mode_t cmode,
struct acl *dacl);
-int acl_nfs4_sync_acl_from_mode(struct acl *aclp,
+void acl_nfs4_sync_acl_from_mode(struct acl *aclp,
mode_t mode, int file_owner_id);
void acl_nfs4_sync_mode_from_acl(mode_t *mode,
const struct acl *aclp);
-int acl_nfs4_is_trivial(const struct acl *aclp);
-int acl_nfs4_compute_inherited_acl(
+int acl_nfs4_is_trivial(const struct acl *aclp,
+ int file_owner_id);
+void acl_nfs4_compute_inherited_acl(
const struct acl *parent_aclp,
struct acl *child_aclp, mode_t mode,
int file_owner_id, int is_directory);
==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_acl.c#12 (text+ko) ====
@@ -163,10 +163,9 @@
* Legitimately no ACL set on object, purely
* emulate it through the inode.
*/
- error = acl_nfs4_sync_acl_from_mode(ap->a_aclp, ip->i_mode,
- ip->i_uid);
+ acl_nfs4_sync_acl_from_mode(ap->a_aclp, ip->i_mode, ip->i_uid);
- return (error);
+ return (0);
}
if (error)
@@ -404,16 +403,23 @@
return (EPERM);
/*
- * Must hold VADMIN (be file owner) or have appropriate privilege.
+ * Must hold VWRITE_ACL or have appropriate privilege.
*/
if ((error = VOP_ACCESS(ap->a_vp, VWRITE_ACL, ap->a_cred, ap->a_td)))
return (error);
- if (acl_nfs4_is_trivial(ap->a_aclp)) {
+ if (acl_nfs4_is_trivial(ap->a_aclp, ip->i_uid)) {
error = vn_extattr_rm(ap->a_vp, IO_NODELOCKED,
NFS4_ACL_EXTATTR_NAMESPACE,
NFS4_ACL_EXTATTR_NAME, ap->a_td);
+ /*
+ * An attempt to remove ACL from a file that didn't have
+ * any extended entries is not an error.
+ */
+ if (error == ENOATTR)
+ error = 0;
+
} else {
ap->a_aclp->acl_magic = ACL_MAGIC;
error = vn_extattr_set(ap->a_vp, IO_NODELOCKED,
==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#16 (text+ko) ====
@@ -669,9 +669,7 @@
if (error)
goto out;
- error = acl_nfs4_sync_acl_from_mode(aclp, mode, file_owner_id);
- if (error)
- goto out;
+ acl_nfs4_sync_acl_from_mode(aclp, mode, file_owner_id);
error = VOP_SETACL(vp, ACL_TYPE_NFS4, aclp, cred, td);
@@ -1430,10 +1428,8 @@
if (error)
goto out;
- error = acl_nfs4_compute_inherited_acl(parent_aclp, child_aclp,
+ acl_nfs4_compute_inherited_acl(parent_aclp, child_aclp,
child_mode, VTOI(tvp)->i_uid, tvp->v_type == VDIR);
- if (error)
- goto out;
error = VOP_SETACL(tvp, ACL_TYPE_NFS4, child_aclp, cred, td);
if (error)
More information about the p4-projects
mailing list