[PATCH 03/18] xfs_scrub: remove moveon from inode iteration

["Darrick J. Wong" <darrick.wong@oracle.com>]

From: Darrick J. Wong <darrick.wong@oracle.com>

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>
---
 scrub/inodes.c |  124 ++++++++++++++++++++++++--------------------------------
 scrub/inodes.h |    6 +--
 scrub/phase3.c |    7 +--
 scrub/phase5.c |   10 ++---
 scrub/phase6.c |    5 +-
 5 files changed, 67 insertions(+), 85 deletions(-)


diff --git a/scrub/inodes.c b/scrub/inodes.c
index 71e53bb6..a2aa6384 100644
--- 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 @@ _("Changed too many times during scan; giving up."));
 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 e58795e7..3affaa00 100644
--- 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 0d2c9019..171be3fd 100644
--- 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 101a0a7a..7f4ae1a8 100644
--- 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 6a8dabe1..c4511b08 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -290,7 +290,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)
@@ -502,7 +502,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. */