Re: modify chmod

[jeff.liu]

Jim Meyering 写道:
> Finally, I see the one true way ;-)
> Do this for each "name":
> 
>     - open each file/dir. with fd = openat (fts_cwd_fd, name, ...
>     - if that succeeds, then call fstatfs (fd, ...
> 
> Using the combination of openat and fstatfs is required in order to
> avoid using the full relative name of each object that chmod processes.
> 
> The above works for all readable objects.
> For unreadable ones, revert to using the statfs with the full relative name.
>

Thanks for the hints.

But according to my tryout, it looks we can not gain performance benefits in 
this way. :(
calling openat() could cause a bits overhead as well.

I have done a simple test against a directory mounted as ext3 fs, set its 
default permission bits to
644.

address@hidden:/ocfs2# du -sh .
2.0G    .

address@hidden:/ocfs2# ls -ld .
drw-r--r-- 8 root root 4096 Feb  8 16:11 .

address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 644 
/ocfs2/

real    0m16.421s
user    0m0.400s
sys     0m15.605s

address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 755 
/ocfs2/

real    0m17.129s
user    0m0.308s
sys     0m16.233s

address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 755 
/ocfs2/

real    0m17.029s
user    0m0.400s
sys     0m16.157s


Could you check the following patch, hope I didn't misunderstood your idea.

>From ec84e84d79a69d1422e0d02ab99a65c30c75b27f Mon Sep 17 00:00:00 2001
From: Jie Liu <address@hidden>
Date: Mon, 8 Feb 2010 16:51:37 +0800
Subject: [PATCH] chmod modify: do not touch inode for same permission bits v2

This patch change the chmod default behavior to skip chmodat(2)
the inode if its new permission bits is same as it was before.

Signed-off-by: Jie Liu <address@hidden>
---
 src/chmod.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/src/chmod.c b/src/chmod.c
index 3dab92e..b49adb8 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -20,6 +20,7 @@
 #include <stdio.h>
 #include <getopt.h>
 #include <sys/types.h>
+#include <unistd.h>

 #include "system.h"
 #include "dev-ino.h"
@@ -32,6 +33,16 @@
 #include "root-dev-ino.h"
 #include "xfts.h"

+/* Some systems only have <sys/vfs.h>, other systems also
+   have <sys/statfs.h>, where the former includes the latter.
+   So it seems including the former is the best choice.  */
+#include "fs.h"
+#if HAVE_SYS_VFS_H
+# include <sys/vfs.h>
+#else
+# include <sys/statfs.h>
+#endif
+
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "chmod"

@@ -83,6 +94,9 @@ static enum Verbosity verbosity = V_off;
    Otherwise NULL.  */
 static struct dev_ino *root_dev_ino;

+/* The effective user ID of the caller process.  */
+static uid_t euid;
+
 /* For long options that have no equivalent short option, use a
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
 enum
@@ -170,6 +184,42 @@ describe_change (const char *file, mode_t mode,
           (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]);
 }

+/* Return true if the file resides on NFS filesystem.
+   Limit this optimization to systems that provide fstatfs.  */
+
+#if HAVE_FSTATFS && HAVE_SYS_STATFS_H && HAVE_STRUCT_STATFS_F_TYPE
+static bool
+may_have_nfsacl (FTSENT const *ent)
+{
+  int fd;
+  int cwd_fd = ent->fts_fts->fts_cwd_fd;
+  char const *file = ent->fts_accpath;
+  struct statfs buf;
+
+  fd = openat (cwd_fd, file, O_RDONLY);
+  if (0 <= fd)
+    {
+      /* If fstatfs fails, assume we can't use the optimization.  */
+      if (fstatfs (fd, &buf) < 0)
+        {
+          close (fd);
+          return true;
+       }
+
+      close (fd);
+      return buf.f_type == S_MAGIC_NFS;
+    }
+
+  char const *file_full_name = ent->fts_path;
+  if (statfs (file_full_name, &buf) < 0)
+    return true;
+
+  return buf.f_type == S_MAGIC_NFS;
+}
+#else
+# define may_have_nfsacl(ignored) (true)
+#endif
+
 /* Change the mode of FILE.
    Return true if successful.  This function is called
    once for every file system object that fts encounters.  */
@@ -258,16 +308,38 @@ process_file (FTS *fts, FTSENT *ent)
       new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value,
                               change, NULL);

-      if (! S_ISLNK (old_mode))
+      /* Do not touch the inode if the new file mode is the same as it was 
before;
+         This is an optimization for some cases. However, the new behavior 
must be
+         disabled for filesystems that support NFSv4 ACLs.
+         This idea suggest by Paul Eggert refer to FreeBSD chmod(1).
+         It uses pathconf(2)/lpathconf(2) to determine whether this is the 
case.
+         But we lack of them on linux, Instead, calling fstatfs(2) and compare 
the
+         f_type we can still do optimization at least its not NFS.  */
+
+      if ((old_mode & CHMOD_MODE_BITS) == new_mode)
         {
-          if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
-            chmod_succeeded = true;
-          else
+          if (!may_have_nfsacl (ent))
             {
-              if (! force_silent)
-                error (0, errno, _("changing permissions of %s"),
+              if (file_stats->st_uid != euid && euid != 0)
+                error (0, EPERM, _("changing permission of %s"),
                        quote (file_full_name));
-              ok = false;
+              else
+                chmod_succeeded = true;
+            }
+       }
+      else
+        {
+          if (! S_ISLNK (old_mode))
+            {
+              if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
+                chmod_succeeded = true;
+              else
+                {
+                  if (! force_silent)
+                    error (0, errno, _("changing permissions of %s"),
+                           quote (file_full_name));
+                  ok = false;
+                }
             }
         }
     }
@@ -545,6 +617,7 @@ main (int argc, char **argv)
       root_dev_ino = NULL;
     }

+  euid = geteuid ();
   ok = process_files (argv + optind,
                       FTS_COMFOLLOW | FTS_PHYSICAL | FTS_DEFER_STAT);

--
1.5.4.3