Re: [PATCH 02/17] bitops: Add generic parity calculation for u64

Yury Norov
2025-02-24T19:27:07Z

On Mon, Feb 24, 2025 at 12:42:02AM +0800, Kuan-Wei Chiu wrote:
> Several parts of the kernel open-code parity calculations using
> different methods. Add a generic parity64() helper implemented with the
> same efficient approach as parity8().

No reason to add parity32() and parity64() in separate patches

> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
> include/linux/bitops.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index fb13dedad7aa..67677057f5e2 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -281,6 +281,28 @@ static inline int parity32(u32 val)
> return (0x6996 >> (val & 0xf)) & 1;
> }
>
> +/**
> + * parity64 - get the parity of an u64 value
> + * @value: the value to be examined
> + *
> + * Determine the parity of the u64 argument.
> + *
> + * Returns:
> + * 0 for even parity, 1 for odd parity
> + */
> +static inline int parity64(u64 val)
> +{
> + /*
> + * One explanation of this algorithm:
> + * https://funloop.org/codex/problem/parity/README.html

This is already referenced in sources. No need to spread it for more.

> + */
> + val ^= val >> 32;
> + val ^= val >> 16;
> + val ^= val >> 8;
> + val ^= val >> 4;
> + return (0x6996 >> (val & 0xf)) & 1;

It's better to avoid duplicating the same logic again and again.

> +}
> +

So maybe make it a macro?

From f17a28ae3429f49825d65ebc0f7717c6a191a3e2 Mon Sep 17 00:00:00 2001
From: Yury Norov <yury.norov@gmail.com>
Date: Mon, 24 Feb 2025 14:14:27 -0500
Subject: [PATCH] bitops: generalize parity8()

The generic parity calculation approach may be easily generalized for
other standard types. Do that and drop sub-optimal implementation of
parity calculation in x86 code.

Signed-off-by: Yury Norov [NVIDIA] <yury.norov@gmail.com>
---
arch/x86/kernel/bootflag.c | 14 +-----------
include/linux/bitops.h | 47 +++++++++++++++++++++++++++-----------
2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/bootflag.c b/arch/x86/kernel/bootflag.c
index 3fed7ae58b60..4a85c69a28f8 100644
--- a/arch/x86/kernel/bootflag.c
+++ b/arch/x86/kernel/bootflag.c
@@ -2,6 +2,7 @@
/*
* Implement 'Simple Boot Flag Specification 2.0'
*/
+#include <linux/bitops.h>
#include <linux/types.h>
#include <linux/kernel.h>
#include <linux/init.h>
@@ -20,19 +21,6 @@

int sbf_port __initdata = -1; /* set via acpi_boot_init() */

-static int __init parity(u8 v)
-{
- int x = 0;
- int i;
-
- for (i = 0; i < 8; i++) {
- x ^= (v & 1);
- v >>= 1;
- }
-
- return x;
-}
-
static void __init sbf_write(u8 v)
{
unsigned long flags;
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index c1cb53cf2f0f..29601434f5f4 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -230,10 +230,10 @@ static inline int get_count_order_long(unsigned long l)
}

/**
- * parity8 - get the parity of an u8 value
+ * parity - get the parity of a value
* @value: the value to be examined
*
- * Determine the parity of the u8 argument.
+ * Determine parity of the argument.
*
* Returns:
* 0 for even parity, 1 for odd parity
@@ -241,24 +241,45 @@ static inline int get_count_order_long(unsigned long l)
* Note: This function informs you about the current parity. Example to bail
* out when parity is odd:
*
- * if (parity8(val) == 1)
+ * if (parity(val) == 1)
* return -EBADMSG;
*
* If you need to calculate a parity bit, you need to draw the conclusion from
* this result yourself. Example to enforce odd parity, parity bit is bit 7:
*
- * if (parity8(val) == 0)
+ * if (parity(val) == 0)
* val ^= BIT(7);
+ *
+ * One explanation of this algorithm:
+ * https://funloop.org/codex/problem/parity/README.html
*/
-static inline int parity8(u8 val)
-{
- /*
- * One explanation of this algorithm:
- * https://funloop.org/codex/problem/parity/README.html
- */
- val ^= val >> 4;
- return (0x6996 >> (val & 0xf)) & 1;
-}
+#define parity(val) \
+({ \
+ u64 __v = (val); \
+ int __ret; \
+ switch (BITS_PER_TYPE(val)) { \
+ case 64: \
+ __v ^= __v >> 32; \
+ fallthrough; \
+ case 32: \
+ __v ^= __v >> 16; \
+ fallthrough; \
+ case 16: \
+ __v ^= __v >> 8; \
+ fallthrough; \
+ case 8: \
+ __v ^= __v >> 4; \
+ __ret = (0x6996 >> (__v & 0xf)) & 1; \
+ break; \
+ default: \
+ BUILD_BUG(); \
+ } \
+ __ret; \
+})
+
+#define parity8(val) parity((u8)(val))
+#define parity32(val) parity((u32)(val))
+#define parity64(val) parity((u64)(val))

/**
* __ffs64 - find first set bit in a 64 bit word
--
2.43.0