errseq: Eliminate special limitation for macro MAX_ERRNO

[PATCH] errseq: Eliminate special limitation for macro MAX_ERRNO

Posted by Zijun Hu 10 months ago

From: Zijun Hu <quic_zijuhu@quicinc.com>

Current errseq implementation depends on a very special precondition
that macro MAX_ERRNO must be (2^n - 1).

Eliminate the limitation by
- redefine macro ERRSEQ_SHIFT.
- define a new macro ERRNO_MASK instead of MAX_ERRNO for errno mask.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 lib/errseq.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/errseq.c b/lib/errseq.c
index 93e9b94358dc63dcc911fd45a01ccf38d2104ecf..13a2581c5a878445f8a089d0d34e901f77a9e074 100644
--- a/lib/errseq.c
+++ b/lib/errseq.c
@@ -34,11 +34,14 @@
  */
 
 /* The low bits are designated for error code (max of MAX_ERRNO) */
-#define ERRSEQ_SHIFT		ilog2(MAX_ERRNO + 1)
+#define ERRSEQ_SHIFT		(ilog2(MAX_ERRNO) + 1)
 
 /* This bit is used as a flag to indicate whether the value has been seen */
 #define ERRSEQ_SEEN		(1 << ERRSEQ_SHIFT)
 
+/* Leverage macro ERRSEQ_SEEN to define errno mask macro here */
+#define ERRNO_MASK		(ERRSEQ_SEEN - 1)
+
 /* The lowest bit of the counter */
 #define ERRSEQ_CTR_INC		(1 << (ERRSEQ_SHIFT + 1))
 
@@ -60,8 +63,6 @@ errseq_t errseq_set(errseq_t *eseq, int err)
 {
 	errseq_t cur, old;
 
-	/* MAX_ERRNO must be able to serve as a mask */
-	BUILD_BUG_ON_NOT_POWER_OF_2(MAX_ERRNO + 1);
 
 	/*
 	 * Ensure the error code actually fits where we want it to go. If it
@@ -79,7 +80,7 @@ errseq_t errseq_set(errseq_t *eseq, int err)
 		errseq_t new;
 
 		/* Clear out error bits and set new error */
-		new = (old & ~(MAX_ERRNO|ERRSEQ_SEEN)) | -err;
+		new = (old & ~(ERRNO_MASK | ERRSEQ_SEEN)) | -err;
 
 		/* Only increment if someone has looked at it */
 		if (old & ERRSEQ_SEEN)
@@ -148,7 +149,7 @@ int errseq_check(errseq_t *eseq, errseq_t since)
 
 	if (likely(cur == since))
 		return 0;
-	return -(cur & MAX_ERRNO);
+	return -(cur & ERRNO_MASK);
 }
 EXPORT_SYMBOL(errseq_check);
 
@@ -200,7 +201,7 @@ int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
 		if (new != old)
 			cmpxchg(eseq, old, new);
 		*since = new;
-		err = -(new & MAX_ERRNO);
+		err = -(new & ERRNO_MASK);
 	}
 	return err;
 }

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250407-improve_errseq-8cfa1539f9e9

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

Re: [PATCH] errseq: Eliminate special limitation for macro MAX_ERRNO

Posted by Jeff Layton 10 months ago

On Mon, 2025-04-07 at 19:44 +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Current errseq implementation depends on a very special precondition
> that macro MAX_ERRNO must be (2^n - 1).
> 
> Eliminate the limitation by
> - redefine macro ERRSEQ_SHIFT.
> - define a new macro ERRNO_MASK instead of MAX_ERRNO for errno mask.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  lib/errseq.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/errseq.c b/lib/errseq.c
> index 93e9b94358dc63dcc911fd45a01ccf38d2104ecf..13a2581c5a878445f8a089d0d34e901f77a9e074 100644
> --- a/lib/errseq.c
> +++ b/lib/errseq.c
> @@ -34,11 +34,14 @@
>   */
>  
>  /* The low bits are designated for error code (max of MAX_ERRNO) */
> -#define ERRSEQ_SHIFT		ilog2(MAX_ERRNO + 1)
> +#define ERRSEQ_SHIFT		(ilog2(MAX_ERRNO) + 1)
>  
>  /* This bit is used as a flag to indicate whether the value has been seen */
>  #define ERRSEQ_SEEN		(1 << ERRSEQ_SHIFT)
>  
> +/* Leverage macro ERRSEQ_SEEN to define errno mask macro here */
> +#define ERRNO_MASK		(ERRSEQ_SEEN - 1)
> +
>  /* The lowest bit of the counter */
>  #define ERRSEQ_CTR_INC		(1 << (ERRSEQ_SHIFT + 1))
>  
> @@ -60,8 +63,6 @@ errseq_t errseq_set(errseq_t *eseq, int err)
>  {
>  	errseq_t cur, old;
>  
> -	/* MAX_ERRNO must be able to serve as a mask */
> -	BUILD_BUG_ON_NOT_POWER_OF_2(MAX_ERRNO + 1);
>  
>  	/*
>  	 * Ensure the error code actually fits where we want it to go. If it
> @@ -79,7 +80,7 @@ errseq_t errseq_set(errseq_t *eseq, int err)
>  		errseq_t new;
>  
>  		/* Clear out error bits and set new error */
> -		new = (old & ~(MAX_ERRNO|ERRSEQ_SEEN)) | -err;
> +		new = (old & ~(ERRNO_MASK | ERRSEQ_SEEN)) | -err;
>  
>  		/* Only increment if someone has looked at it */
>  		if (old & ERRSEQ_SEEN)
> @@ -148,7 +149,7 @@ int errseq_check(errseq_t *eseq, errseq_t since)
>  
>  	if (likely(cur == since))
>  		return 0;
> -	return -(cur & MAX_ERRNO);
> +	return -(cur & ERRNO_MASK);
>  }
>  EXPORT_SYMBOL(errseq_check);
>  
> @@ -200,7 +201,7 @@ int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
>  		if (new != old)
>  			cmpxchg(eseq, old, new);
>  		*since = new;
> -		err = -(new & MAX_ERRNO);
> +		err = -(new & ERRNO_MASK);
>  	}
>  	return err;
>  }
> 
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-improve_errseq-8cfa1539f9e9
> 
> Best regards,

