Skip to contentSkip to conversation

btrfs-progs: crypto/crc32c: cleanup and reimplementation to reduce asm usage

PATCH Superseded Qu Wenruo 1 participant 3 messages Nov 6

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

cover btrfs-progs: crypto/crc32c: cleanup and reimplementation to reduce asm usage

Although kernel is using a lot of ASM files for hardware acceleration, we also have projects like google's crc32c
(https://github.com/google/crc32c) which doesn't have any inlined or dedicated asm file.

The idea is that we have compiler wrappers for those instructions already, considering how rare modern developers read/write asm code, reducing asm code will improve readability in the long run.

The first patch is remove the inline asm usages, the second one is to remove the only asm file for crc32-pcl optimization, and replace it with a C function.

Qu Wenruo (2):
btrfs-progs: cleanup inline asm usage
btrfs-progs: crypto/crc32c: port crc32c-pcl optimization to C

 Makefile                         |  14 --
 common/cpu-utils.c               |  31 +--
 crypto/crc32c-pcl-intel-asm_64.S | 370 -------------------------------
 crypto/crc32c.c                  | 282 +++++++++++++++++++++--
 crypto/hash-speedtest.c          |  14 +-
 libbtrfs/crc32c.c                |  43 +---
 6 files changed, 284 insertions(+), 470 deletions(-)
 delete mode 100644 crypto/crc32c-pcl-intel-asm_64.S

--
2.56.0

author
#1 lore raw

PATCH 1/2 btrfs-progs: cleanup inline asm usage

Both clang and gcc have lowlevel instruction wrappers already, there is no need to use inline asm just for several small operations like cpuid.

The cleanup includes:

- Use compiler wrappers to replace inlined asm

- Add dedicated target attributes for involved hardware acceleration

Asssited-by LLM (locating the wrappers)
---
 common/cpu-utils.c      | 31 ++++-------------------------
 crypto/crc32c.c         | 20 ++++++++-----------
 crypto/hash-speedtest.c | 14 +++-----------
 libbtrfs/crc32c.c       | 43 +++++++++++------------------------------
 4 files changed, 26 insertions(+), 82 deletions(-)
common/cpu-utils.c +4 -27
index 32914d360e63..a99efb932ff2 100644
@@ -15,37 +15,14 @@
  */
 
 #include <stdio.h>
+#ifdef __x86_64__
+#include <cpuid.h>
+#endif
 #include "common/cpu-utils.h"
 
 unsigned long __cpu_flags = CPU_FLAG_NONE;
 unsigned long __cpu_flags_orig = CPU_FLAG_NONE;
 
-#ifdef __x86_64__
-/*
- * Do manual cpuid as cpuid.h is not available for all versions.
- *
- * Plain cpuid(level) is __cpuidex(level, 0), otherwise look for the leaf/subleaf
- * modes at https://en.wikipedia.org/wiki/CPUID or compilers' cpuid.h.
- */
-
-#ifndef __cpuidex
-#define __cpuidex(leaf, subleaf, a, b, c, d)				\
-	__asm__ __volatile__ ("cpuid\n\t"				\
-			: "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
-			: "0" (leaf), "2" (subleaf))
-#endif
-
-#else
-
-#define __cpuidex(leaf, subleaf, a, b, c, d)				\
-	do {								\
-		a = 0;							\
-		b = 0;							\
-		c = 0;							\
-		d = 0;							\
-	} while(0)
-#endif
-
 #define FLAG(name)	if (__cpu_flags & CPU_FLAG_ ## name) printf(" " #name)
 void cpu_print_flags(void) {
 	printf("CPU flags: 0x%lx\n", __cpu_flags);
@@ -88,7 +65,7 @@ void cpu_detect_flags(void)
 		__cpu_flags |= CPU_FLAG_AVX2;
 
 	/* Flags unsupported by builtins */
-	__cpuidex(7, 0, a, b, c, d);
+	__cpuid_count(7, 0, a, b, c, d);
 	if (b & (1UL << 29))
 		__cpu_flags |= CPU_FLAG_SHA;
 
crypto/crc32c.c +8 -12
index f0a23f836741..9a3c9fc14eab 100644
@@ -18,6 +18,8 @@ static uint32_t (*crc32c_impl)(uint32_t crc, unsigned char const *data, uint32_t
 
 #ifdef __x86_64__
 
+#include <nmmintrin.h>
+
 /* asmlinkage */ unsigned int crc_pcl(const unsigned char *buffer, unsigned int len, unsigned int crc_init);
 static unsigned int crc32c_pcl(uint32_t crc, unsigned char const *data, uint32_t len) {
 	return crc_pcl(data, len, crc);
@@ -34,22 +36,19 @@ static unsigned int crc32c_pcl(uint32_t crc, unsigned char const *data, uint32_t
  * Volume 2A: Instruction Set Reference, A-M
  */
 #if  __SIZEOF_LONG__ == 8
-#define REX_PRE "0x48, "
 #define SCALE_F 8
+#define crc32c_hw_long(crc, v)	_mm_crc32_u64((crc), (v))
 #else
-#define REX_PRE
 #define SCALE_F 4
+#define crc32c_hw_long(crc, v)	_mm_crc32_u32((crc), (v))
 #endif
 
+__attribute__((target("sse4.2")))
 static uint32_t crc32c_intel_le_hw_byte(uint32_t crc, unsigned char const *data,
 					uint32_t length)
 {
 	while (length--) {
-		__asm__ __volatile__(
-			".byte 0xf2, 0xf, 0x38, 0xf0, 0xf1"
-			:"=S"(crc)
-			:"0"(crc), "c"(*data)
-		);
+		crc = _mm_crc32_u8(crc, *data);
 		data++;
 	}
 
@@ -61,6 +60,7 @@ static uint32_t crc32c_intel_le_hw_byte(uint32_t crc, unsigned char const *data,
  * Steps through buffer one byte at at time, calculates reflected crc using
  * table.
  */
+__attribute__((target("sse4.2")))
 static uint32_t crc32c_sse42(uint32_t crc, unsigned char const *data, uint32_t length)
 {
 	unsigned int iquotient = length / SCALE_F;
@@ -68,11 +68,7 @@ static uint32_t crc32c_sse42(uint32_t crc, unsigned char const *data, uint32_t l
 	unsigned long *ptmp = (unsigned long *)data;
 
 	while (iquotient--) {
-		__asm__ __volatile__(
-			".byte 0xf2, " REX_PRE "0xf, 0x38, 0xf1, 0xf1;"
-			:"=S"(crc)
-			:"0"(crc), "c"(*ptmp)
-		);
+		crc = crc32c_hw_long(crc, *ptmp);
 		ptmp++;
 	}
 
crypto/hash-speedtest.c +3 -11
index 6caa89f7d6f8..95ba63ee6025 100644
@@ -30,6 +30,7 @@
 #include "common/cpu-utils.h"
 
 #ifdef __x86_64__
+#include <x86intrin.h>
 static const int cycles_supported = 1;
 #else
 static const int cycles_supported = 0;
@@ -45,19 +46,10 @@ const int blocksize = 4096;
 int iterations = 100000;
 
 #ifdef __x86_64__
-static __always_inline unsigned long long rdtsc(void)
-{
-	unsigned low, high;
-
-        asm volatile("rdtsc" : "=a" (low), "=d" (high));
-
-        return (low | ((u64)(high) << 32));
-}
-
 static inline u64 read_tsc(void)
 {
-       asm volatile("mfence");
-       return rdtsc();
+	_mm_mfence();
+	return __rdtsc();
 }
 
 #define cpu_cycles()		read_tsc()
libbtrfs/crc32c.c +11 -32

90 lines not on this page · raw message

Signature · 1 line
2.56.0
author
#2 lore raw

PATCH 2/2 btrfs-progs: crypto/crc32c: port crc32c-pcl optimization to C

In reply to Qu Wenruo

This is inspired by Google's CRC32C library which has all the hardware acceleration, but without any ASM code.

The idea is to use compiler wrapped commands instead of the hand written ASM code.

The performance is at the same level as the old code:

The hardware is Ryzen 9 HX 370, on a single CPU core, unit is GiB/s.

Size | Old (asm) | New (C)
-----------+-------------+-------------

 64 B      | 4.76–4.80   | 4.80–4.82
 256 B     | 8.76–8.88   | 8.82–8.87
 1 KiB     | 12.26–12.41 | 12.47–12.51
 4 KiB     | 13.35–13.49 | 13.48
 16 KiB    | 13.69–13.78 | 13.66–13.79
 64 KiB    | 13.73–13.83 | 13.75–13.83
 1 MiB     | 13.62–13.72 | 13.61–13.63

We should have a better hash-vectest to cover larger block sizes, but that will be added later by the newer ARMv8 CRC optimization.

Asssited-by LLM (code porting, testing)
---
 Makefile                         |  14 --
 crypto/crc32c-pcl-intel-asm_64.S | 370 -------------------------------
 crypto/crc32c.c                  | 262 +++++++++++++++++++++-
 3 files changed, 258 insertions(+), 388 deletions(-)
 delete mode 100644 crypto/crc32c-pcl-intel-asm_64.S
Makefile +0 -14
index 78c23ad3c694..e5a5534b4fb7 100644
@@ -107,8 +107,6 @@ ENABLE_WARNING_FLAGS := $(call cc-option, -Wimplicit-fallthrough) \
 			$(call cc-option, -Wmissing-prototypes) \
 			-Wshadow
 
-ASFLAGS =
-
 # Common build flags
 CFLAGS = $(SUBST_CFLAGS) \
 	 -std=gnu11 \
@@ -413,11 +411,6 @@ CRYPTO_OBJECTS = crypto/sha224-256.o crypto/blake2b-ref.o crypto/blake2b-sse2.o
 CRYPTO_CFLAGS = -DCRYPTOPROVIDER_BUILTIN=1
 endif
 
-ifeq ($(TARGET_CPU),x86_64)
-CRYPTO_OBJECTS += crypto/crc32c-pcl-intel-asm_64.o
-ASFLAGS += -fPIC
-endif
-
 CHECKER_FLAGS += $(btrfs_convert_cflags)
 
 # collect values of the variables above
@@ -478,13 +471,6 @@ else
 	check_echo = true
 endif
 
-.S.o:
-	@echo "  AS       $@"
-	$(Q)$(CC) $(CFLAGS) $(ASFLAGS) -c $< -o $@
-
-%.static.o: %.S
-	@echo "  AS       $@"
-	$(Q)$(CC) $(CFLAGS) $(ASFLAGS) -c $< -o $@
 #
 # Pick from per-file variables, btrfs_*_cflags
 #
crypto/crc32c-pcl-intel-asm_64.S deleted +0 -370

370 lines not on this page · raw message

crypto/crc32c.c +258 -4

281 lines not on this page · raw message

Signature · 1 line
2.56.0