diff mbox series

[46/50] xfs_scrub: call GETFSMAP for each rt group in parallel

Message ID 173352752648.126362.13619225422874515961.stgit@frogsfrogsfrogs (mailing list archive)
State Not Applicable, archived
Headers show
Series [01/50] libxfs: remove XFS_ILOCK_RT* | expand

Commit Message

Darrick J. Wong Dec. 7, 2024, 12:16 a.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

If realtime groups are enabled, we should take advantage of the sharding
to speed up the spacemap scans.  Do so by issuing per-rtgroup GETFSMAP
calls.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 scrub/spacemap.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 8 deletions(-)

Comments

Christoph Hellwig Dec. 10, 2024, 6:04 a.m. UTC | #1 
> +	struct fsmap		keys[2];
> +	off_t			bperrg = bytes_per_rtgroup(&ctx->mnt.fsgeom);
> +	int			ret;
> +
> +
> +	memset(keys, 0, sizeof(struct fsmap) * 2);

This could be simplified to 

	struct fsmap		keys[2] = { };

instead of the manual initialization.

> +	keys->fmr_device = ctx->fsinfo.fs_rtdev;
> +	keys->fmr_physical = (xfs_rtblock_t)rgno * bperrg;
> +	(keys + 1)->fmr_device = ctx->fsinfo.fs_rtdev;
> +	(keys + 1)->fmr_physical = ((rgno + 1) * bperrg) - 1;
> +	(keys + 1)->fmr_owner = ULLONG_MAX;
> +	(keys + 1)->fmr_offset = ULLONG_MAX;
> +	(keys + 1)->fmr_flags = UINT_MAX;

The usage of keys here and various other places looks really odd.
It's an array, so doing pointer math instad of the simple

	
	keys[0].fmr_device = ctx->fsinfo.fs_rtdev;

	keys[1].fmr_device = ctx->fsinfo.fs_rtdev;

..

is rather confusing.  I've actually cleaned this up but forgot to
send it to you.  Feel free to grab the patch here:

http://git.infradead.org/?p=users/hch/xfsprogs.git;a=commitdiff;h=5699e03cf03e6b1189a89f903631046d16980ff6

and fold it in.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Darrick J. Wong Dec. 11, 2024, 10:18 p.m. UTC | #2 
On Mon, Dec 09, 2024 at 10:04:54PM -0800, Christoph Hellwig wrote:
> > +	struct fsmap		keys[2];
> > +	off_t			bperrg = bytes_per_rtgroup(&ctx->mnt.fsgeom);
> > +	int			ret;
> > +
> > +
> > +	memset(keys, 0, sizeof(struct fsmap) * 2);
> 
> This could be simplified to 
> 
> 	struct fsmap		keys[2] = { };
> 
> instead of the manual initialization.
> 
> > +	keys->fmr_device = ctx->fsinfo.fs_rtdev;
> > +	keys->fmr_physical = (xfs_rtblock_t)rgno * bperrg;
> > +	(keys + 1)->fmr_device = ctx->fsinfo.fs_rtdev;
> > +	(keys + 1)->fmr_physical = ((rgno + 1) * bperrg) - 1;
> > +	(keys + 1)->fmr_owner = ULLONG_MAX;
> > +	(keys + 1)->fmr_offset = ULLONG_MAX;
> > +	(keys + 1)->fmr_flags = UINT_MAX;
> 
> The usage of keys here and various other places looks really odd.
> It's an array, so doing pointer math instad of the simple
> 
> 	
> 	keys[0].fmr_device = ctx->fsinfo.fs_rtdev;
> 
> 	keys[1].fmr_device = ctx->fsinfo.fs_rtdev;
> 
> ..
> 
> is rather confusing.  I've actually cleaned this up but forgot to
> send it to you.  Feel free to grab the patch here:
> 
> http://git.infradead.org/?p=users/hch/xfsprogs.git;a=commitdiff;h=5699e03cf03e6b1189a89f903631046d16980ff6
> 
> and fold it in.

