Re: modify chmod

[jeff.liu]

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 also tried to pass over the openat(), instead just comparing the fts_cwd_fd 
againt AT_FDCWD,
if the pathname is interpreted relative to the current working directory of the 
calling process,
call statfs().
or call fstafs(fts_cwd_fd...).

But consider the bind-mounted directory, it is not a proper way.
Anyway, I add a timer instruments to help figure out the performance difference 
between
may_have_acl() and chmodat(),
If does not have to call openat(), the performance looks fine.

Please check the following test results:
address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 755 
/ocfs2/
Time spent: 1.419816

real    0m10.584s
user    0m0.296s
sys     0m10.045s
address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 755 
/ocfs2/
Time spent: 1.256965

real    0m9.176s
user    0m0.220s
sys     0m8.841s
address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 644 
/ocfs2/
Time spent: 9.245956

real    0m17.541s
user    0m0.344s
sys     0m17.053s
address@hidden:~/opensource_dev/coreutils$ time yes | sudo ./src/chmod -R 644 
/ocfs2/
Time spent: 1.494338

real    0m10.972s
user    0m0.360s
sys     0m10.293s


code diff based on the last replay:

diff --git a/src/chmod.c b/src/chmod.c
index b49adb8..430ee6f 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -21,6 +21,7 @@
 #include <getopt.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <sys/time.h>

 #include "system.h"
 #include "dev-ino.h"
@@ -194,8 +195,10 @@ may_have_nfsacl (FTSENT const *ent)
   int fd;
   int cwd_fd = ent->fts_fts->fts_cwd_fd;
   char const *file = ent->fts_accpath;
+  char const *file_full_name = ent->fts_path;
   struct statfs buf;

+#if 0
   fd = openat (cwd_fd, file, O_RDONLY);
   if (0 <= fd)
     {
@@ -209,10 +212,18 @@ may_have_nfsacl (FTSENT const *ent)
       close (fd);
       return buf.f_type == S_MAGIC_NFS;
     }
+#endif

-  char const *file_full_name = ent->fts_path;
-  if (statfs (file_full_name, &buf) < 0)
-    return true;
+  if (cwd_fd != AT_FDCWD)
+    {
+      if (fstatfs (cwd_fd, &buf) < 0)
+        return true;
+    }
+  else
+    {
+      if (statfs (file_full_name, &buf) < 0)
+        return true;
+    }

   return buf.f_type == S_MAGIC_NFS;
 }
@@ -220,6 +231,8 @@ may_have_nfsacl (FTSENT const *ent)
 # define may_have_nfsacl(ignored) (true)
 #endif

+double tot_timer_spent;
+
 /* Change the mode of FILE.
    Return true if successful.  This function is called
    once for every file system object that fts encounters.  */
@@ -234,6 +247,9 @@ process_file (FTS *fts, FTSENT *ent)
   mode_t new_mode IF_LINT (= 0);
   bool ok = true;
   bool chmod_succeeded = false;
+  struct timeval timer_start;
+  struct timeval timer_end;
+  double timer_spent;

   switch (ent->fts_info)
     {
@@ -318,6 +334,8 @@ process_file (FTS *fts, FTSENT *ent)

       if ((old_mode & CHMOD_MODE_BITS) == new_mode)
         {
+          gettimeofday(&timer_start, NULL);
+
           if (!may_have_nfsacl (ent))
             {
               if (file_stats->st_uid != euid && euid != 0)
@@ -326,11 +344,18 @@ process_file (FTS *fts, FTSENT *ent)
               else
                 chmod_succeeded = true;
             }
+
+          gettimeofday(&timer_end, NULL);
+          timer_spent = ((timer_end.tv_sec - timer_start.tv_sec) +
+                         (timer_end.tv_usec - timer_start.tv_usec) / 
1000000.0);
+          tot_timer_spent += timer_spent;
        }
       else
         {
           if (! S_ISLNK (old_mode))
             {
+              gettimeofday(&timer_start, NULL);
+
               if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
                 chmod_succeeded = true;
               else
@@ -340,6 +365,11 @@ process_file (FTS *fts, FTSENT *ent)
                            quote (file_full_name));
                   ok = false;
                 }
+
+             gettimeofday(&timer_end, NULL);
+              timer_spent = ((timer_end.tv_sec - timer_start.tv_sec) +
+                             (timer_end.tv_usec - timer_start.tv_usec) / 
1000000.0);
+              tot_timer_spent += timer_spent;
             }
         }
     }
@@ -416,6 +446,8 @@ process_files (char **files, int bit_flags)
       ok &= process_file (fts, ent);
     }

+  fprintf(stderr, "Time spent: %.6f\n", tot_timer_spent);
+
   if (fts_close (fts) != 0)
     {
       error (0, errno, _("fts_close failed"));