Thread (28 messages) 28 messages, 2 authors, 2017-09-01

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 iteration
Thanks 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help