Re: [PATCH V3 02/14] sbitmap: introduce __sbitmap_for_each_set()
From: Ming Lei <hidden>
Date: 2017-08-31 03:33:55
On Wed, Aug 30, 2017 at 03:55:13PM +0000, Bart Van Assche wrote:
On Sun, 2017-08-27 at 00:33 +0800, Ming Lei wrote:quoted
/** * sbitmap_for_each_set() - Iterate over each set bit in a &struct sbitmap. + * @start: Where to start the iterationThanks for having changed the name of this argument ...quoted
-static inline void sbitmap_for_each_set(struct sbitmap *sb, sb_for_each_fn fn, - void *data) +static inline void __sbitmap_for_each_set(struct sbitmap *sb, + unsigned int start, + sb_for_each_fn fn, void *data) { - unsigned int i; + unsigned int index = SB_NR_TO_INDEX(sb, start); + unsigned int nr = SB_NR_TO_BIT(sb, start); + unsigned int scanned = 0;... but I'm still missing a check here whether or not index >= sb->map_nr.
The reason I didn't do that is because the only one user is always to pass correct 'start'. OK, will add it.
quoted
- for (i = 0; i < sb->map_nr; i++) { - struct sbitmap_word *word = &sb->map[i]; - unsigned int off, nr; + while (1) { + struct sbitmap_word *word = &sb->map[index]; + unsigned int depth = min_t(unsigned int, word->depth - nr, + sb->depth - scanned); + scanned += depth; if (!word->word) - continue; + goto next; - nr = 0; - off = i << sb->shift; + depth += nr; + start = index << sb->shift;The above statement reuses the argument 'start' for a new purpose. This is confusing - please don't do this. Why not to keep the name 'off'? That will keep the changes minimal.
OK, that will rename the parameter of 'start' as 'off', so that we can save one local variable. -- Ming