Looks sane to me, and it gets rid of the BUILD_BUG_ON.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH] errseq: Eliminate special limitation for macro MAX_ERRNO

Posted by Jeff Layton 10 months ago

On Mon, 2025-04-07 at 19:44 +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Current errseq implementation depends on a very special precondition
> that macro MAX_ERRNO must be (2^n - 1).
> 
> Eliminate the limitation by
> - redefine macro ERRSEQ_SHIFT.
> - define a new macro ERRNO_MASK instead of MAX_ERRNO for errno mask.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  lib/errseq.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/errseq.c b/lib/errseq.c
> index 93e9b94358dc63dcc911fd45a01ccf38d2104ecf..13a2581c5a878445f8a089d0d34e901f77a9e074 100644
> --- a/lib/errseq.c
> +++ b/lib/errseq.c
> @@ -34,11 +34,14 @@
>   */
>  
>  /* The low bits are designated for error code (max of MAX_ERRNO) */
> -#define ERRSEQ_SHIFT		ilog2(MAX_ERRNO + 1)
> +#define ERRSEQ_SHIFT		(ilog2(MAX_ERRNO) + 1)
>  
>  /* This bit is used as a flag to indicate whether the value has been seen */
>  #define ERRSEQ_SEEN		(1 << ERRSEQ_SHIFT)
>  
> +/* Leverage macro ERRSEQ_SEEN to define errno mask macro here */
> +#define ERRNO_MASK		(ERRSEQ_SEEN - 1)
> +
>  /* The lowest bit of the counter */
>  #define ERRSEQ_CTR_INC		(1 << (ERRSEQ_SHIFT + 1))
>  
> @@ -60,8 +63,6 @@ errseq_t errseq_set(errseq_t *eseq, int err)
>  {
>  	errseq_t cur, old;
>  
> -	/* MAX_ERRNO must be able to serve as a mask */
> -	BUILD_BUG_ON_NOT_POWER_OF_2(MAX_ERRNO + 1);
>  
>  	/*
>  	 * Ensure the error code actually fits where we want it to go. If it
> @@ -79,7 +80,7 @@ errseq_t errseq_set(errseq_t *eseq, int err)
>  		errseq_t new;
>  
>  		/* Clear out error bits and set new error */
> -		new = (old & ~(MAX_ERRNO|ERRSEQ_SEEN)) | -err;
> +		new = (old & ~(ERRNO_MASK | ERRSEQ_SEEN)) | -err;
>  
>  		/* Only increment if someone has looked at it */
>  		if (old & ERRSEQ_SEEN)
> @@ -148,7 +149,7 @@ int errseq_check(errseq_t *eseq, errseq_t since)
>  
>  	if (likely(cur == since))
>  		return 0;
> -	return -(cur & MAX_ERRNO);
> +	return -(cur & ERRNO_MASK);
>  }
>  EXPORT_SYMBOL(errseq_check);
>  
> @@ -200,7 +201,7 @@ int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
>  		if (new != old)
>  			cmpxchg(eseq, old, new);
>  		*since = new;
> -		err = -(new & MAX_ERRNO);
> +		err = -(new & ERRNO_MASK);
>  	}
>  	return err;
>  }
> 
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-improve_errseq-8cfa1539f9e9
> 
> Best regards,

Patch looks like it will do the right thing, but why change this? Is
there some plan to change the value of MAX_ERRNO that I'm not aware of?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] errseq: Eliminate special limitation for macro MAX_ERRNO

Posted by Zijun Hu 10 months ago

On 2025/4/7 20:05, Jeff Layton wrote:
> Patch looks like it will do the right thing, but why change this? Is

just make errseq implementation more generic.

> there some plan to change the value of MAX_ERRNO that I'm not aware of?

no.