From a01ec45f14b0cb6c003ba73cd7861ecc806e86ce Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Thu, 10 Sep 2026 07:39:21 +0200 Subject: [PATCH] Remove stream CRC and optimize CRC compute CRC32C using two lanes --- cmdline/mkstream.c | 8 --- cmdline/selftest.c | 27 +++++++++ cmdline/state.c | 25 +------- cmdline/stream.c | 12 ---- cmdline/stream.h | 39 ------------ cmdline/util.c | 70 +++++++++++++++++++++- cmdline/util.h | 144 +++++++++++++++++++++++++++++++-------------- 7 files changed, 196 insertions(+), 129 deletions(-) diff --git a/cmdline/mkstream.c b/cmdline/mkstream.c index fdcf6074..224e8380 100644 --- a/cmdline/mkstream.c +++ b/cmdline/mkstream.c @@ -20,7 +20,6 @@ void test(int advise_mode) uint32_t u32 = -1L; uint64_t u64 = -1LL; uint32_t put_crc_stored; - uint32_t put_crc_computed; crc32c_init(); @@ -79,13 +78,6 @@ void test(int advise_mode) } put_crc_stored = scrc(s); - put_crc_computed = scrc_stream(s); - - if (put_crc_stored != put_crc_computed) { - /* LCOV_EXCL_START */ - exit(EXIT_FAILURE); - /* LCOV_EXCL_STOP */ - } if (sputble32(put_crc_stored, s) != 0) { /* LCOV_EXCL_START */ diff --git a/cmdline/selftest.c b/cmdline/selftest.c index 5d7b8f65..61db7912 100644 --- a/cmdline/selftest.c +++ b/cmdline/selftest.c @@ -297,6 +297,33 @@ static void test_crc32c(void) /* LCOV_EXCL_STOP */ } } + + { + unsigned char data[8255 + 7]; + static const size_t size[] = { 4095, 4096, 4097, 5000, 8255 }; + static const unsigned offset[] = { 0, 1, 3, 7 }; + unsigned j; + unsigned k; + + for (i = 0; i < sizeof(data); ++i) + data[i] = (unsigned char)(i * 29 + i / 7); + + for (j = 0; j < sizeof(size) / sizeof(size[0]); ++j) { + for (k = 0; k < sizeof(offset) / sizeof(offset[0]); ++k) { + uint32_t seed = 0x12345678; + uint32_t digest = crc32c(seed, data + offset[k], size[j]); + uint32_t digest_gen = crc32c_gen(seed, data + offset[k], size[j]); + + if (digest != digest_gen) { + /* LCOV_EXCL_START */ + log_fatal(EINTERNAL, "Failed CRC32C large buffer test\n"); + exit(EXIT_FAILURE); + /* LCOV_EXCL_STOP */ + } + } + } + + } } static void test_parity(void) diff --git a/cmdline/state.c b/cmdline/state.c index 6b6cfeed..5e92a713 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -3855,23 +3855,18 @@ struct state_write_thread_context { * Write all hashes in a contiguous run of file blocks. * * The source hashes are separated by the block state byte, so they cannot be - * copied with a single memcpy(). Keep the stream pointer and the independent - * CRC local while filling each buffered group to avoid the per-block stream - * call and field updates. The CRC is still computed before each hash is copied, - * preserving the memory-corruption check performed by swrite(). + * copied with a single memcpy(). Keep the stream pointer local while filling + * each buffered group to avoid the per-block stream call and field updates. */ static int state_write_block_run(STREAM* f, struct snapraid_file* file, block_off_t file_pos, block_off_t count) { unsigned char* block_ptr = (unsigned char*)file_block(file, file_pos); size_t block_stride = block_sizeof(); - assert(f->flags & STREAM_FLAGS_CRC); - while (count) { size_t available = (size_t)(f->end - f->pos); block_off_t cached_count = available / BLOCK_HASH_SIZE; unsigned char* output; - uint32_t crc; if (cached_count > count) cached_count = count; @@ -3888,18 +3883,15 @@ static int state_write_block_run(STREAM* f, struct snapraid_file* file, block_of } output = f->pos; - crc = f->crc_stream; for (block_off_t i = 0; i < cached_count; ++i) { struct snapraid_block* block = (struct snapraid_block*)block_ptr; - crc = crc32c_plain(crc, block->hash, BLOCK_HASH_SIZE); hash_copy(output, block->hash); output += BLOCK_HASH_SIZE; block_ptr += block_stride; } - f->crc_stream = crc; f->pos = output; count -= cached_count; } @@ -4473,19 +4465,6 @@ static void* state_write_thread(void* arg) /* get the file crc */ crc = scrc(f); - /* - * Compare the crc of the data written to file - * with the one of the data written to the stream - */ - if (crc != scrc_stream(f)) { - /* LCOV_EXCL_START */ - log_fatal(ECONTENT, "CRC mismatch while writing the content stream.\n"); - log_fatal(ECONTENT, "DANGER! Your RAM memory is faulty! DO NOT PROCEED UNTIL FIXED!\n"); - log_fatal(ECONTENT, "Try running a memory test like http://www.memtest86.com/\n"); - goto bail; - /* LCOV_EXCL_STOP */ - } - sputble32(crc, f); if (serror(f)) { /* LCOV_EXCL_START */ diff --git a/cmdline/stream.c b/cmdline/stream.c index 8e5d6458..d5b4a861 100644 --- a/cmdline/stream.c +++ b/cmdline/stream.c @@ -68,7 +68,6 @@ STREAM* sopen_read(const char* file, int flags) s->offset_uncached = 0; s->crc = 0; s->crc_uncached = 0; - s->crc_stream = CRC_IV; return s; } @@ -100,7 +99,6 @@ STREAM* sopen_multi_write(unsigned count, int flags) s->offset_uncached = 0; s->crc = 0; s->crc_uncached = 0; - s->crc_stream = CRC_IV; return s; } @@ -389,12 +387,6 @@ uint32_t scrc(STREAM* s) return crc32c(s->crc_uncached, s->buffer, s->pos - s->buffer); } -uint32_t scrc_stream(STREAM* s) -{ - assert(s->flags & STREAM_FLAGS_CRC); - return s->crc_stream ^ CRC_IV; -} - int sgetc_uncached(STREAM* s) { /* if at the end of the buffer, fill it */ @@ -735,10 +727,6 @@ int swrite_uncached(const void* void_data, size_t size, STREAM* f) if (available > size) available = size; - /* compute the independent CRC before copying to detect buffer corruption */ - if (f->flags & STREAM_FLAGS_CRC) - f->crc_stream = crc32c_plain(f->crc_stream, data, available); - memcpy(f->pos, data, available); f->pos += available; data += available; diff --git a/cmdline/stream.h b/cmdline/stream.h index b444ca47..d432d878 100644 --- a/cmdline/stream.h +++ b/cmdline/stream.h @@ -65,26 +65,6 @@ struct stream { */ uint32_t crc_uncached; - /** - * CRC of the data written to the stream. - * - * This is an extra check of the data that is written to - * file to ensure that it's consistent even in case - * of memory errors. - * - * This extra check takes about 2 seconds for each GB of - * content file with the Intel CRC instruction, - * and about 4 seconds without it. - * But usually this doesn't slow down the write process, - * as the disk is the bottle-neck. - * - * Note that this CRC doesn't have the IV processing. - * - * Not used in reading. - * In writing, it's all the data wrote calling sput() functions. - */ - uint32_t crc_stream; - struct advise_struct* advise; /**< Cache advice state for each handle. */ }; @@ -147,11 +127,6 @@ int64_t stell(STREAM* s); */ uint32_t scrc(STREAM* s); -/** - * Get the CRC of the processed data in put. - */ -uint32_t scrc_stream(STREAM* s); - /** * Check if the buffer has enough data loaded. */ @@ -369,16 +344,6 @@ static inline int sputc(int c, STREAM* s) return -1; } - /** - * Update the crc *before* writing the data in the buffer - * - * This must be done before the memory write, - * to be able to detect memory errors on the buffer, - * happening before we write it on the file. - */ - if (s->flags & STREAM_FLAGS_CRC) - s->crc_stream = crc32c_plain_char(s->crc_stream, c); - *s->pos++ = c; return 0; @@ -412,10 +377,6 @@ static __always_inline int swrite(const void* data, size_t size, STREAM* f) if (tommy_unlikely(!sptrlookup(f, size))) return swrite_uncached(data, size, f); - /* compute the independent CRC before copying to detect buffer corruption */ - if (f->flags & STREAM_FLAGS_CRC) - f->crc_stream = crc32c_plain(f->crc_stream, data, size); - memcpy(f->pos, data, size); f->pos += size; diff --git a/cmdline/util.c b/cmdline/util.c index ee94480e..199e52ff 100644 --- a/cmdline/util.c +++ b/cmdline/util.c @@ -398,8 +398,68 @@ uint32_t CRC32C_3[256] = { 0x4a21617b, 0x9764cbc3, 0xf54642fa, 0x2803e842 }; -#if CONFIG_X86 -int crc_x86; +#if CONFIG_X86 || CONFIG_ARM_CRC +static uint32_t crc32c_shift_table[sizeof(size_t) * 8][32]; + +static uint32_t crc32c_matrix_times(const uint32_t* matrix, uint32_t vector) +{ + uint32_t value = 0; + + while (vector) { + if (vector & 1) + value ^= *matrix; + vector >>= 1; + ++matrix; + } + + return value; +} + +static void crc32c_matrix_square(uint32_t* square, const uint32_t* matrix) +{ + unsigned i; + + for (i = 0; i < 32; ++i) + square[i] = crc32c_matrix_times(matrix, matrix[i]); +} + +static void crc32c_shift_init(void) +{ + uint32_t matrix[32]; + uint32_t square[32]; + uint32_t row; + unsigned i; + + /* reflected Castagnoli polynomial, represented as one zero-bit operator. */ + matrix[0] = 0x82f63b78; + row = 1; + for (i = 1; i < 32; ++i) { + matrix[i] = row; + row <<= 1; + } + + /* square three times to obtain the operator for one zero byte. */ + crc32c_matrix_square(square, matrix); + crc32c_matrix_square(matrix, square); + crc32c_matrix_square(crc32c_shift_table[0], matrix); + + for (i = 1; i < sizeof(size_t) * 8; ++i) + crc32c_matrix_square(crc32c_shift_table[i], crc32c_shift_table[i - 1]); +} + +uint32_t crc32c_shift(uint32_t crc, size_t size) +{ + unsigned i = 0; + + while (size) { + if (size & 1) + crc = crc32c_matrix_times(crc32c_shift_table[i], crc); + size >>= 1; + ++i; + } + + return crc; +} #endif uint32_t crc32c_gen(uint32_t crc, const unsigned char* ptr, size_t size) @@ -446,10 +506,14 @@ void crc32c_init(void) crc32c = crc32c_gen; #if CONFIG_X86 if (raid_cpu_has_crc32()) { - crc_x86 = 1; + crc32c_shift_init(); crc32c = crc32c_x86; } #endif +#if CONFIG_ARM_CRC + crc32c_shift_init(); + crc32c = crc32c_arm64; +#endif } /****************************************************************************/ diff --git a/cmdline/util.h b/cmdline/util.h index 9db2c402..fd2a3e7b 100644 --- a/cmdline/util.h +++ b/cmdline/util.h @@ -91,31 +91,14 @@ extern uint32_t CRC32C_1[256]; extern uint32_t CRC32C_2[256]; extern uint32_t CRC32C_3[256]; -/** - * If the CPU support the CRC instructions. - */ -#if CONFIG_X86 -extern int crc_x86; -#endif +/** Minimum size worth splitting into two hardware CRC lanes. */ +#define CRC32C_2LANE_MIN 4096 /** - * Compute CRC-32 (Castagnoli) for a single byte without the IV. + * Advance a raw CRC-32C by a number of zero bytes. + * Used to combine independent hardware CRC lanes. */ -static inline uint32_t crc32c_plain_char(uint32_t crc, unsigned char c) -{ -#if CONFIG_X86 - if (tommy_likely(crc_x86)) { - asm ("crc32b %1, %0\n" : "+r" (crc) : "m" (c)); - return crc; - } -#endif -#if CONFIG_ARM_CRC - asm ("crc32cb %w0, %w0, %w1\n" : "+r" (crc) : "r" (c)); - return crc; -#else - return CRC32C_0[(crc ^ c) & 0xff] ^ (crc >> 8); -#endif -} +uint32_t crc32c_shift(uint32_t crc, size_t size); /** * Compute the CRC-32 (Castagnoli) without the IV. @@ -146,12 +129,31 @@ static inline uint32_t crc32c_x86_plain(uint32_t crc, const unsigned char* ptr, { #ifdef CONFIG_X86_64 uint64_t crc64 = crc; + if (size >= CRC32C_2LANE_MIN) { + const unsigned char* ptr0 = ptr; + size_t lane_size = (size / 2) & ~(size_t)31; + const unsigned char* ptr1 = ptr + lane_size; + const unsigned char* end = ptr1; + uint64_t crc0 = crc; + uint64_t crc1 = 0; - /* block hashes are normally 16 bytes and dominate content serialization */ - if (tommy_likely(size == 16)) { - asm ("crc32q %1, %0\n" : "+r" (crc64) : "m" (*(const uint64_t*)ptr)); - asm ("crc32q %1, %0\n" : "+r" (crc64) : "m" (*(const uint64_t*)(ptr + 8))); - return (uint32_t)crc64; + /* Keep two dependency chains in flight and combine them only once. */ + while (ptr0 < end) { + asm ("crc32q %1, %0\n" : "+r" (crc0) : "m" (*(const uint64_t*)ptr0)); + asm ("crc32q %1, %0\n" : "+r" (crc1) : "m" (*(const uint64_t*)ptr1)); + asm ("crc32q %1, %0\n" : "+r" (crc0) : "m" (*(const uint64_t*)(ptr0 + 8))); + asm ("crc32q %1, %0\n" : "+r" (crc1) : "m" (*(const uint64_t*)(ptr1 + 8))); + asm ("crc32q %1, %0\n" : "+r" (crc0) : "m" (*(const uint64_t*)(ptr0 + 16))); + asm ("crc32q %1, %0\n" : "+r" (crc1) : "m" (*(const uint64_t*)(ptr1 + 16))); + asm ("crc32q %1, %0\n" : "+r" (crc0) : "m" (*(const uint64_t*)(ptr0 + 24))); + asm ("crc32q %1, %0\n" : "+r" (crc1) : "m" (*(const uint64_t*)(ptr1 + 24))); + ptr0 += 32; + ptr1 += 32; + } + + crc64 = crc32c_shift((uint32_t)crc0, lane_size) ^ (uint32_t)crc1; + ptr += 2 * lane_size; + size -= 2 * lane_size; } while (size >= 32) { @@ -175,6 +177,33 @@ static inline uint32_t crc32c_x86_plain(uint32_t crc, const unsigned char* ptr, } crc = (uint32_t)crc64; #else + if (size >= CRC32C_2LANE_MIN) { + const unsigned char* ptr0 = ptr; + size_t lane_size = (size / 2) & ~(size_t)15; + const unsigned char* ptr1 = ptr + lane_size; + const unsigned char* end = ptr1; + uint32_t crc0 = crc; + uint32_t crc1 = 0; + + /* Keep two dependency chains in flight and combine them only once. */ + while (ptr0 < end) { + asm ("crc32l %1, %0\n" : "+r" (crc0) : "m" (*(const uint32_t*)ptr0)); + asm ("crc32l %1, %0\n" : "+r" (crc1) : "m" (*(const uint32_t*)ptr1)); + asm ("crc32l %1, %0\n" : "+r" (crc0) : "m" (*(const uint32_t*)(ptr0 + 4))); + asm ("crc32l %1, %0\n" : "+r" (crc1) : "m" (*(const uint32_t*)(ptr1 + 4))); + asm ("crc32l %1, %0\n" : "+r" (crc0) : "m" (*(const uint32_t*)(ptr0 + 8))); + asm ("crc32l %1, %0\n" : "+r" (crc1) : "m" (*(const uint32_t*)(ptr1 + 8))); + asm ("crc32l %1, %0\n" : "+r" (crc0) : "m" (*(const uint32_t*)(ptr0 + 12))); + asm ("crc32l %1, %0\n" : "+r" (crc1) : "m" (*(const uint32_t*)(ptr1 + 12))); + ptr0 += 16; + ptr1 += 16; + } + + crc = crc32c_shift(crc0, lane_size) ^ crc1; + ptr += 2 * lane_size; + size -= 2 * lane_size; + } + while (size >= 16) { asm ("crc32l %1, %0\n" : "+r" (crc) : "m" (*(const uint32_t*)ptr)); asm ("crc32l %1, %0\n" : "+r" (crc) : "m" (*(const uint32_t*)(ptr + 4))); @@ -211,6 +240,50 @@ static inline uint32_t crc32c_x86_plain(uint32_t crc, const unsigned char* ptr, #if CONFIG_ARM_CRC static inline uint32_t crc32c_arm64_plain(uint32_t crc, const unsigned char* ptr, size_t size) { + if (size >= CRC32C_2LANE_MIN) { + const unsigned char* ptr0 = ptr; + size_t lane_size = (size / 2) & ~(size_t)31; + const unsigned char* ptr1 = ptr + lane_size; + const unsigned char* end = ptr1; + uint32_t crc0 = crc; + uint32_t crc1 = 0; + + /* Keep two dependency chains in flight and combine them only once. */ + while (ptr0 < end) { + uint64_t val00; + uint64_t val01; + uint64_t val02; + uint64_t val03; + uint64_t val10; + uint64_t val11; + uint64_t val12; + uint64_t val13; + + __builtin_memcpy(&val00, ptr0, 8); + __builtin_memcpy(&val10, ptr1, 8); + __builtin_memcpy(&val01, ptr0 + 8, 8); + __builtin_memcpy(&val11, ptr1 + 8, 8); + __builtin_memcpy(&val02, ptr0 + 16, 8); + __builtin_memcpy(&val12, ptr1 + 16, 8); + __builtin_memcpy(&val03, ptr0 + 24, 8); + __builtin_memcpy(&val13, ptr1 + 24, 8); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc0) : "r" (val00)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc1) : "r" (val10)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc0) : "r" (val01)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc1) : "r" (val11)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc0) : "r" (val02)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc1) : "r" (val12)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc0) : "r" (val03)); + asm ("crc32cx %w0, %w0, %x1\n" : "+r" (crc1) : "r" (val13)); + ptr0 += 32; + ptr1 += 32; + } + + crc = crc32c_shift(crc0, lane_size) ^ crc1; + ptr += 2 * lane_size; + size -= 2 * lane_size; + } + while (size >= 8) { uint64_t val; __builtin_memcpy(&val, ptr, 8); @@ -235,23 +308,6 @@ static inline uint32_t crc32c_arm64_plain(uint32_t crc, const unsigned char* ptr } #endif -/** - * Compute CRC-32 (Castagnoli) without the IV. - */ -static inline uint32_t crc32c_plain(uint32_t crc, const unsigned char* ptr, size_t size) -{ -#if CONFIG_X86 - if (tommy_likely(crc_x86)) { - return crc32c_x86_plain(crc, ptr, size); - } -#endif -#if CONFIG_ARM_CRC - return crc32c_arm64_plain(crc, ptr, size); -#else - return crc32c_gen_plain(crc, ptr, size); -#endif -} - /** * Compute the CRC-32 (Castagnoli) */