Folded, thanks for the cleanup.

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!  I've folded the scan_rtg_rmaps cleanup into this patch but left
the other two as a separate patch.

--D
diff mbox series

Patch

diff --git a/scrub/spacemap.c b/scrub/spacemap.c
index e35756db2eed43..5b6bad138ce502 100644
--- a/scrub/spacemap.c
+++ b/scrub/spacemap.c
@@ -128,6 +128,45 @@  scan_ag_rmaps(
 	}
 }
 
+/* Iterate all the reverse mappings of a realtime group. */
+static void
+scan_rtg_rmaps(
+	struct workqueue	*wq,
+	xfs_agnumber_t		rgno,
+	void			*arg)
+{
+	struct scrub_ctx	*ctx = (struct scrub_ctx *)wq->wq_ctx;
+	struct scan_blocks	*sbx = arg;
+	struct fsmap		keys[2];
+	off_t			bperrg = bytes_per_rtgroup(&ctx->mnt.fsgeom);
+	int			ret;
+
+
+	memset(keys, 0, sizeof(struct fsmap) * 2);
+	keys->fmr_device = ctx->fsinfo.fs_rtdev;
+	keys->fmr_physical = (xfs_rtblock_t)rgno * bperrg;
+	(keys + 1)->fmr_device = ctx->fsinfo.fs_rtdev;
+	(keys + 1)->fmr_physical = ((rgno + 1) * bperrg) - 1;
+	(keys + 1)->fmr_owner = ULLONG_MAX;
+	(keys + 1)->fmr_offset = ULLONG_MAX;
+	(keys + 1)->fmr_flags = UINT_MAX;
+
+	if (sbx->aborted)
+		return;
+
+	ret = scrub_iterate_fsmap(ctx, keys, sbx->fn, sbx->arg);
+	if (ret) {
+		char		descr[DESCR_BUFSZ];
+
+		snprintf(descr, DESCR_BUFSZ, _("dev %d:%d rtgroup %u fsmap"),
+					major(ctx->fsinfo.fs_datadev),
+					minor(ctx->fsinfo.fs_datadev),
+					rgno);
+		str_liberror(ctx, ret, descr);
+		sbx->aborted = true;
+	}
+}
+
 /* Iterate all the reverse mappings of a standalone device. */
 static void
 scan_dev_rmaps(
@@ -208,14 +247,6 @@  scrub_scan_all_spacemaps(
 		str_liberror(ctx, ret, _("creating fsmap workqueue"));
 		return ret;
 	}
-	if (ctx->fsinfo.fs_rt) {
-		ret = -workqueue_add(&wq, scan_rt_rmaps, 0, &sbx);
-		if (ret) {
-			sbx.aborted = true;
-			str_liberror(ctx, ret, _("queueing rtdev fsmap work"));
-			goto out;
-		}
-	}
 	if (ctx->fsinfo.fs_log) {
 		ret = -workqueue_add(&wq, scan_log_rmaps, 0, &sbx);
 		if (ret) {
@@ -232,6 +263,31 @@  scrub_scan_all_spacemaps(
 			break;
 		}
 	}
+	if (ctx->fsinfo.fs_rt) {
+		for (agno = 0; agno < ctx->mnt.fsgeom.rgcount; agno++) {
+			ret = -workqueue_add(&wq, scan_rtg_rmaps, agno, &sbx);
+			if (ret) {
+				sbx.aborted = true;
+				str_liberror(ctx, ret,
+						_("queueing rtgroup fsmap work"));
+				break;
+			}
+		}
+
+		/*
+		 * If the fs doesn't have any realtime groups, scan the entire
+		 * volume all at once, since the above loop did nothing.
+		 */
+		if (ctx->mnt.fsgeom.rgcount == 0) {
+			ret = -workqueue_add(&wq, scan_rt_rmaps, 0, &sbx);
+			if (ret) {
+				sbx.aborted = true;
+				str_liberror(ctx, ret,
+						_("queueing rtdev fsmap work"));
+				goto out;
+			}
+		}
+	}
 out:
 	ret = -workqueue_terminate(&wq);
 	if (ret) {