From e8b8c010e6f7fa26bb4110d606ab7972018302a1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 15 Dec 2023 01:24:06 -0600 Subject: [PATCH] Dropped uncrc32c, use flcksum for aligning checksums While definitely winning cool points, uncrc32cs have a number of problems: 1. uncrc32c is relatively unflexible, being limited to only CRC-related checksums, and probably violating some properties of cryptographic hashes if possible there. 2. Code savings are minimal, a reversed crc32c implementation is a only a little less costly than the logic to save aligned CRCs, and since it's not on the hot-path, the stack cost is ~zero. 3. uncrc32c may come with a high computation cost. We aren't measuring this, but uncrc32c either operates at the bit-level, or requires a second set of tables which is unreasonable for littlefs's use case. With uncrc32c you need to update the checksum based on every bit in the extra unaligned data, up to prog_size. With flcksums it's just a copy of a word, and prog_size has no impact. So for now dropping uncrc32c, though this can always be reverted in the future. --- lfs.c | 33 ++++----------------------------- lfs_util.c | 22 ---------------------- lfs_util.h | 7 ------- 3 files changed, 4 insertions(+), 58 deletions(-) diff --git a/lfs.c b/lfs.c index 7fad1de5..a102d06a 100644 --- a/lfs.c +++ b/lfs.c @@ -9950,12 +9950,7 @@ static int lfsr_ftree_flush(lfs_t *lfs, err = lfsr_bd_prog(lfs, bptr.data.u.disk.block, bptr.cksize, &buffer[pos_ - pos], d_, - #ifndef LFS_NO_UNCRC32C - &bptr.cksum, NULL - #else - NULL, &bptr.cksum - #endif - ); + NULL, &bptr.cksum); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); return err; @@ -10011,12 +10006,7 @@ static int lfsr_ftree_flush(lfs_t *lfs, lfsr_data_slice(bptr_.data, pos_ - (bid_-(weight_-1)), d_), - #ifndef LFS_NO_UNCRC32C - &bptr.cksum, NULL - #else - NULL, &bptr.cksum - #endif - ); + NULL, &bptr.cksum); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); return err; @@ -10037,12 +10027,7 @@ static int lfsr_ftree_flush(lfs_t *lfs, err = lfsr_bd_prog(lfs, bptr.data.u.disk.block, bptr.cksize + i, &(uint8_t){0}, 1, - #ifndef LFS_NO_UNCRC32C - &bptr.cksum, NULL - #else - NULL, &bptr.cksum - #endif - ); + NULL, &bptr.cksum); if (err) { LFS_ASSERT(err != LFS_ERR_RANGE); return err; @@ -10060,22 +10045,12 @@ static int lfsr_ftree_flush(lfs_t *lfs, lfs_ssize_t d = bptr.cksize % lfs->cfg->prog_size; LFS_ASSERT(d <= lfs->pcache.size); lfs->pcache.size -= d; - #ifndef LFS_NO_UNCRC32C - bptr.cksum = lfs_uncrc32c(bptr.cksum, - &lfs->pcache.buffer[lfs->pcache.size], - d); - #endif bptr.cksize -= d; // TODO validate? // finalize our write err = lfsr_bd_flush(lfs, - #ifndef LFS_NO_UNCRC32C - NULL - #else - &bptr.cksum - #endif - ); + &bptr.cksum); if (err) { return err; } diff --git a/lfs_util.c b/lfs_util.c index e8e75856..03865d79 100644 --- a/lfs_util.c +++ b/lfs_util.c @@ -195,26 +195,4 @@ uint32_t lfs_crc32c(uint32_t crc, const void *buffer, size_t size) { return crc; } -#ifndef LFS_NO_UNCRC32C -// Undoes a crc32c -// -// like crc32c, but backwards -uint32_t lfs_uncrc32c(uint32_t crc, const void *buffer, size_t size) { - // init with 0xffffffff so prefixed zeros affect the crc - const uint8_t *data = buffer; - crc ^= 0xffffffff; - - for (size_t i = 0; i < size; i++) { - for (size_t j = 0; j < 8; j++) { - crc = (crc << 1) ^ ((crc & 0x80000000) ? 0x05ec76f1 : 0); - } - crc = crc ^ data[size-1-i]; - } - - // fini with 0xffffffff to cancel out init when called incrementally - crc ^= 0xffffffff; - return crc; -} -#endif - #endif diff --git a/lfs_util.h b/lfs_util.h index 7d677e01..b816d305 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -361,13 +361,6 @@ uint32_t lfs_crc(uint32_t crc, const void *buffer, size_t size); // uint32_t lfs_crc32c(uint32_t crc, const void *buffer, size_t size); -#ifndef LFS_NO_UNCRC32C -// Undoes a crc32c -// -// like crc32c, but backwards -uint32_t lfs_uncrc32c(uint32_t crc, const void *buffer, size_t size); -#endif - // Allocate memory, only used if buffers are not provided to littlefs // Note, memory must be 64-bit aligned