LU-7239 mdd: make mdd_attr_set() synchronous less often

In permission_needs_sync(), test mode changes for S_ISUID or S_ISVTX
rather than simply checking whether the new mode in the incoming
request has those bits set.  Similarly check for uid and gid changes.

Prior to these changes, copying a directory tree into lustre, whose
directories' modes have S_ISUID | S_ISGID | S_ISVTX set, is
unnecessarily slow because chmod() system calls are synchronous
whether they change the actual permissions or not.

Change-Id: I41d3297a007eb4b796790b3b3de0ad355d17cb58
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Reviewed-on: https://review.whamcloud.com/16699
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Hongchao Zhang <hongchao@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c
index 4bc830c..1a16bf7 100644 (file)
--- a/lustre/mdd/mdd_object.c
+++ b/lustre/mdd/mdd_object.c
@@ -1099,6 +1099,7 @@ static int mdd_declare_attr_set(const struct lu_env *env,
 
 /*
  * LU-3671
+ * LU-7239
  *
  * permission changes may require sync operation, to mitigate performance
  * impact, only do this for dir and when permission is reduced.
@@ -1113,17 +1114,24 @@ static inline bool permission_needs_sync(const struct lu_attr *old,
        if (!S_ISDIR(old->la_mode))
                return false;
 
-       if (new->la_valid & (LA_UID | LA_GID))
+       if (new->la_valid & LA_UID && old->la_uid != new->la_uid)
                return true;
 
-       if (new->la_valid & LA_MODE &&
-           new->la_mode & (S_ISUID | S_ISGID | S_ISVTX))
+       if (new->la_valid & LA_GID && old->la_gid != new->la_gid)
                return true;
 
-       if ((new->la_valid & LA_MODE) &&
-           ((new->la_mode & old->la_mode) & S_IRWXUGO) !=
-            (old->la_mode & S_IRWXUGO))
-               return true;
+       if (new->la_valid & LA_MODE) {
+               /* turned on sticky bit */
+               if (!(old->la_mode & S_ISVTX) && (new->la_mode & S_ISVTX))
+                       return true;
+
+               /* set-GID has no impact on what is allowed, not checked */
+
+               /* turned off setuid bit, or one of rwx for someone */
+               if (((new->la_mode & old->la_mode) & (0777 | S_ISUID)) !=
+                    (old->la_mode & (0777 | S_ISUID)))
+                       return true;
+       }
 
        return false;
 }