Please add more comments to the functions in dm-bitset.h to confirm
what they do and how to use them. (I've not repeated comments about
dm-array.h that apply here too.)
On Thu, Dec 13, 2012 at 08:19:15PM +0000, Joe Thornber wrote:
> --- /dev/null
> +++ b/drivers/md/persistent-data/dm-bitset.h
> +struct dm_bitset_info {
> + struct dm_array_info array_info;
> +
> + uint32_t current_index;
> + uint64_t current_bits;
Put the uint64_t before the uint32_t perhaps? (Better packing?)
> +
> + bool current_index_set:1;
> +};
> +
> +void dm_bitset_info_init(struct dm_transaction_manager *tm,
> + struct dm_bitset_info *info);
Does this function populate info->array_info and track the array
size so the caller doesn't need to do it in the way described in
dm-array.h?
> +int dm_bitset_empty(struct dm_bitset_info *info, dm_block_t *root);
> +
> +int dm_bitset_resize(struct dm_bitset_info *info, dm_block_t root,
> + uint32_t old_nr_entries, uint32_t new_nr_entries,
> + bool default_value, dm_block_t *new_root);
> +
But does the caller need to track old/new nr_entries?
> +
> +/*
> + * May flush and thus update the root.
> + */
> +int dm_bitset_set_bit(struct dm_bitset_info *info, dm_block_t root,
> + uint32_t index, dm_block_t *new_root);
What are the constraints on index?
Can index be too big and require a resize first? If so, what
error is returned?
> +int dm_bitset_clear_bit(struct dm_bitset_info *info, dm_block_t root,
> + uint32_t index, dm_block_t *new_root);
Error if out of defined range?
> +int dm_bitset_test_bit(struct dm_bitset_info *info, dm_block_t root,
> + uint32_t index, dm_block_t *new_root, bool *result);
> +
Error if out of defined range?
> +/*
> + * You must call this to flush recent changes to disk.
> + */
> +int dm_bitset_flush(struct dm_bitset_info *info, dm_block_t root,
> + dm_block_t *new_root);
If there's a disk error, does flush fail and then could it be retried?
Or does the bitset become unusable or read-only after a disk error?
> +
> +/*----------------------------------------------------------------*/
> +
> +#endif
Conventionally, add /* _LINUX_DM_BITSET_H */ after the #endif.
(Helps when reading files with nested includes.)
- dm-array.h similarly.
Alasdair
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
