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

While profiling the performance of xfs_scrub, I noticed that phase3 only
employs the scrub-by-handle interface.  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 phase 3 runtime by 5% on
the author's system.

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 868f444d593e3af8dd8a8144a6b8e93715923811..7da112999e7949ee30c931da1f7ded31eb158274 100644 (file)
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -59,16 +59,27 @@ scrub_inode(
        agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino);
        background_sleep();
 
-       /* Try to open the inode to pin it. */
+       /*
+        * Open this regular file to pin it in memory.  Avoiding the use of
+        * scan-by-handle means that the in-kernel scrubber doesn't pay the
+        * cost of opening the handle (looking up the inode in the inode btree,
+        * grabbing the inode, checking the generation) with every scrub call.
+        *
+        * Note: We cannot use this same trick for directories because the VFS
+        * will try to reconnect directory file handles to the root directory
+        * by walking '..' entries upwards, and loops in the dirent index
+        * btree will cause livelocks.
+        *
+        * ESTALE means we scan the whole cluster again.
+        */
        if (S_ISREG(bstat->bs_mode)) {
                fd = scrub_open_handle(handle);
-               /* Stale inode means we scan the whole cluster again. */
                if (fd < 0 && errno == ESTALE)
                        return ESTALE;
        }
 
        /* Scrub the inode. */
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_INODE, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_INODE, &alist);
        if (error)
                goto out;
 
@@ -77,13 +88,13 @@ scrub_inode(
                goto out;
 
        /* Scrub all block mappings. */
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_BMBTD, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_BMBTD, &alist);
        if (error)
                goto out;
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_BMBTA, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_BMBTA, &alist);
        if (error)
                goto out;
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_BMBTC, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_BMBTC, &alist);
        if (error)
                goto out;
 
@@ -93,21 +104,22 @@ scrub_inode(
 
        if (S_ISLNK(bstat->bs_mode)) {
                /* Check symlink contents. */
-               error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_SYMLINK, &alist);
+               error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_SYMLINK,
+                               &alist);
        } else if (S_ISDIR(bstat->bs_mode)) {
                /* Check the directory entries. */
-               error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_DIR, &alist);
+               error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_DIR, &alist);
        }
        if (error)
                goto out;
 
        /* Check all the extended attributes. */
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_XATTR, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_XATTR, &alist);
        if (error)
                goto out;
 
        /* Check parent pointers. */
-       error = scrub_file(ctx, bstat, XFS_SCRUB_TYPE_PARENT, &alist);
+       error = scrub_file(ctx, fd, bstat, XFS_SCRUB_TYPE_PARENT, &alist);
        if (error)
                goto out;
 

diff --git a/scrub/scrub.c b/scrub/scrub.c
index 0034f11dd9a388d03eee48decd2ef4748cf81d78..19a0b2d089f50914ea7c06b3f3df02d78ee8d86a 100644 (file)
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -122,6 +122,7 @@ scrub_warn_incomplete_scrub(
 static enum check_outcome
 xfs_check_metadata(
        struct scrub_ctx                *ctx,
+       struct xfs_fd                   *xfdp,
        struct xfs_scrub_metadata       *meta,
        bool                            is_inode)
 {
@@ -135,7 +136,7 @@ xfs_check_metadata(
 
        dbg_printf("check %s flags %xh\n", descr_render(&dsc), meta->sm_flags);
 retry:
-       error = -xfrog_scrub_metadata(&ctx->mnt, meta);
+       error = -xfrog_scrub_metadata(xfdp, meta);
        if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !error)
                meta->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
        switch (error) {
@@ -316,7 +317,7 @@ scrub_meta_type(
        background_sleep();
 
        /* Check the item. */
-       fix = xfs_check_metadata(ctx, &meta, false);
+       fix = xfs_check_metadata(ctx, &ctx->mnt, &meta, false);
        progress_add(1);
 
        switch (fix) {
@@ -452,11 +453,14 @@ scrub_estimate_ag_work(
 int
 scrub_file(
        struct scrub_ctx                *ctx,
+       int                             fd,
        const struct xfs_bulkstat       *bstat,
        unsigned int                    type,
        struct action_list              *alist)
 {
        struct xfs_scrub_metadata       meta = {0};
+       struct xfs_fd                   xfd;
+       struct xfs_fd                   *xfdp = &ctx->mnt;
        enum check_outcome              fix;
 
        assert(type < XFS_SCRUB_TYPE_NR);
@@ -466,8 +470,19 @@ scrub_file(
        meta.sm_ino = bstat->bs_ino;
        meta.sm_gen = bstat->bs_gen;
 
+       /*
+        * 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;
+       }
+
        /* Scrub the piece of metadata. */
-       fix = xfs_check_metadata(ctx, &meta, true);
+       fix = xfs_check_metadata(ctx, xfdp, &meta, true);
        if (fix == CHECK_ABORT)
                return ECANCELED;
        if (fix == CHECK_DONE)

diff --git a/scrub/scrub.h b/scrub/scrub.h
index 5b5f6b65b095608d35626696c6aa1b0552554940..325d8f95ec783fe5e95133cac1c29739caea4e3a 100644 (file)
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -34,7 +34,7 @@ bool can_scrub_symlink(struct scrub_ctx *ctx);
 bool can_scrub_parent(struct scrub_ctx *ctx);
 bool xfs_can_repair(struct scrub_ctx *ctx);
 
-int scrub_file(struct scrub_ctx *ctx, const struct xfs_bulkstat *bstat,
+int scrub_file(struct scrub_ctx *ctx, int fd, const struct xfs_bulkstat *bstat,
                unsigned int type, struct action_list *alist);
 
 /* Repair parameters are the scrub inputs and retry count. */