[6/7] xfs_scrub: refactor xfs_scrub_excessive_errors
diff mbox series

Message ID 157177016827.1460394.10119847764483927499.stgit@magnolia
State Accepted
Headers show
Series
  • xfs_scrub: clean up error classifications
Related show

Commit Message

Darrick J. Wong Oct. 22, 2019, 6:49 p.m. UTC
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(-)

Comments

Eric Sandeen Nov. 1, 2019, 9:25 p.m. UTC | #1
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 {
>
Darrick J. Wong Nov. 1, 2019, 9:46 p.m. UTC | #2
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 {
> >
Eric Sandeen Nov. 1, 2019, 9:50 p.m. UTC | #3
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>

Patch
diff mbox series

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 {