[PATCH 0/2] xfs_scrub: random fixes

[PATCH 0/2] xfs_scrub: random fixes

[Darrick J. Wong]

Hi all,

A couple of small cleanups and refactorings to the scrub code that don't
fit elsewhere.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=scrub-random-fixes

[PATCH 1/2] xfs_scrub: clean out the nproc global variable

[Darrick J. Wong]

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

Get rid of this global variable since we already have a libfrog function
that does exactly what it does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
 scrub/disk.c      |    2 ++
 scrub/xfs_scrub.c |    8 --------
 scrub/xfs_scrub.h |    1 -
 3 files changed, 2 insertions(+), 9 deletions(-)


diff --git a/scrub/disk.c b/scrub/disk.c
index 8a8a411b..a1ef798a 100644
--- a/scrub/disk.c
+++ b/scrub/disk.c
@@ -22,6 +22,7 @@
 #include "xfs_scrub.h"
 #include "common.h"
 #include "disk.h"
+#include "platform_defs.h"
 
 #ifndef BLKROTATIONAL
 # define BLKROTATIONAL	_IO(0x12, 126)
@@ -42,6 +43,7 @@ __disk_heads(
 {
 	int			iomin;
 	int			ioopt;
+	int			nproc = platform_nproc();
 	unsigned short		rot;
 	int			error;
 
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index b6a01274..147c114c 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -131,9 +131,6 @@ static bool			display_rusage;
 /* Background mode; higher values insert more pauses between scrub calls. */
 unsigned int			bg_mode;
 
-/* Maximum number of processors available to us. */
-int				nproc;
-
 /* Number of threads we're allowed to use. */
 unsigned int			force_nr_threads;
 
@@ -717,11 +714,6 @@ main(
 	}
 	memcpy(&ctx.fsinfo, fsp, sizeof(struct fs_path));
 
-	/* How many CPUs? */
-	nproc = sysconf(_SC_NPROCESSORS_ONLN);
-	if (nproc < 1)
-		nproc = 1;
-
 	/* Set up a page-aligned buffer for read verification. */
 	page_size = sysconf(_SC_PAGESIZE);
 	if (page_size < 0) {
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index f9a72052..37d78f61 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -15,7 +15,6 @@ extern char *progname;
 extern unsigned int		force_nr_threads;
 extern unsigned int		bg_mode;
 extern unsigned int		debug;
-extern int			nproc;
 extern bool			verbose;
 extern long			page_size;
 extern bool			want_fstrim;

[PATCH 2/2] xfs_scrub: refactor xfs_iterate_inodes_range_check

[Darrick J. Wong]

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

Move all the bulkstat action into a single helper function.  This gets
rid of the awkward name and increases cohesion.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/inodes.c |   40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)


diff --git a/scrub/inodes.c b/scrub/inodes.c
index 7aa61ebe..7c04d7f6 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -43,19 +43,37 @@
  */
 
 /*
- * Did we get exactly the inodes we expected?  If not, load them one at a
- * time (or fake it) into the bulkstat data.
+ * Run bulkstat on an entire inode allocation group, then check that we got
+ * exactly the inodes we expected.  If not, load them one at a time (or fake
+ * it) into the bulkstat data.
  */
 static void
-xfs_iterate_inodes_range_check(
+bulkstat_for_inumbers(
 	struct scrub_ctx	*ctx,
-	struct xfs_inumbers	*inumbers,
-	struct xfs_bulkstat	*bstat)
+	const char		*descr,
+	const struct xfs_inumbers *inumbers,
+	struct xfs_bulkstat_req	*breq)
 {
+	struct xfs_bulkstat	*bstat = breq->bulkstat;
 	struct xfs_bulkstat	*bs;
 	int			i;
 	int			error;
 
+	/* First we try regular bulkstat, for speed. */
+	breq->hdr.ino = inumbers->xi_startino;
+	breq->hdr.icount = inumbers->xi_alloccount;
+	error = xfrog_bulkstat(&ctx->mnt, breq);
+	if (error) {
+		char	errbuf[DESCR_BUFSZ];
+
+		str_info(ctx, descr, "%s",
+			 strerror_r(error, errbuf, DESCR_BUFSZ));
+	}
+
+	/*
+	 * Check each of the stats we got back to make sure we got the inodes
+	 * we asked for.
+	 */
 	for (i = 0, bs = bstat; i < XFS_INODES_PER_CHUNK; i++) {
 		if (!(inumbers->xi_allocmask & (1ULL << i)))
 			continue;
@@ -131,17 +149,7 @@ xfs_iterate_inodes_ag(
 		if (inumbers->xi_alloccount == 0)
 			goto igrp_retry;
 
-		breq->hdr.ino = inumbers->xi_startino;
-		breq->hdr.icount = inumbers->xi_alloccount;
-		error = xfrog_bulkstat(&ctx->mnt, breq);
-		if (error) {
-			char	errbuf[DESCR_BUFSZ];
-
-			str_info(ctx, descr, "%s", strerror_r(error,
-						errbuf, DESCR_BUFSZ));
-		}
-
-		xfs_iterate_inodes_range_check(ctx, inumbers, breq->bulkstat);
+		bulkstat_for_inumbers(ctx, descr, inumbers, breq);
 
 		/* Iterate all the inodes. */
 		for (i = 0, bs = breq->bulkstat;

Re: [PATCH 2/2] xfs_scrub: refactor xfs_iterate_inodes_range_check

[Eric Sandeen]

On 10/22/19 1:48 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move all the bulkstat action into a single helper function.  This gets
> rid of the awkward name and increases cohesion.

mmm, cohesion.

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>