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 <zab@versity.com>
This commit is contained in:
Zach Brown
2017-09-19 21:36:11 -07:00
committed by Mark Fasheh
parent 0b15cfe7f8
commit 42b33d616e

View File

@@ -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;