diff mbox series

[14/36] xfs_scrub: don't expose internal pool state

Message ID 155259751554.31886.1451967052557022474.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfsprogs-5.0: fix various problems | expand

Commit Message

Darrick J. Wong March 14, 2019, 9:05 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

In xfs_scrub, the read/verify pool tries to coalesce the media
verification requests into a smaller number of large IOs.  There's no
need to force callers to keep track of this internal state, so just move
all that into read_verify.c.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/phase6.c      |   23 +++++++----------------
 scrub/read_verify.c |   40 ++++++++++++++++++++++++++++++++++------
 scrub/read_verify.h |   16 ++++------------
 3 files changed, 45 insertions(+), 34 deletions(-)

Comments

Eric Sandeen March 15, 2019, 5:49 p.m. UTC | #1 
On 3/14/19 4:05 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In xfs_scrub, the read/verify pool tries to coalesce the media
> verification requests into a smaller number of large IOs.  There's no
> need to force callers to keep track of this internal state, so just move
> all that into read_verify.c.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

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

> ---
>  scrub/phase6.c      |   23 +++++++----------------
>  scrub/read_verify.c |   40 ++++++++++++++++++++++++++++++++++------
>  scrub/read_verify.h |   16 ++++------------
>  3 files changed, 45 insertions(+), 34 deletions(-)
> 
> 
> diff --git a/scrub/phase6.c b/scrub/phase6.c
> index ead48d77..fe121769 100644
> --- a/scrub/phase6.c
> +++ b/scrub/phase6.c
> @@ -9,7 +9,6 @@
>  #include <sys/statvfs.h>
>  #include "handle.h"
>  #include "path.h"
> -#include "ptvar.h"
>  #include "workqueue.h"
>  #include "xfs_scrub.h"
>  #include "common.h"
> @@ -290,7 +289,6 @@ xfs_report_verify_errors(
>  
>  struct xfs_verify_extent {
>  	struct read_verify_pool	*readverify;
> -	struct ptvar		*rvstate;
>  	struct bitmap		*d_bad;		/* bytes */
>  	struct bitmap		*r_bad;		/* bytes */
>  };
> @@ -424,13 +422,13 @@ xfs_check_rmap(
>  	/* Schedule the read verify command for (eventual) running. */
>  	disk = xfs_dev_to_disk(ctx, map->fmr_device);
>  
> -	read_verify_schedule_io(ve->readverify, ptvar_get(ve->rvstate), disk,
> -			map->fmr_physical, map->fmr_length, ve);
> +	read_verify_schedule_io(ve->readverify, disk, map->fmr_physical,
> +			map->fmr_length, ve);
>  
>  out:
>  	/* Is this the last extent?  Fire off the read. */
>  	if (map->fmr_flags & FMR_OF_LAST)
> -		read_verify_force_io(ve->readverify, ptvar_get(ve->rvstate));
> +		read_verify_force_io(ve->readverify);
>  
>  	return true;
>  }
> @@ -450,16 +448,10 @@ xfs_scan_blocks(
>  	struct xfs_verify_extent	ve;
>  	bool				moveon;
>  
> -	ve.rvstate = ptvar_init(scrub_nproc(ctx), sizeof(struct read_verify));
> -	if (!ve.rvstate) {
> -		str_errno(ctx, ctx->mntpoint);
> -		return false;
> -	}
> -
>  	moveon = bitmap_init(&ve.d_bad);
>  	if (!moveon) {
>  		str_errno(ctx, ctx->mntpoint);
> -		goto out_ve;
> +		goto out;
>  	}
>  
>  	moveon = bitmap_init(&ve.r_bad);
> @@ -469,7 +461,8 @@ xfs_scan_blocks(
>  	}
>  
>  	ve.readverify = read_verify_pool_init(ctx, ctx->geo.blocksize,
> -			xfs_check_rmap_ioerr, disk_heads(ctx->datadev));
> +			xfs_check_rmap_ioerr, disk_heads(ctx->datadev),
> +			scrub_nproc(ctx));
>  	if (!ve.readverify) {
>  		moveon = false;
>  		str_info(ctx, ctx->mntpoint,
> @@ -489,7 +482,6 @@ _("Could not create media verifier."));
>  
>  	bitmap_free(&ve.r_bad);
>  	bitmap_free(&ve.d_bad);
> -	ptvar_free(ve.rvstate);
>  	return moveon;
>  
>  out_pool:
> @@ -498,8 +490,7 @@ _("Could not create media verifier."));
>  	bitmap_free(&ve.r_bad);
>  out_dbad:
>  	bitmap_free(&ve.d_bad);
> -out_ve:
> -	ptvar_free(ve.rvstate);
> +out:
>  	return moveon;
>  }
>  
> diff --git a/scrub/read_verify.c b/scrub/read_verify.c
> index 75cb53ca..b5774736 100644
> --- a/scrub/read_verify.c
> +++ b/scrub/read_verify.c
> @@ -7,6 +7,7 @@
>  #include <stdint.h>
>  #include <stdlib.h>
>  #include <sys/statvfs.h>
> +#include "ptvar.h"
>  #include "workqueue.h"
>  #include "path.h"
>  #include "xfs_scrub.h"
> @@ -36,22 +37,40 @@
>  /* Tolerate 64k holes in adjacent read verify requests. */
>  #define RVP_IO_BATCH_LOCALITY	(65536)
>  
> +struct read_verify {
> +	void			*io_end_arg;
> +	struct disk		*io_disk;
> +	uint64_t		io_start;	/* bytes */
> +	uint64_t		io_length;	/* bytes */
> +};
> +
>  struct read_verify_pool {
>  	struct workqueue	wq;		/* thread pool */
>  	struct scrub_ctx	*ctx;		/* scrub context */
>  	void			*readbuf;	/* read buffer */
>  	struct ptcounter	*verified_bytes;
> +	struct ptvar		*rvstate;	/* combines read requests */
>  	read_verify_ioerr_fn_t	ioerr_fn;	/* io error callback */
>  	size_t			miniosz;	/* minimum io size, bytes */
>  };
>  
> -/* Create a thread pool to run read verifiers. */
> +/*
> + * Create a thread pool to run read verifiers.
> + *
> + * @miniosz is the minimum size of an IO to expect (in bytes).
> + * @ioerr_fn will be called when IO errors occur.
> + * @nproc is the maximum number of verify requests that may be sent to a disk
> + * at any given time.
> + * @submitter_threads is the number of threads that may be sending verify
> + * requests at any given time.
> + */
>  struct read_verify_pool *
>  read_verify_pool_init(
>  	struct scrub_ctx		*ctx,
>  	size_t				miniosz,
>  	read_verify_ioerr_fn_t		ioerr_fn,
> -	unsigned int			nproc)
> +	unsigned int			nproc,
> +	unsigned int			submitter_threads)
>  {
>  	struct read_verify_pool		*rvp;
>  	bool				ret;
> @@ -71,14 +90,20 @@ read_verify_pool_init(
>  	rvp->miniosz = miniosz;
>  	rvp->ctx = ctx;
>  	rvp->ioerr_fn = ioerr_fn;
> +	rvp->rvstate = ptvar_init(submitter_threads,
> +			sizeof(struct read_verify));
> +	if (rvp->rvstate == NULL)
> +		goto out_counter;
>  	/* Run in the main thread if we only want one thread. */
>  	if (nproc == 1)
>  		nproc = 0;
>  	ret = workqueue_create(&rvp->wq, (struct xfs_mount *)rvp, nproc);
>  	if (ret)
> -		goto out_counter;
> +		goto out_rvstate;
>  	return rvp;
>  
> +out_rvstate:
> +	ptvar_free(rvp->rvstate);
>  out_counter:
>  	ptcounter_free(rvp->verified_bytes);
>  out_buf:
> @@ -101,6 +126,7 @@ void
>  read_verify_pool_destroy(
>  	struct read_verify_pool		*rvp)
>  {
> +	ptvar_free(rvp->rvstate);
>  	ptcounter_free(rvp->verified_bytes);
>  	free(rvp->readbuf);
>  	free(rvp);
> @@ -186,16 +212,17 @@ _("Could not queue read-verify work."));
>  bool
>  read_verify_schedule_io(
>  	struct read_verify_pool		*rvp,
> -	struct read_verify		*rv,
>  	struct disk			*disk,
>  	uint64_t			start,
>  	uint64_t			length,
>  	void				*end_arg)
>  {
> +	struct read_verify		*rv;
>  	uint64_t			req_end;
>  	uint64_t			rv_end;
>  
>  	assert(rvp->readbuf);
> +	rv = ptvar_get(rvp->rvstate);
>  	req_end = start + length;
>  	rv_end = rv->io_start + rv->io_length;
>  
> @@ -229,12 +256,13 @@ read_verify_schedule_io(
>  /* Force any stashed IOs into the verifier. */
>  bool
>  read_verify_force_io(
> -	struct read_verify_pool		*rvp,
> -	struct read_verify		*rv)
> +	struct read_verify_pool		*rvp)
>  {
> +	struct read_verify		*rv;
>  	bool				moveon;
>  
>  	assert(rvp->readbuf);
> +	rv = ptvar_get(rvp->rvstate);
>  	if (rv->io_length == 0)
>  		return true;
>  
> diff --git a/scrub/read_verify.h b/scrub/read_verify.h
> index 38f1cd1a..1e7fd83f 100644
> --- a/scrub/read_verify.h
> +++ b/scrub/read_verify.h
> @@ -16,21 +16,13 @@ typedef void (*read_verify_ioerr_fn_t)(struct scrub_ctx *ctx,
>  
>  struct read_verify_pool *read_verify_pool_init(struct scrub_ctx *ctx,
>  		size_t miniosz, read_verify_ioerr_fn_t ioerr_fn,
> -		unsigned int nproc);
> +		unsigned int nproc, unsigned int submitter_threads);
>  void read_verify_pool_flush(struct read_verify_pool *rvp);
>  void read_verify_pool_destroy(struct read_verify_pool *rvp);
>  
> -struct read_verify {
> -	void			*io_end_arg;
> -	struct disk		*io_disk;
> -	uint64_t		io_start;	/* bytes */
> -	uint64_t		io_length;	/* bytes */
> -};
> -
> -bool read_verify_schedule_io(struct read_verify_pool *rvp,
> -		struct read_verify *rv, struct disk *disk, uint64_t start,
> -		uint64_t length, void *end_arg);
> -bool read_verify_force_io(struct read_verify_pool *rvp, struct read_verify *rv);
> +bool read_verify_schedule_io(struct read_verify_pool *rvp, struct disk *disk,
> +		uint64_t start, uint64_t length, void *end_arg);
> +bool read_verify_force_io(struct read_verify_pool *rvp);
>  uint64_t read_verify_bytes(struct read_verify_pool *rvp);
>  
>  #endif /* XFS_SCRUB_READ_VERIFY_H_ */
>
diff mbox series

Patch

diff --git a/scrub/phase6.c b/scrub/phase6.c
index ead48d77..fe121769 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -9,7 +9,6 @@ 
 #include <sys/statvfs.h>
 #include "handle.h"
 #include "path.h"
-#include "ptvar.h"
 #include "workqueue.h"
 #include "xfs_scrub.h"
 #include "common.h"
@@ -290,7 +289,6 @@  xfs_report_verify_errors(
 
 struct xfs_verify_extent {
 	struct read_verify_pool	*readverify;
-	struct ptvar		*rvstate;
 	struct bitmap		*d_bad;		/* bytes */
 	struct bitmap		*r_bad;		/* bytes */
 };
@@ -424,13 +422,13 @@  xfs_check_rmap(
 	/* Schedule the read verify command for (eventual) running. */
 	disk = xfs_dev_to_disk(ctx, map->fmr_device);
 
-	read_verify_schedule_io(ve->readverify, ptvar_get(ve->rvstate), disk,
-			map->fmr_physical, map->fmr_length, ve);
+	read_verify_schedule_io(ve->readverify, disk, map->fmr_physical,
+			map->fmr_length, ve);
 
 out:
 	/* Is this the last extent?  Fire off the read. */
 	if (map->fmr_flags & FMR_OF_LAST)
-		read_verify_force_io(ve->readverify, ptvar_get(ve->rvstate));
+		read_verify_force_io(ve->readverify);
 
 	return true;
 }
@@ -450,16 +448,10 @@  xfs_scan_blocks(
 	struct xfs_verify_extent	ve;
 	bool				moveon;
 
-	ve.rvstate = ptvar_init(scrub_nproc(ctx), sizeof(struct read_verify));
-	if (!ve.rvstate) {
-		str_errno(ctx, ctx->mntpoint);
-		return false;
-	}
-
 	moveon = bitmap_init(&ve.d_bad);
 	if (!moveon) {
 		str_errno(ctx, ctx->mntpoint);
-		goto out_ve;
+		goto out;
 	}
 
 	moveon = bitmap_init(&ve.r_bad);
@@ -469,7 +461,8 @@  xfs_scan_blocks(
 	}
 
 	ve.readverify = read_verify_pool_init(ctx, ctx->geo.blocksize,
-			xfs_check_rmap_ioerr, disk_heads(ctx->datadev));
+			xfs_check_rmap_ioerr, disk_heads(ctx->datadev),
+			scrub_nproc(ctx));
 	if (!ve.readverify) {
 		moveon = false;
 		str_info(ctx, ctx->mntpoint,
@@ -489,7 +482,6 @@  _("Could not create media verifier."));
 
 	bitmap_free(&ve.r_bad);
 	bitmap_free(&ve.d_bad);
-	ptvar_free(ve.rvstate);
 	return moveon;
 
 out_pool:
@@ -498,8 +490,7 @@  _("Could not create media verifier."));
 	bitmap_free(&ve.r_bad);
 out_dbad:
 	bitmap_free(&ve.d_bad);
-out_ve:
-	ptvar_free(ve.rvstate);
+out:
 	return moveon;
 }
 
diff --git a/scrub/read_verify.c b/scrub/read_verify.c
index 75cb53ca..b5774736 100644
--- a/scrub/read_verify.c
+++ b/scrub/read_verify.c
@@ -7,6 +7,7 @@ 
 #include <stdint.h>
 #include <stdlib.h>
 #include <sys/statvfs.h>
+#include "ptvar.h"
 #include "workqueue.h"
 #include "path.h"
 #include "xfs_scrub.h"
@@ -36,22 +37,40 @@ 
 /* Tolerate 64k holes in adjacent read verify requests. */
 #define RVP_IO_BATCH_LOCALITY	(65536)
 
+struct read_verify {
+	void			*io_end_arg;
+	struct disk		*io_disk;
+	uint64_t		io_start;	/* bytes */
+	uint64_t		io_length;	/* bytes */
+};
+
 struct read_verify_pool {
 	struct workqueue	wq;		/* thread pool */
 	struct scrub_ctx	*ctx;		/* scrub context */
 	void			*readbuf;	/* read buffer */
 	struct ptcounter	*verified_bytes;
+	struct ptvar		*rvstate;	/* combines read requests */
 	read_verify_ioerr_fn_t	ioerr_fn;	/* io error callback */
 	size_t			miniosz;	/* minimum io size, bytes */
 };
 
-/* Create a thread pool to run read verifiers. */
+/*
+ * Create a thread pool to run read verifiers.
+ *
+ * @miniosz is the minimum size of an IO to expect (in bytes).
+ * @ioerr_fn will be called when IO errors occur.
+ * @nproc is the maximum number of verify requests that may be sent to a disk
+ * at any given time.
+ * @submitter_threads is the number of threads that may be sending verify
+ * requests at any given time.
+ */
 struct read_verify_pool *
 read_verify_pool_init(
 	struct scrub_ctx		*ctx,
 	size_t				miniosz,
 	read_verify_ioerr_fn_t		ioerr_fn,
-	unsigned int			nproc)
+	unsigned int			nproc,
+	unsigned int			submitter_threads)
 {
 	struct read_verify_pool		*rvp;
 	bool				ret;
@@ -71,14 +90,20 @@  read_verify_pool_init(
 	rvp->miniosz = miniosz;
 	rvp->ctx = ctx;
 	rvp->ioerr_fn = ioerr_fn;
+	rvp->rvstate = ptvar_init(submitter_threads,
+			sizeof(struct read_verify));
+	if (rvp->rvstate == NULL)
+		goto out_counter;
 	/* Run in the main thread if we only want one thread. */
 	if (nproc == 1)
 		nproc = 0;
 	ret = workqueue_create(&rvp->wq, (struct xfs_mount *)rvp, nproc);
 	if (ret)
-		goto out_counter;
+		goto out_rvstate;
 	return rvp;
 
+out_rvstate:
+	ptvar_free(rvp->rvstate);
 out_counter:
 	ptcounter_free(rvp->verified_bytes);
 out_buf:
@@ -101,6 +126,7 @@  void
 read_verify_pool_destroy(
 	struct read_verify_pool		*rvp)
 {
+	ptvar_free(rvp->rvstate);
 	ptcounter_free(rvp->verified_bytes);
 	free(rvp->readbuf);
 	free(rvp);
@@ -186,16 +212,17 @@  _("Could not queue read-verify work."));
 bool
 read_verify_schedule_io(
 	struct read_verify_pool		*rvp,
-	struct read_verify		*rv,
 	struct disk			*disk,
 	uint64_t			start,
 	uint64_t			length,
 	void				*end_arg)
 {
+	struct read_verify		*rv;
 	uint64_t			req_end;
 	uint64_t			rv_end;
 
 	assert(rvp->readbuf);
+	rv = ptvar_get(rvp->rvstate);
 	req_end = start + length;
 	rv_end = rv->io_start + rv->io_length;
 
@@ -229,12 +256,13 @@  read_verify_schedule_io(
 /* Force any stashed IOs into the verifier. */
 bool
 read_verify_force_io(
-	struct read_verify_pool		*rvp,
-	struct read_verify		*rv)
+	struct read_verify_pool		*rvp)
 {
+	struct read_verify		*rv;
 	bool				moveon;
 
 	assert(rvp->readbuf);
+	rv = ptvar_get(rvp->rvstate);
 	if (rv->io_length == 0)
 		return true;
 
diff --git a/scrub/read_verify.h b/scrub/read_verify.h
index 38f1cd1a..1e7fd83f 100644
--- a/scrub/read_verify.h
+++ b/scrub/read_verify.h
@@ -16,21 +16,13 @@  typedef void (*read_verify_ioerr_fn_t)(struct scrub_ctx *ctx,
 
 struct read_verify_pool *read_verify_pool_init(struct scrub_ctx *ctx,
 		size_t miniosz, read_verify_ioerr_fn_t ioerr_fn,
-		unsigned int nproc);
+		unsigned int nproc, unsigned int submitter_threads);
 void read_verify_pool_flush(struct read_verify_pool *rvp);
 void read_verify_pool_destroy(struct read_verify_pool *rvp);
 
-struct read_verify {
-	void			*io_end_arg;
-	struct disk		*io_disk;
-	uint64_t		io_start;	/* bytes */
-	uint64_t		io_length;	/* bytes */
-};
-
-bool read_verify_schedule_io(struct read_verify_pool *rvp,
-		struct read_verify *rv, struct disk *disk, uint64_t start,
-		uint64_t length, void *end_arg);
-bool read_verify_force_io(struct read_verify_pool *rvp, struct read_verify *rv);
+bool read_verify_schedule_io(struct read_verify_pool *rvp, struct disk *disk,
+		uint64_t start, uint64_t length, void *end_arg);
+bool read_verify_force_io(struct read_verify_pool *rvp);
 uint64_t read_verify_bytes(struct read_verify_pool *rvp);
 
 #endif /* XFS_SCRUB_READ_VERIFY_H_ */