xfs_scrub: remove moveon from inode iteration

Replace the moveon retuns in the inode iteration functions with a direct
integer error return.  While we're at it, drop the xfs_ prefix.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/scrub/inodes.c b/scrub/inodes.c
index 71e53bb61136dd47d82578d1bdc8957201dca32a..a2aa63845e15e5a8adad7b51d1bbf485e74916f5 100644 (file)
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -94,54 +94,66 @@ bulkstat_for_inumbers(
        }
 }
 
+/* BULKSTAT wrapper routines. */
+struct scan_inodes {
+       scrub_inode_iter_fn     fn;
+       void                    *arg;
+       bool                    aborted;
+};
+
 /*
  * Call into the filesystem for inode/bulkstat information and call our
  * iterator function.  We'll try to fill the bulkstat information in batches,
  * but we also can detect iget failures.
  */
-static bool
-xfs_iterate_inodes_ag(
-       struct scrub_ctx        *ctx,
-       const char              *descr,
-       void                    *fshandle,
-       uint32_t                agno,
-       xfs_inode_iter_fn       fn,
+static void
+scan_ag_inodes(
+       struct workqueue        *wq,
+       xfs_agnumber_t          agno,
        void                    *arg)
 {
        struct xfs_handle       handle;
+       char                    descr[DESCR_BUFSZ];
        struct xfs_inumbers_req *ireq;
        struct xfs_bulkstat_req *breq;
-       char                    idescr[DESCR_BUFSZ];
+       struct scan_inodes      *si = arg;
+       struct scrub_ctx        *ctx = (struct scrub_ctx *)wq->wq_ctx;
        struct xfs_bulkstat     *bs;
        struct xfs_inumbers     *inumbers;
-       bool                    moveon = true;
        int                     i;
        int                     error;
        int                     stale_count = 0;
 
-       memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+       snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u inodes"),
+                               major(ctx->fsinfo.fs_datadev),
+                               minor(ctx->fsinfo.fs_datadev),
+                               agno);
+
+       memcpy(&handle.ha_fsid, ctx->fshandle, sizeof(handle.ha_fsid));
        handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
                        sizeof(handle.ha_fid.fid_len);
        handle.ha_fid.fid_pad = 0;
 
        breq = xfrog_bulkstat_alloc_req(XFS_INODES_PER_CHUNK, 0);
        if (!breq) {
-               str_liberror(ctx, ENOMEM, _("allocating bulkstat request"));
-               return false;
+               str_errno(ctx, descr);
+               si->aborted = true;
+               return;
        }
 
        ireq = xfrog_inumbers_alloc_req(1, 0);
        if (!ireq) {
-               str_liberror(ctx, ENOMEM, _("allocating inumbers request"));
+               str_errno(ctx, descr);
                free(breq);
-               return false;
+               si->aborted = true;
+               return;
        }
        inumbers = &ireq->inumbers[0];
        xfrog_inumbers_set_ag(ireq, agno);
 
        /* Find the inode chunk & alloc mask */
        error = xfrog_inumbers(&ctx->mnt, ireq);
-       while (!error && ireq->hdr.ocount > 0) {
+       while (!error && !si->aborted && ireq->hdr.ocount > 0) {
                /*
                 * We can have totally empty inode chunks on filesystems where
                 * there are more than 64 inodes per block.  Skip these.
@@ -153,15 +165,17 @@ xfs_iterate_inodes_ag(
 
                /* Iterate all the inodes. */
                for (i = 0, bs = breq->bulkstat;
-                    i < inumbers->xi_alloccount;
+                    !si->aborted && i < inumbers->xi_alloccount;
                     i++, bs++) {
                        handle.ha_fid.fid_ino = bs->bs_ino;
                        handle.ha_fid.fid_gen = bs->bs_gen;
-                       error = fn(ctx, &handle, bs, arg);
+                       error = si->fn(ctx, &handle, bs, si->arg);
                        switch (error) {
                        case 0:
                                break;
-                       case ESTALE:
+                       case ESTALE: {
+                               char    idescr[DESCR_BUFSZ];
+
                                stale_count++;
                                if (stale_count < 30) {
                                        ireq->hdr.ino = inumbers->xi_startino;
@@ -172,16 +186,15 @@ xfs_iterate_inodes_ag(
                                str_info(ctx, idescr,
 _("Changed too many times during scan; giving up."));
                                break;
+                       }
                        case XFS_ITERATE_INODES_ABORT:
                                error = 0;
                                /* fall thru */
                        default:
-                               moveon = false;
-                               errno = error;
                                goto err;
                        }
                        if (xfs_scrub_excessive_errors(ctx)) {
-                               moveon = false;
+                               si->aborted = true;
                                goto out;
                        }
                }
@@ -194,71 +207,42 @@ igrp_retry:
 err:
        if (error) {
                str_liberror(ctx, error, descr);
-               moveon = false;
+               si->aborted = true;
        }
 out:
        free(ireq);
        free(breq);
-       return moveon;
 }
 
-/* BULKSTAT wrapper routines. */
-struct xfs_scan_inodes {
-       xfs_inode_iter_fn       fn;
-       void                    *arg;
-       bool                    moveon;
-};
-
-/* Scan all the inodes in an AG. */
-static void
-xfs_scan_ag_inodes(
-       struct workqueue        *wq,
-       xfs_agnumber_t          agno,
-       void                    *arg)
-{
-       struct xfs_scan_inodes  *si = arg;
-       struct scrub_ctx        *ctx = (struct scrub_ctx *)wq->wq_ctx;
-       char                    descr[DESCR_BUFSZ];
-       bool                    moveon;
-
-       snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u inodes"),
-                               major(ctx->fsinfo.fs_datadev),
-                               minor(ctx->fsinfo.fs_datadev),
-                               agno);
-
-       moveon = xfs_iterate_inodes_ag(ctx, descr, ctx->fshandle, agno,
-                       si->fn, si->arg);
-       if (!moveon)
-               si->moveon = false;
-}
-
-/* Scan all the inodes in a filesystem. */
-bool
-xfs_scan_all_inodes(
+/*
+ * Scan all the inodes in a filesystem.  On error, this function will log
+ * an error message and return -1.
+ */
+int
+scrub_scan_all_inodes(
        struct scrub_ctx        *ctx,
-       xfs_inode_iter_fn       fn,
+       scrub_inode_iter_fn     fn,
        void                    *arg)
 {
-       struct xfs_scan_inodes  si;
+       struct scan_inodes      si = {
+               .fn             = fn,
+               .arg            = arg,
+       };
        xfs_agnumber_t          agno;
        struct workqueue        wq;
        int                     ret;
 
-       si.moveon = true;
-       si.fn = fn;
-       si.arg = arg;
-
        ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
                        scrub_nproc_workqueue(ctx));
        if (ret) {
                str_liberror(ctx, ret, _("creating bulkstat workqueue"));
-               return false;
+               return -1;
        }
 
        for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
-               ret = workqueue_add(&wq, xfs_scan_ag_inodes, agno, &si);
+               ret = workqueue_add(&wq, scan_ag_inodes, agno, &si);
                if (ret) {
-                       si.moveon = false;
+                       si.aborted = true;
                        str_liberror(ctx, ret, _("queueing bulkstat work"));
                        break;
                }
@@ -266,19 +250,17 @@ xfs_scan_all_inodes(
 
        ret = workqueue_terminate(&wq);
        if (ret) {
-               si.moveon = false;
+               si.aborted = true;
                str_liberror(ctx, ret, _("finishing bulkstat work"));
        }
        workqueue_destroy(&wq);
 
-       return si.moveon;
+       return si.aborted ? -1 : 0;
 }
 
-/*
- * Open a file by handle, or return a negative error code.
- */
+/* Open a file by handle, returning either the fd or -1 on error. */
 int
-xfs_open_handle(
+scrub_open_handle(
        struct xfs_handle       *handle)
 {
        return open_by_fshandle(handle, sizeof(*handle),

diff --git a/scrub/inodes.h b/scrub/inodes.h
index e58795e7f4c67b2ffeb7d4da5f10da455d5e2f4f..3affaa0032e95c1b64280c6cfeab686d62415b06 100644 (file)
--- a/scrub/inodes.h
+++ b/scrub/inodes.h
@@ -12,13 +12,13 @@
  * iteration will be restarted from the beginning of the inode allocation
  * group.  Any other non zero value will stop iteration.
  */
-typedef int (*xfs_inode_iter_fn)(struct scrub_ctx *ctx,
+typedef int (*scrub_inode_iter_fn)(struct scrub_ctx *ctx,
                struct xfs_handle *handle, struct xfs_bulkstat *bs, void *arg);
 
 #define XFS_ITERATE_INODES_ABORT       (-1)
-bool xfs_scan_all_inodes(struct scrub_ctx *ctx, xfs_inode_iter_fn fn,
+int scrub_scan_all_inodes(struct scrub_ctx *ctx, scrub_inode_iter_fn fn,
                void *arg);
 
-int xfs_open_handle(struct xfs_handle *handle);
+int scrub_open_handle(struct xfs_handle *handle);
 
 #endif /* XFS_SCRUB_INODES_H_ */

diff --git a/scrub/phase3.c b/scrub/phase3.c
index 0d2c9019bdbeec0330a678fe8a041c2b1a75bb8d..171be3fd883b936e93785ffc976f2d9f7f711e7a 100644 (file)
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -78,7 +78,7 @@ xfs_scrub_inode(
 
        /* Try to open the inode to pin it. */
        if (S_ISREG(bstat->bs_mode)) {
-               fd = xfs_open_handle(handle);
+               fd = scrub_open_handle(handle);
                /* Stale inode means we scan the whole cluster again. */
                if (fd < 0 && errno == ESTALE)
                        return ESTALE;
@@ -161,7 +161,6 @@ xfs_scan_inodes(
        struct scrub_inode_ctx  ictx;
        uint64_t                val;
        int                     err;
-       bool                    ret;
 
        ictx.moveon = true;
        err = ptcounter_alloc(scrub_nproc(ctx), &ictx.icount);
@@ -170,8 +169,8 @@ xfs_scan_inodes(
                return false;
        }
 
-       ret = xfs_scan_all_inodes(ctx, xfs_scrub_inode, &ictx);
-       if (!ret)
+       err = scrub_scan_all_inodes(ctx, xfs_scrub_inode, &ictx);
+       if (err)
                ictx.moveon = false;
        if (!ictx.moveon)
                goto free;

diff --git a/scrub/phase5.c b/scrub/phase5.c
index 101a0a7a4a31e845b1ca8b8975f0e015eaee834d..7f4ae1a81105c990543f8c2cebafa3b9818e8350 100644 (file)
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -264,7 +264,7 @@ xfs_scrub_connections(
 
        /* Open the dir, let the kernel try to reconnect it to the root. */
        if (S_ISDIR(bstat->bs_mode)) {
-               fd = xfs_open_handle(handle);
+               fd = scrub_open_handle(handle);
                if (fd < 0) {
                        if (errno == ESTALE)
                                return ESTALE;
@@ -360,7 +360,7 @@ xfs_scan_connections(
        struct scrub_ctx        *ctx)
 {
        bool                    moveon = true;
-       bool                    ret;
+       int                     ret;
 
        if (ctx->corruptions_found || ctx->unfixable_errors) {
                str_info(ctx, ctx->mntpoint,
@@ -372,9 +372,9 @@ _("Filesystem has errors, skipping connectivity checks."));
        if (!moveon)
                return false;
 
-       ret = xfs_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
-       if (!ret)
-               moveon = false;
+       ret = scrub_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
+       if (ret)
+               return false;
        if (!moveon)
                return false;
        xfs_scrub_report_preen_triggers(ctx);

diff --git a/scrub/phase6.c b/scrub/phase6.c
index f0226218eb6781970154736d989cafea4bfabc3a..9348f8e4677bcdd9965a22c12e81e8c1578ca5ad 100644 (file)
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -288,7 +288,7 @@ xfs_report_verify_inode(
                        bstat->bs_ino, bstat->bs_gen, _("(unlinked)"));
 
        /* Try to open the inode. */
-       fd = xfs_open_handle(handle);
+       fd = scrub_open_handle(handle);
        if (fd < 0) {
                error = errno;
                if (error == ESTALE)
@@ -500,7 +500,8 @@ report_all_media_errors(
                return false;
 
        /* Scan for unlinked files. */
-       return xfs_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
+       ret = scrub_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
+       return ret == 0;
 }
 
 /* Schedule a read-verify of a (data block) extent. */