xfs_scrub: in phase 3, use the opened file descriptor for repair calls

While profiling the performance of xfs_scrub, I noticed that phase3 only
employs the scrub-by-handle interface for repairs.  The kernel has had
the ability to skip the untrusted iget lookup if the fd matches the
handle data since the beginning, and using it reduces the repair runtime
by 5% on the author's system.  Normally, we shouldn't be running that
many repairs or optimizations, but we did this for scrub, so we should
do the same for repair.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/scrub/phase3.c b/scrub/phase3.c
index fd8e541940d2c14e94617f0008710b1e7cb99e4c..d659a7791825ca871713fa9bc0d091418407662e 100644 (file)
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -52,9 +52,12 @@ report_close_error(
 static int
 try_inode_repair(
        struct scrub_inode_ctx  *ictx,
+       int                     fd,
        xfs_agnumber_t          agno,
        struct action_list      *alist)
 {
+       int                     ret;
+
        /*
         * If at the start of phase 3 we already had ag/rt metadata repairs
         * queued up for phase 4, leave the action list untouched so that file
@@ -63,7 +66,13 @@ try_inode_repair(
        if (ictx->always_defer_repairs)
                return 0;
 
-       return action_list_process_or_defer(ictx->ctx, agno, alist);
+       ret = action_list_process(ictx->ctx, fd, alist,
+                       ALP_REPAIR_ONLY | ALP_NOPROGRESS);
+       if (ret)
+               return ret;
+
+       action_list_defer(ictx->ctx, agno, alist);
+       return 0;
 }
 
 /* Verify the contents, xattrs, and extent maps of an inode. */
@@ -117,7 +126,7 @@ scrub_inode(
        if (error)
                goto out;
 
-       error = try_inode_repair(ictx, agno, &alist);
+       error = try_inode_repair(ictx, fd, agno, &alist);
        if (error)
                goto out;
 
@@ -132,7 +141,7 @@ scrub_inode(
        if (error)
                goto out;
 
-       error = try_inode_repair(ictx, agno, &alist);
+       error = try_inode_repair(ictx, fd, agno, &alist);
        if (error)
                goto out;
 
@@ -158,7 +167,7 @@ scrub_inode(
                goto out;
 
        /* Try to repair the file while it's open. */
-       error = try_inode_repair(ictx, agno, &alist);
+       error = try_inode_repair(ictx, fd, agno, &alist);
        if (error)
                goto out;
 

diff --git a/scrub/phase4.c b/scrub/phase4.c
index 4fd1cb799a4619dd6804107560b82a382074c71d..e78bc1e48c2fa39c8ea8be0bc0501af2c36a4c01 100644 (file)
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -40,7 +40,7 @@ repair_ag(
 
        /* Repair anything broken until we fail to make progress. */
        do {
-               ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
+               ret = action_list_process(ctx, -1, alist, flags);
                if (ret) {
                        *aborted = true;
                        return;
@@ -55,7 +55,7 @@ repair_ag(
 
        /* Try once more, but this time complain if we can't fix things. */
        flags |= ALP_COMPLAIN_IF_UNFIXED;
-       ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
+       ret = action_list_process(ctx, -1, alist, flags);
        if (ret)
                *aborted = true;
 }

diff --git a/scrub/phase7.c b/scrub/phase7.c
index 84546b1cb5cc2b29047f151cf892ef5b5e8038f0..8d8034c36afb68c0d085346c4b99a9ee68703ca7 100644 (file)
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -121,7 +121,7 @@ phase7_func(
        error = scrub_fs_summary(ctx, &alist);
        if (error)
                return error;
-       error = action_list_process(ctx, ctx->mnt.fd, &alist,
+       error = action_list_process(ctx, -1, &alist,
                        ALP_COMPLAIN_IF_UNFIXED | ALP_NOPROGRESS);
        if (error)
                return error;

diff --git a/scrub/repair.c b/scrub/repair.c
index 1ef6372e868def63f56137fc1218f254a0491dc2..bb026101e43853b10010eb6e7881b6ead235d290 100644 (file)
--- a/scrub/repair.c
+++ b/scrub/repair.c
@@ -230,17 +230,30 @@ action_list_process(
        struct action_list              *alist,
        unsigned int                    repair_flags)
 {
+       struct xfs_fd                   xfd;
+       struct xfs_fd                   *xfdp = &ctx->mnt;
        struct action_item              *aitem;
        struct action_item              *n;
        enum check_outcome              fix;
 
+       /*
+        * If the caller passed us a file descriptor for a scrub, use it
+        * instead of scrub-by-handle because this enables the kernel to skip
+        * costly inode btree lookups.
+        */
+       if (fd >= 0) {
+               memcpy(&xfd, xfdp, sizeof(xfd));
+               xfd.fd = fd;
+               xfdp = &xfd;
+       }
+
        if (!alist->sorted) {
                list_sort(NULL, &alist->list, xfs_action_item_compare);
                alist->sorted = true;
        }
 
        list_for_each_entry_safe(aitem, n, &alist->list, list) {
-               fix = xfs_repair_metadata(ctx, fd, aitem, repair_flags);
+               fix = xfs_repair_metadata(ctx, xfdp, aitem, repair_flags);
                switch (fix) {
                case CHECK_DONE:
                        if (!(repair_flags & ALP_NOPROGRESS))
@@ -284,7 +297,7 @@ action_list_process_or_defer(
 {
        int                             ret;
 
-       ret = action_list_process(ctx, ctx->mnt.fd, alist,
+       ret = action_list_process(ctx, -1, alist,
                        ALP_REPAIR_ONLY | ALP_NOPROGRESS);
        if (ret)
                return ret;

diff --git a/scrub/scrub.c b/scrub/scrub.c
index 19a0b2d089f50914ea7c06b3f3df02d78ee8d86a..e83d0d9ce99b0594ea3f28d887833a75ffee1101 100644 (file)
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -611,7 +611,7 @@ xfs_can_repair(
 enum check_outcome
 xfs_repair_metadata(
        struct scrub_ctx                *ctx,
-       int                             fd,
+       struct xfs_fd                   *xfdp,
        struct action_item              *aitem,
        unsigned int                    repair_flags)
 {
@@ -649,7 +649,7 @@ xfs_repair_metadata(
                str_info(ctx, descr_render(&dsc),
                                _("Attempting optimization."));
 
-       error = -xfrog_scrub_metadata(&ctx->mnt, &meta);
+       error = -xfrog_scrub_metadata(xfdp, &meta);
        switch (error) {
        case 0:
                /* No operational errors encountered. */

diff --git a/scrub/scrub.h b/scrub/scrub.h
index 325d8f95ec783fe5e95133cac1c29739caea4e3a..fccd82f21554f89ef1ec709ac29d9f11194470a3 100644 (file)
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -57,7 +57,8 @@ struct action_item {
 /* Complain if still broken even after fix. */
 #define XRM_COMPLAIN_IF_UNFIXED        (1U << 1)
 
-enum check_outcome xfs_repair_metadata(struct scrub_ctx *ctx, int fd,
-               struct action_item *aitem, unsigned int repair_flags);
+enum check_outcome xfs_repair_metadata(struct scrub_ctx *ctx,
+               struct xfs_fd *xfdp, struct action_item *aitem,
+               unsigned int repair_flags);
 
 #endif /* XFS_SCRUB_SCRUB_H_ */