Whamcloud - gitweb
LU-14801: utils: improve performance of 'lfs find -perm' 18/44118/11
authorCourrier Guillaume <guillaume.courrier@cea.fr>
Wed, 23 Jun 2021 09:15:08 +0000 (11:15 +0200)
committerOleg Drokin <green@whamcloud.com>
Sun, 10 Oct 2021 03:31:09 +0000 (03:31 +0000)
The current implementation of the "-perm" predicate queries the
stat information on the OST while it is not necessary. This patch
fixes that issue by moving the check to the correct location in
`cb_find_init`

On a simple setup with 1 MDT and 2 OSTs we observed for around 8000
files:
- 2.3s without the patch
- 1.9s with the patch

Test-Parameters: trivial
Change-Id: I30c0e89d136556058eadf6bede062577c6d36eaf
Signed-off-by: Courrier Guillaume <guillaume.courrier@cea.fr>
Reviewed-on: https://review.whamcloud.com/44118
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Olaf Faaland-LLNL <faaland1@llnl.gov>
Reviewed-by: Rick Mohr <mohrrf@ornl.gov>
Reviewed-by: Anjus George <georgea@ornl.gov>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
lustre/utils/liblustreapi.c

index 88d39bf..511b48f 100644 (file)
@@ -4892,20 +4892,21 @@ static int fget_projid(int fd, int *projid)
  * Check that the file's permissions in *st matches the one in find_param
  */
 static int check_file_permissions(const struct find_param *param,
  * Check that the file's permissions in *st matches the one in find_param
  */
 static int check_file_permissions(const struct find_param *param,
-                       const lstat_t *st)
+                       mode_t mode)
 {
 {
-       const mode_t st_mode = st->st_mode & 07777;
        int decision = 0;
 
        int decision = 0;
 
+       mode &= 07777;
+
        switch (param->fp_perm_sign) {
        case LFS_FIND_PERM_EXACT:
        switch (param->fp_perm_sign) {
        case LFS_FIND_PERM_EXACT:
-               decision = (st_mode == param->fp_perm);
+               decision = (mode == param->fp_perm);
                break;
        case LFS_FIND_PERM_ALL:
                break;
        case LFS_FIND_PERM_ALL:
-               decision = ((st_mode & param->fp_perm) == param->fp_perm);
+               decision = ((mode & param->fp_perm) == param->fp_perm);
                break;
        case LFS_FIND_PERM_ANY:
                break;
        case LFS_FIND_PERM_ANY:
-               decision = ((st_mode & param->fp_perm) != 0);
+               decision = ((mode & param->fp_perm) != 0);
                break;
        }
 
                break;
        }
 
@@ -5106,6 +5107,13 @@ static int cb_find_init(char *path, int p, int *dp,
                }
        }
 
                }
        }
 
+       /* Check the file permissions from the stat info */
+       if (param->fp_perm_sign) {
+               decision = check_file_permissions(param, lmd->lmd_stx.stx_mode);
+               if (decision == -1)
+                       goto decided;
+       }
+
        if (param->fp_type && !checked_type) {
                if ((param->fp_check_mdt_count || param->fp_check_hash_flag ||
                     param->fp_hash_type) && !S_ISDIR(lmd->lmd_stx.stx_mode))
        if (param->fp_type && !checked_type) {
                if ((param->fp_check_mdt_count || param->fp_check_hash_flag ||
                     param->fp_hash_type) && !S_ISDIR(lmd->lmd_stx.stx_mode))
@@ -5328,9 +5336,6 @@ obd_matches:
              (param->fp_lazy && flags & OBD_MD_FLLAZYBLOCKS)))
                decision = 0;
 
              (param->fp_lazy && flags & OBD_MD_FLLAZYBLOCKS)))
                decision = 0;
 
-       if (param->fp_perm_sign)
-               decision = 0;
-
        /*
         * If file still fits the request, ask ost for updated info.
         * The regular stat is almost of the same speed as some new
        /*
         * If file still fits the request, ask ost for updated info.
         * The regular stat is almost of the same speed as some new
@@ -5387,13 +5392,6 @@ obd_matches:
                                goto out;
                        }
                }
                                goto out;
                        }
                }
-
-               /* Check the file permissions from the stat info */
-               if (param->fp_perm_sign) {
-                       decision = check_file_permissions(param, &st);
-                       if (decision == -1)
-                               goto decided;
-               }
        }
 
        if (param->fp_check_size) {
        }
 
        if (param->fp_check_size) {