From 42b33d616e6d60f90e01c22e31b5fe663d42c38b Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Tue, 19 Sep 2017 21:36:11 -0700 Subject: [PATCH] scoutfs: fix btree bit iteration store_pos_bits() was trying to iterate over bits that were different between the existing bits set in the item and the new bits that will be set. It used a too clever for_each helper that tried to only iterate as many times as there were bits. But it messed up and only used ffs to find the next bit for the first iteration. From then on it would iterate over bits that weren't different. This would cause the counts to be changed when the bits didn't change and end up being wildly wrong. Fix this by using a much clearer loop. It still breaks out when there are no more different bits and we're only using a few low bits so the number of iterations is tiny. Signed-off-by: Zach Brown --- kmod/src/btree.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/kmod/src/btree.c b/kmod/src/btree.c index 38d70586..1d6a5c9a 100644 --- a/kmod/src/btree.c +++ b/kmod/src/btree.c @@ -409,13 +409,6 @@ static u8 bits_from_counts(struct scoutfs_btree_block *bt) return bits; } -/* - * Iterate through 0-based bit numbers set in 'bits' from least to - * greatest. It modifies 'bits' as it goes! - */ -#define for_each_bit(i, bits) \ - for (i = bits ? ffs(bits) : 0; i-- > 0; bits &= ~(1 < i)) - /* * Store the new bits and update the counts to match the difference from * the previously set bits. Callers use this to keep item bits in sync @@ -424,16 +417,17 @@ static u8 bits_from_counts(struct scoutfs_btree_block *bt) static void store_pos_bits(struct scoutfs_btree_block *bt, int pos, u8 bits) { u8 diff = bits ^ pos_bits(bt, pos); - int b; + int i; + u8 b; - if (!diff) - return; - - for_each_bit(b, diff) { - if (bits & (1 << b)) - le16_add_cpu(&bt->bit_counts[b], 1); - else - le16_add_cpu(&bt->bit_counts[b], -1); + for (i = 0, b = 1; diff != 0; i++, b <<= 1) { + if (diff & b) { + if (bits & b) + le16_add_cpu(&bt->bit_counts[i], 1); + else + le16_add_cpu(&bt->bit_counts[i], -1); + diff ^= b; + } } bt->item_hdrs[pos].bits = bits;