blk-mq: fix false negative out-of-tags condition
authorJens Axboe <axboe@fb.com>
Wed, 14 Jan 2015 15:49:55 +0000 (08:49 -0700)
committerJens Axboe <axboe@fb.com>
Wed, 14 Jan 2015 15:49:55 +0000 (08:49 -0700)
The blk-mq tagging tries to maintain some locality between CPUs and
the tags issued. The tags are split into groups of words, and the
words may not be fully populated. When searching for a new free tag,
blk-mq may look at partial words, hence it passes in an offset/size
to find_next_zero_bit(). However, it does that wrong, the size must
always be the full length of the number of tags in that word,
otherwise we'll potentially miss some near the end.

Another issue is when __bt_get() goes from one word set to the next.
It bumps the index, but not the last_tag associated with the
previous index. Bump that to be in the range of the new word.

Finally, clean up __bt_get() and __bt_get_word() a bit and get
rid of the goto in there, and the unnecessary 'wrap' variable.

Signed-off-by: Jens Axboe <axboe@fb.com>
block/blk-mq-tag.c

index 60c9d4a93fe470ced7471cd8653d8a00fc8922d7..d4daee385a235ebd7c8bb86c3ecbaef74e46f233 100644 (file)
@@ -142,29 +142,30 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
 
 static int __bt_get_word(struct blk_align_bitmap *bm, unsigned int last_tag)
 {
-       int tag, org_last_tag, end;
-       bool wrap = last_tag != 0;
+       int tag, org_last_tag = last_tag;
 
-       org_last_tag = last_tag;
-       end = bm->depth;
-       do {
-restart:
-               tag = find_next_zero_bit(&bm->word, end, last_tag);
-               if (unlikely(tag >= end)) {
+       while (1) {
+               tag = find_next_zero_bit(&bm->word, bm->depth, last_tag);
+               if (unlikely(tag >= bm->depth)) {
                        /*
-                        * We started with an offset, start from 0 to
+                        * We started with an offset, and we didn't reset the
+                        * offset to 0 in a failure case, so start from 0 to
                         * exhaust the map.
                         */
-                       if (wrap) {
-                               wrap = false;
-                               end = org_last_tag;
-                               last_tag = 0;
-                               goto restart;
+                       if (org_last_tag && last_tag) {
+                               last_tag = org_last_tag = 0;
+                               continue;
                        }
                        return -1;
                }
+
+               if (!test_and_set_bit(tag, &bm->word))
+                       break;
+
                last_tag = tag + 1;
-       } while (test_and_set_bit(tag, &bm->word));
+               if (last_tag >= bm->depth - 1)
+                       last_tag = 0;
+       }
 
        return tag;
 }
@@ -199,9 +200,17 @@ static int __bt_get(struct blk_mq_hw_ctx *hctx, struct blk_mq_bitmap_tags *bt,
                        goto done;
                }
 
-               last_tag = 0;
-               if (++index >= bt->map_nr)
+               /*
+                * Jump to next index, and reset the last tag to be the
+                * first tag of that index
+                */
+               index++;
+               last_tag = (index << bt->bits_per_word);
+
+               if (index >= bt->map_nr) {
                        index = 0;
+                       last_tag = 0;
+               }
        }
 
        *tag_cache = 0;