| Message ID | 157177016827.1460394.10119847764483927499.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs_scrub: clean up error classifications | expand |
On 10/22/19 1:49 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Refactor this helper to avoid cycling the scrub context lock when the > user hasn't configured a maximum error count threshold. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > scrub/common.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > > diff --git a/scrub/common.c b/scrub/common.c > index b1c6abd1..261c6bb2 100644 > --- a/scrub/common.c > +++ b/scrub/common.c > @@ -33,13 +33,20 @@ bool > xfs_scrub_excessive_errors( > struct scrub_ctx *ctx) > { > - bool ret; > + unsigned long long errors_seen; > + > + /* > + * We only set max_errors at the start of the program, so it's safe to > + * access it locklessly. > + */ > + if (ctx->max_errors <= 0) max_errors is an /unsigned/ long long, 'sup w/ the < part? == maybe? > + return false; > > pthread_mutex_lock(&ctx->lock); > - ret = ctx->max_errors > 0 && ctx->corruptions_found >= ctx->max_errors; > + errors_seen = ctx->corruptions_found; > pthread_mutex_unlock(&ctx->lock); > > - return ret; > + return errors_seen >= ctx->max_errors; > } > > static struct { >
On Fri, Nov 01, 2019 at 04:25:55PM -0500, Eric Sandeen wrote: > On 10/22/19 1:49 PM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Refactor this helper to avoid cycling the scrub context lock when the > > user hasn't configured a maximum error count threshold. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > scrub/common.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > diff --git a/scrub/common.c b/scrub/common.c > > index b1c6abd1..261c6bb2 100644 > > --- a/scrub/common.c > > +++ b/scrub/common.c > > @@ -33,13 +33,20 @@ bool > > xfs_scrub_excessive_errors( > > struct scrub_ctx *ctx) > > { > > - bool ret; > > + unsigned long long errors_seen; > > + > > + /* > > + * We only set max_errors at the start of the program, so it's safe to > > + * access it locklessly. > > + */ > > + if (ctx->max_errors <= 0) > > max_errors is an /unsigned/ long long, 'sup w/ the < part? Being thorough. :) > == maybe? Yes, that works. --D > > + return false; > > > > pthread_mutex_lock(&ctx->lock); > > - ret = ctx->max_errors > 0 && ctx->corruptions_found >= ctx->max_errors; > > + errors_seen = ctx->corruptions_found; > > pthread_mutex_unlock(&ctx->lock); > > > > - return ret; > > + return errors_seen >= ctx->max_errors; > > } > > > > static struct { > >
On 10/22/19 1:49 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Refactor this helper to avoid cycling the scrub context lock when the > user hasn't configured a maximum error count threshold. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Ok I'll change the ULL <= 0 thing on the way in. Reviewed-by: Eric Sandeen <sandeen@redhat.com>
diff --git a/scrub/common.c b/scrub/common.c index b1c6abd1..261c6bb2 100644 --- a/scrub/common.c +++ b/scrub/common.c @@ -33,13 +33,20 @@ bool xfs_scrub_excessive_errors( struct scrub_ctx *ctx) { - bool ret; + unsigned long long errors_seen; + + /* + * We only set max_errors at the start of the program, so it's safe to + * access it locklessly. + */ + if (ctx->max_errors <= 0) + return false; pthread_mutex_lock(&ctx->lock); - ret = ctx->max_errors > 0 && ctx->corruptions_found >= ctx->max_errors; + errors_seen = ctx->corruptions_found; pthread_mutex_unlock(&ctx->lock); - return ret; + return errors_seen >= ctx->max_errors; } static struct {