blk-mq: generalization and bug fixes for tag allocation
Subsystems (2)
v4 cover blk-mq: generalization and bug fixes for tag allocation
From: Omar Sandoval <[email protected]>
This is v4 of the patch series to turn blk-mq's scalable tag allocation into a generic library, fixing a few bugs along the way. This is both cleanup and preparation for coming blk-mq work.
v1 is here [1], v2 is here [2], v3 is here [3].
Changes in v4:
- Added patch 6 to fix a bug in blk-mq where shrinking the tag set doesn't stop allocations from happening to high tags.
- Changed the interface for raw sbitmap_get() to pass the allocation hint as a value, not a pointer. This pushes the hint updating out into sbitmap_queue_get() where it makes more sense.
Changes in v3:
- Renamed scale_bitmap to sbitmap
Changes in v2:
- Return -EINVAL instead of BUG_ON() if an invalid shift is passed to the initialization functions.
- Rename last_cache to alloc_hint.
- Split the wait queue allocation change into a separate patch. Patch 1 should now be a no-op.
- Add patches 3 and 4 to make the API cleaner by pushing some context from the blk-mq data structures into the common scale_bitmap code.
- Add patch 5 to randomize the allocation hint on initialization like
Applies to v4.8-rc6.
Thanks!
1: http://marc.info/?l=linux-block&m=147251402805405&w=2
2: http://marc.info/?l=linux-block&m=147329198222482&w=2
3: http://marc.info/?l=linux-block&m=147344663202718&w=2
Omar Sandoval (6):
blk-mq: abstract tag allocation out into sbitmap library
sbitmap: allocate wait queues on a specific node
sbitmap: push per-cpu last_tag into sbitmap_queue
sbitmap: push alloc policy into sbitmap_queue
sbitmap: randomize initial alloc_hint values
sbitmap: re-initialize allocation hints after resize
MAINTAINERS | 1 + block/Kconfig | 1 + block/blk-mq-tag.c | 503 +++++++++++------------------------------------- block/blk-mq-tag.h | 42 ++-- block/blk-mq.c | 114 ++++------- block/blk-mq.h | 11 -- include/linux/blk-mq.h | 9 +- include/linux/sbitmap.h | 373 +++++++++++++++++++++++++++++++++++ lib/Kconfig | 3 + lib/Makefile | 2 + lib/sbitmap.c | 347 +++++++++++++++++++++++++++++++++ 11 files changed, 889 insertions(+), 517 deletions(-) create mode 100644 include/linux/sbitmap.h create mode 100644 lib/sbitmap.c
Signature · 1 line
2.9.3
PATCH v4 1/6 blk-mq: abstract tag allocation out into sbitmap library
From: Omar Sandoval <[email protected]>
This is a generally useful data structure, so make it available to anyone else who might want to use it. It's also a nice cleanup separating the allocation logic from the rest of the tag handling logic.
The code is behind a new Kconfig option, CONFIG_SBITMAP, which is only selected by CONFIG_BLOCK for now.
This should be a complete noop functionality-wise.
--- MAINTAINERS | 1 + block/Kconfig | 1 + block/blk-mq-tag.c | 463 ++++++++++++------------------------------------ block/blk-mq-tag.h | 37 ++-- block/blk-mq.c | 112 ++++-------- block/blk-mq.h | 9 - include/linux/blk-mq.h | 9 +- include/linux/sbitmap.h | 327 ++++++++++++++++++++++++++++++++++ lib/Kconfig | 3 + lib/Makefile | 2 + lib/sbitmap.c | 301 +++++++++++++++++++++++++++++++ 11 files changed, 789 insertions(+), 476 deletions(-) create mode 100644 include/linux/sbitmap.h create mode 100644 lib/sbitmap.c
MAINTAINERS +1 -0
@@ -2452,6 +2452,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git S: Maintained F: block/ F: kernel/trace/blktrace.c +F: lib/sbitmap.c BLOCK2MTD DRIVER M: Joern Engel <[email protected]>
block/Kconfig +1 -0
@@ -4,6 +4,7 @@ menuconfig BLOCK bool "Enable the block layer" if EXPERT default y + select SBITMAP help Provide block layer support for the kernel.
block/blk-mq-tag.c +110 -353
615 lines not on this page · raw message
block/blk-mq-tag.h +10 -27
@@ -3,31 +3,6 @@#include "blk-mq.h" -enum { -BT_WAIT_QUEUES =8 , -BT_WAIT_BATCH =8 , -}; - -struct bt_wait_state { -atomic_t wait_cnt; -wait_queue_head_t wait; -}____cacheline_aligned_in_smp ; - -#define TAG_TO_INDEX (bt, tag) ((tag) >> (bt)->bits_per_word) -#define TAG_TO_BIT (bt, tag) ((tag) & ((1 << (bt)->bits_per_word) - 1)) - -struct blk_mq_bitmap_tags { -unsigned int depth; -unsigned int wake_cnt; -unsigned int bits_per_word; - -unsigned int map_nr; -struct blk_align_bitmap *map; - -atomic_t wake_index; -struct bt_wait_state *bs; -}; -/* * Tag address space map. */
@@ -37,8 +12,8 @@ struct blk_mq_tags {atomic_t active_queues; -struct blk_mq_bitmap_tags bitmap_tags; -struct blk_mq_bitmap_tags breserved_tags; +struct sbitmap_queue bitmap_tags; +struct sbitmap_queue breserved_tags;struct request **rqs;struct list_head page_list;
@@ -61,6 +36,14 @@ extern void blk_mq_tag_wakeup_all(struct blk_mq_tags *tags, bool);void blk_mq_queue_tag_busy_iter (struct request_queue *q,busy_iter_fn *fn,void *priv); +static inline struct sbq_wait_state *bt_wait_ptr (struct sbitmap_queue *bt, +struct blk_mq_hw_ctx *hctx) +{ +if (!hctx) +return &bt->ws[0 ]; +return sbq_wait_ptr (bt, &hctx->wait_index); +} +enum {BLK_MQ_TAG_CACHE_MIN =1 ,BLK_MQ_TAG_CACHE_MAX =64 ,
block/blk-mq.c +32 -80
180 lines not on this page · raw message
block/blk-mq.h +0 -9
@@ -63,15 +63,6 @@ extern void blk_mq_rq_timed_out(struct request *req, bool reserved);void blk_mq_release (struct request_queue *q); -/* -* Basic implementation of sparser bitmap, allowing the user to spread -* the bits over more cachelines. -*/ -struct blk_align_bitmap { -unsigned long word; -unsigned long depth; -}____cacheline_aligned_in_smp ; -static inline struct blk_mq_ctx *__blk_mq_get_ctx (struct request_queue *q,unsigned int cpu) {
include/linux/blk-mq.h +2 -7
@@ -2,6 +2,7 @@#define BLK_MQ_H #include <linux/blkdev.h> +#include <linux/sbitmap.h> struct blk_mq_tags ;struct blk_flush_queue ;
@@ -12,12 +13,6 @@ struct blk_mq_cpu_notifier {int (*notify)(void *data,unsigned long action,unsigned int cpu); }; -struct blk_mq_ctxmap { -unsigned int size; -unsigned int bits_per_word; -struct blk_align_bitmap *map; -}; - struct blk_mq_hw_ctx { struct { spinlock_t lock;
@@ -38,7 +33,7 @@ struct blk_mq_hw_ctx {void *driver_data; -struct blk_mq_ctxmap ctx_map; +struct sbitmap ctx_map;unsigned int nr_ctx;struct blk_mq_ctx **ctxs;
include/linux/sbitmap.h new file +327 -0
327 lines not on this page · raw message
lib/Kconfig +3 -0
@@ -550,4 +550,7 @@ config STACKDEPOT bool select STACKTRACE +config SBITMAP + bool + endmenu
lib/Makefile +2 -0
@@ -227,3 +227,5 @@ obj-$(CONFIG_UCS2_STRING) += ucs2_string.o obj-$(CONFIG_UBSAN) += ubsan.o UBSAN_SANITIZE_ubsan.o := n + +obj-$(CONFIG_SBITMAP) += sbitmap.o
lib/sbitmap.c new file +301 -0
301 lines not on this page · raw message
Signature · 1 line
2.9.3
PATCH v4 2/6 sbitmap: allocate wait queues on a specific node
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
The original bt_alloc() we converted from was using kzalloc(), not kzalloc_node(), to allocate the wait queues. This was probably an oversight, so fix it for sbitmap_queue_init_node().
--- lib/sbitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/sbitmap.c +1 -1
@@ -208,7 +208,7 @@ int sbitmap_queue_init_node(struct sbitmap_queue *sbq, unsigned int depth, sbq->wake_batch =sbq_calc_wake_batch (depth);atomic_set (&sbq->wake_index,0 ); - sbq->ws =kzalloc (SBQ_WAIT_QUEUES *sizeof (*sbq->ws), flags); + sbq->ws =kzalloc_node (SBQ_WAIT_QUEUES *sizeof (*sbq->ws), flags, node);if (!sbq->ws) {sbitmap_free (&sbq->sb);return -ENOMEM ;
Signature · 1 line
2.9.3
PATCH v4 3/6 sbitmap: push per-cpu last_tag into sbitmap_queue
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Allocating your own per-cpu allocation hint separately makes for an awkward API. Instead, allocate the per-cpu hint as part of the struct sbitmap_queue. There's no point for a struct sbitmap_queue without the cache, but you can still use a bare struct sbitmap.
--- block/blk-mq-tag.c | 53 ++++++++++++++++--------------------------------- block/blk-mq-tag.h | 3 ++- block/blk-mq.c | 2 +- block/blk-mq.h | 2 -- include/linux/sbitmap.h | 45 ++++++++++++++++++++++++++++++++++++++++- lib/sbitmap.c | 35 +++++++++++++++++++++++++++++++- 6 files changed, 98 insertions(+), 42 deletions(-)
block/blk-mq-tag.c +17 -36
@@ -94,39 +94,21 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,#define BT_ALLOC_RR (tags) (tags->alloc_policy == BLK_TAG_ALLOC_RR)static int __bt_get (struct blk_mq_hw_ctx *hctx,struct sbitmap_queue *bt, -unsigned int *tag_cache,struct blk_mq_tags *tags) +struct blk_mq_tags *tags) { -unsigned int last_tag; -int tag; -if (!hctx_may_queue (hctx, bt))return -1 ; - - last_tag = *tag_cache; - tag =sbitmap_get (&bt->sb, last_tag,BT_ALLOC_RR (tags)); - -if (tag ==-1 ) { - *tag_cache =0 ; - }else if (tag == last_tag ||unlikely (BT_ALLOC_RR (tags))) { - last_tag = tag +1 ; -if (last_tag >= bt->sb.depth -1 ) - last_tag =0 ; - *tag_cache = last_tag; - } - -return tag; +return __sbitmap_queue_get (bt,BT_ALLOC_RR (tags)); } -static int bt_get (struct blk_mq_alloc_data *data, -struct sbitmap_queue *bt, -struct blk_mq_hw_ctx *hctx, -unsigned int *last_tag,struct blk_mq_tags *tags) +static int bt_get (struct blk_mq_alloc_data *data,struct sbitmap_queue *bt, +struct blk_mq_hw_ctx *hctx,struct blk_mq_tags *tags) {struct sbq_wait_state *ws;DEFINE_WAIT (wait);int tag; - tag =__bt_get (hctx, bt, last_tag, tags); + tag =__bt_get (hctx, bt, tags);if (tag !=-1 )return tag;
@@ -137,7 +119,7 @@ static int bt_get(struct blk_mq_alloc_data *data, do { prepare_to_wait(&ws->wait, &wait, TASK_UNINTERRUPTIBLE); - tag = __bt_get(hctx, bt, last_tag, tags); + tag = __bt_get(hctx, bt, tags); if (tag != -1) break;
@@ -154,7 +136,7 @@ static int bt_get(struct blk_mq_alloc_data *data,* Retry tag allocation after running the hardware queue, * as running the queue may also have found completions. */ - tag =__bt_get (hctx, bt, last_tag, tags); + tag =__bt_get (hctx, bt, tags);if (tag !=-1 )break ;
@@ -168,7 +150,6 @@ static int bt_get(struct blk_mq_alloc_data *data,if (data->flags &BLK_MQ_REQ_RESERVED ) { bt = &data->hctx->tags->breserved_tags; }else { - last_tag = &data->ctx->last_tag; hctx = data->hctx; bt = &hctx->tags->bitmap_tags; }
@@ -185,7 +166,7 @@ static unsigned int __blk_mq_get_tag(struct blk_mq_alloc_data *data)int tag; tag =bt_get (data, &data->hctx->tags->bitmap_tags, data->hctx, - &data->ctx->last_tag, data->hctx->tags); + data->hctx->tags);if (tag >=0 )return tag + data->hctx->tags->nr_reserved_tags;
@@ -194,15 +175,15 @@ static unsigned int __blk_mq_get_tag(struct blk_mq_alloc_data *data)static unsigned int __blk_mq_get_reserved_tag (struct blk_mq_alloc_data *data) { -int tag, zero =0 ; +int tag;if (unlikely (!data->hctx->tags->nr_reserved_tags)) {WARN_ON_ONCE (1 );return BLK_MQ_TAG_FAIL ; } - tag =bt_get (data, &data->hctx->tags->breserved_tags,NULL , &zero, - data->hctx->tags); + tag =bt_get (data, &data->hctx->tags->breserved_tags,NULL , + data->hctx->tags);if (tag <0 )return BLK_MQ_TAG_FAIL ;
@@ -216,8 +197,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)return __blk_mq_get_tag (data); } -void blk_mq_put_tag (struct blk_mq_hw_ctx *hctx,unsigned int tag, -unsigned int *last_tag) +void blk_mq_put_tag (struct blk_mq_hw_ctx *hctx,struct blk_mq_ctx *ctx, +unsigned int tag) {struct blk_mq_tags *tags = hctx->tags;
@@ -225,12 +206,12 @@ void blk_mq_put_tag(struct blk_mq_hw_ctx *hctx, unsigned int tag,const int real_tag = tag - tags->nr_reserved_tags;BUG_ON (real_tag >= tags->nr_tags); -sbitmap_queue_clear (&tags->bitmap_tags, real_tag); -if (likely (tags->alloc_policy ==BLK_TAG_ALLOC_FIFO )) - *last_tag = real_tag; +sbitmap_queue_clear (&tags->bitmap_tags, real_tag, +BT_ALLOC_RR (tags), ctx->cpu); }else {BUG_ON (tag >= tags->nr_reserved_tags); -sbitmap_queue_clear (&tags->breserved_tags, tag); +sbitmap_queue_clear (&tags->breserved_tags, tag, +BT_ALLOC_RR (tags), ctx->cpu); } }
block/blk-mq-tag.h +2 -1
@@ -27,7 +27,8 @@ extern struct blk_mq_tags *blk_mq_init_tags(unsigned int nr_tags, unsigned int rextern void blk_mq_free_tags (struct blk_mq_tags *tags);extern unsigned int blk_mq_get_tag (struct blk_mq_alloc_data *data); -extern void blk_mq_put_tag (struct blk_mq_hw_ctx *hctx,unsigned int tag,unsigned int *last_tag); +extern void blk_mq_put_tag (struct blk_mq_hw_ctx *hctx,struct blk_mq_ctx *ctx, +unsigned int tag);extern bool blk_mq_has_free_tags (struct blk_mq_tags *tags);extern ssize_t blk_mq_tag_sysfs_show (struct blk_mq_tags *tags,char *page);extern void blk_mq_tag_init_last_tag (struct blk_mq_tags *tags,unsigned int *last_tag);
block/blk-mq.c +1 -1
@@ -302,7 +302,7 @@ static void __blk_mq_free_request(struct blk_mq_hw_ctx *hctx, rq->cmd_flags =0 ;clear_bit (REQ_ATOM_STARTED , &rq->atomic_flags); -blk_mq_put_tag (hctx, tag, &ctx->last_tag); +blk_mq_put_tag (hctx, ctx, tag);blk_queue_exit (q); }
block/blk-mq.h +0 -2
@@ -12,8 +12,6 @@ struct blk_mq_ctx {unsigned int cpu;unsigned int index_hw; -unsigned int last_tag____cacheline_aligned_in_smp ; -/* incremented at dispatch time */ unsigned long rq_dispatched[2 ];unsigned long rq_merged;
include/linux/sbitmap.h +44 -1
68 lines not on this page · raw message
lib/sbitmap.c +34 -1
@@ -205,11 +205,18 @@ int sbitmap_queue_init_node(struct sbitmap_queue *sbq, unsigned int depth,if (ret)return ret; + sbq->alloc_hint = alloc_percpu_gfp(unsigned int, flags); +if (!sbq->alloc_hint) { +sbitmap_free (&sbq->sb); +return -ENOMEM ; + } + sbq->wake_batch =sbq_calc_wake_batch (depth);atomic_set (&sbq->wake_index,0 ); sbq->ws =kzalloc_node (SBQ_WAIT_QUEUES *sizeof (*sbq->ws), flags, node);if (!sbq->ws) { +free_percpu (sbq->alloc_hint);sbitmap_free (&sbq->sb);return -ENOMEM ; }
@@ -229,6 +236,29 @@ void sbitmap_queue_resize(struct sbitmap_queue *sbq, unsigned int depth) }EXPORT_SYMBOL_GPL (sbitmap_queue_resize); +int __sbitmap_queue_get (struct sbitmap_queue *sbq,bool round_robin) +{ +unsigned int hint; +int nr; + + hint =this_cpu_read (*sbq->alloc_hint); + nr =sbitmap_get (&sbq->sb, hint, round_robin); + +if (nr ==-1 ) { +/* If the map is full, a hint won't do us much good. */ +this_cpu_write (*sbq->alloc_hint,0 ); + }else if (nr == hint ||unlikely (round_robin)) { +/* Only update the hint if we used it. */ + hint = nr +1 ; +if (hint >= sbq->sb.depth -1 ) + hint =0 ; +this_cpu_write (*sbq->alloc_hint, hint); + } + +return nr; +} +EXPORT_SYMBOL_GPL (__sbitmap_queue_get); +static struct sbq_wait_state *sbq_wake_ptr (struct sbitmap_queue *sbq) {int i, wake_index;
@@ -273,10 +303,13 @@ static void sbq_wake_up(struct sbitmap_queue *sbq) } } -void sbitmap_queue_clear (struct sbitmap_queue *sbq,unsigned int nr) +void sbitmap_queue_clear (struct sbitmap_queue *sbq,unsigned int nr, +bool round_robin,unsigned int cpu) {sbitmap_clear_bit (&sbq->sb, nr);sbq_wake_up (sbq); +if (likely (!round_robin)) + *per_cpu_ptr (sbq->alloc_hint, cpu) = nr; }EXPORT_SYMBOL_GPL (sbitmap_queue_clear);
Signature · 1 line
2.9.3
PATCH v4 4/6 sbitmap: push alloc policy into sbitmap_queue
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
Again, there's no point in passing this in every time. Make it part of struct sbitmap_queue and clean up the API.
--- block/blk-mq-tag.c | 33 +++++++++++++++------------------ block/blk-mq-tag.h | 1 - include/linux/sbitmap.h | 19 +++++++++++-------- lib/sbitmap.c | 14 ++++++++------ 4 files changed, 34 insertions(+), 33 deletions(-)
block/blk-mq-tag.c +15 -18
@@ -91,14 +91,11 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,return atomic_read (&hctx->nr_active) < depth; } -#define BT_ALLOC_RR (tags) (tags->alloc_policy == BLK_TAG_ALLOC_RR) - -static int __bt_get (struct blk_mq_hw_ctx *hctx,struct sbitmap_queue *bt, -struct blk_mq_tags *tags) +static int __bt_get (struct blk_mq_hw_ctx *hctx,struct sbitmap_queue *bt) {if (!hctx_may_queue (hctx, bt))return -1 ; -return __sbitmap_queue_get (bt,BT_ALLOC_RR (tags)); +return __sbitmap_queue_get (bt); } static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt,
@@ -108,7 +105,7 @@ static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt,DEFINE_WAIT (wait);int tag; - tag =__bt_get (hctx, bt, tags); + tag =__bt_get (hctx, bt);if (tag !=-1 )return tag;
@@ -119,7 +116,7 @@ static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt, do { prepare_to_wait(&ws->wait, &wait, TASK_UNINTERRUPTIBLE); - tag = __bt_get(hctx, bt, tags); + tag = __bt_get(hctx, bt); if (tag != -1) break;
@@ -136,7 +133,7 @@ static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt,* Retry tag allocation after running the hardware queue, * as running the queue may also have found completions. */ - tag =__bt_get (hctx, bt, tags); + tag =__bt_get (hctx, bt);if (tag !=-1 )break ;
@@ -206,12 +203,10 @@ void blk_mq_put_tag(struct blk_mq_hw_ctx *hctx, struct blk_mq_ctx *ctx,const int real_tag = tag - tags->nr_reserved_tags;BUG_ON (real_tag >= tags->nr_tags); -sbitmap_queue_clear (&tags->bitmap_tags, real_tag, -BT_ALLOC_RR (tags), ctx->cpu); +sbitmap_queue_clear (&tags->bitmap_tags, real_tag, ctx->cpu); }else {BUG_ON (tag >= tags->nr_reserved_tags); -sbitmap_queue_clear (&tags->breserved_tags, tag, -BT_ALLOC_RR (tags), ctx->cpu); +sbitmap_queue_clear (&tags->breserved_tags, tag, ctx->cpu); } }
@@ -363,21 +358,23 @@ static unsigned int bt_unused_tags(const struct sbitmap_queue *bt)return bt->sb.depth -sbitmap_weight (&bt->sb); } -static int bt_alloc (struct sbitmap_queue *bt,unsigned int depth,int node) +static int bt_alloc (struct sbitmap_queue *bt,unsigned int depth, +bool round_robin,int node) { -return sbitmap_queue_init_node (bt, depth,-1 ,GFP_KERNEL , node); +return sbitmap_queue_init_node (bt, depth,-1 , round_robin,GFP_KERNEL , + node); }static struct blk_mq_tags *blk_mq_init_bitmap_tags (struct blk_mq_tags *tags,int node,int alloc_policy) {unsigned int depth = tags->nr_tags - tags->nr_reserved_tags; +bool round_robin = alloc_policy ==BLK_TAG_ALLOC_RR ; - tags->alloc_policy = alloc_policy; - -if (bt_alloc (&tags->bitmap_tags, depth, node)) +if (bt_alloc (&tags->bitmap_tags, depth, round_robin, node)) goto free_tags; -if (bt_alloc (&tags->breserved_tags, tags->nr_reserved_tags, node)) +if (bt_alloc (&tags->breserved_tags, tags->nr_reserved_tags, round_robin, + node)) goto free_bitmap_tags;return tags;
block/blk-mq-tag.h +0 -1
@@ -18,7 +18,6 @@ struct blk_mq_tags {struct request **rqs;struct list_head page_list; -int alloc_policy;cpumask_var_t cpumask; };
include/linux/sbitmap.h +11 -8
@@ -122,6 +122,11 @@ struct sbitmap_queue {* @ws: Wait queues. */ struct sbq_wait_state *ws; + +/** +* @round_robin: Allocate bits in strict round-robin order. +*/ +bool round_robin; };/**
@@ -259,13 +264,14 @@ unsigned int sbitmap_weight(const struct sbitmap *sb);* @sbq: Bitmap queue to initialize. * @depth: See sbitmap_init_node(). * @shift: See sbitmap_init_node(). +* @round_robin: See sbitmap_get(). * @flags: Allocation flags. * @node: Memory node to allocate on. * * Return: Zero on success or negative errno on failure. */ int sbitmap_queue_init_node (struct sbitmap_queue *sbq,unsigned int depth, -int shift,gfp_t flags,int node); +int shift,bool round_robin,gfp_t flags,int node);/** * sbitmap_queue_free() - Free memory used by a &struct sbitmap_queue.
@@ -294,29 +300,27 @@ void sbitmap_queue_resize(struct sbitmap_queue *sbq, unsigned int depth);* __sbitmap_queue_get() - Try to allocate a free bit from a &struct * sbitmap_queue with preemption already disabled. * @sbq: Bitmap queue to allocate from. -* @round_robin: See sbitmap_get(). * * Return: Non-negative allocated bit number if successful, -1 otherwise. */ -int __sbitmap_queue_get (struct sbitmap_queue *sbq,bool round_robin); +int __sbitmap_queue_get (struct sbitmap_queue *sbq);/** * sbitmap_queue_get() - Try to allocate a free bit from a &struct * sbitmap_queue. * @sbq: Bitmap queue to allocate from. -* @round_robin: See sbitmap_get(). * @cpu: Output parameter; will contain the CPU we ran on (e.g., to be passed to * sbitmap_queue_clear()). * * Return: Non-negative allocated bit number if successful, -1 otherwise. */ -static inline int sbitmap_queue_get (struct sbitmap_queue *sbq,bool round_robin, +static inline int sbitmap_queue_get (struct sbitmap_queue *sbq,unsigned int *cpu) {int nr; *cpu =get_cpu (); - nr =__sbitmap_queue_get (sbq, round_robin); + nr =__sbitmap_queue_get (sbq);put_cpu ();return nr; }
@@ -326,11 +330,10 @@ static inline int sbitmap_queue_get(struct sbitmap_queue *sbq, bool round_robin,* &struct sbitmap_queue. * @sbq: Bitmap to free from. * @nr: Bit number to free. -* @round_robin: See sbitmap_get(). * @cpu: CPU the bit was allocated on. */ void sbitmap_queue_clear (struct sbitmap_queue *sbq,unsigned int nr, -bool round_robin,unsigned int cpu); +unsigned int cpu);static inline int sbq_index_inc (int index) {
lib/sbitmap.c +8 -6
@@ -196,7 +196,7 @@ static unsigned int sbq_calc_wake_batch(unsigned int depth) }int sbitmap_queue_init_node (struct sbitmap_queue *sbq,unsigned int depth, -int shift,gfp_t flags,int node) +int shift,bool round_robin,gfp_t flags,int node) {int ret;int i;
@@ -225,6 +225,8 @@ int sbitmap_queue_init_node(struct sbitmap_queue *sbq, unsigned int depth,init_waitqueue_head (&sbq->ws[i].wait);atomic_set (&sbq->ws[i].wait_cnt, sbq->wake_batch); } + + sbq->round_robin = round_robin;return 0 ; }EXPORT_SYMBOL_GPL (sbitmap_queue_init_node);
@@ -236,18 +238,18 @@ void sbitmap_queue_resize(struct sbitmap_queue *sbq, unsigned int depth) }EXPORT_SYMBOL_GPL (sbitmap_queue_resize); -int __sbitmap_queue_get (struct sbitmap_queue *sbq,bool round_robin) +int __sbitmap_queue_get (struct sbitmap_queue *sbq) {unsigned int hint;int nr; hint =this_cpu_read (*sbq->alloc_hint); - nr =sbitmap_get (&sbq->sb, hint, round_robin); + nr =sbitmap_get (&sbq->sb, hint, sbq->round_robin);if (nr ==-1 ) {/* If the map is full, a hint won't do us much good. */ this_cpu_write (*sbq->alloc_hint,0 ); - }else if (nr == hint ||unlikely (round_robin)) { + }else if (nr == hint ||unlikely (sbq->round_robin)) {/* Only update the hint if we used it. */ hint = nr +1 ;if (hint >= sbq->sb.depth -1 )
@@ -304,11 +306,11 @@ static void sbq_wake_up(struct sbitmap_queue *sbq) }void sbitmap_queue_clear (struct sbitmap_queue *sbq,unsigned int nr, -bool round_robin,unsigned int cpu) +unsigned int cpu) {sbitmap_clear_bit (&sbq->sb, nr);sbq_wake_up (sbq); -if (likely (!round_robin)) +if (likely (!sbq->round_robin)) *per_cpu_ptr (sbq->alloc_hint, cpu) = nr; }EXPORT_SYMBOL_GPL (sbitmap_queue_clear);
Signature · 1 line
2.9.3
PATCH v4 5/6 sbitmap: randomize initial alloc_hint values
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
In order to get good cache behavior from a sbitmap, we want each CPU to stick to its own cacheline(s) as much as possible. This might happen naturally as the bitmap gets filled up and the alloc_hint values spread out, but we really want this behavior from the start. blk-mq apparently intended to do this, but the code to do this was never wired up. Get rid of the dead code and make it part of the sbitmap library.
--- block/blk-mq-tag.c | 8 -------- block/blk-mq-tag.h | 1 - lib/sbitmap.c | 6 ++++++ 3 files changed, 6 insertions(+), 9 deletions(-)
block/blk-mq-tag.c +0 -8
@@ -7,7 +7,6 @@*/ #include <linux/kernel.h> #include <linux/module.h> -#include <linux/random.h> #include <linux/blk-mq.h> #include "blk.h"
@@ -419,13 +418,6 @@ void blk_mq_free_tags(struct blk_mq_tags *tags)kfree (tags); } -void blk_mq_tag_init_last_tag (struct blk_mq_tags *tags,unsigned int *tag) -{ -unsigned int depth = tags->nr_tags - tags->nr_reserved_tags; - - *tag =prandom_u32 () % depth; -} -int blk_mq_tag_update_depth (struct blk_mq_tags *tags,unsigned int tdepth) { tdepth -= tags->nr_reserved_tags;
block/blk-mq-tag.h +0 -1
@@ -30,7 +30,6 @@ extern void blk_mq_put_tag(struct blk_mq_hw_ctx *hctx, struct blk_mq_ctx *ctx, unsigned int tag);extern bool blk_mq_has_free_tags (struct blk_mq_tags *tags);extern ssize_t blk_mq_tag_sysfs_show (struct blk_mq_tags *tags,char *page); -extern void blk_mq_tag_init_last_tag (struct blk_mq_tags *tags,unsigned int *last_tag);extern int blk_mq_tag_update_depth (struct blk_mq_tags *tags,unsigned int depth);extern void blk_mq_tag_wakeup_all (struct blk_mq_tags *tags,bool ); void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,
lib/sbitmap.c +6 -0
@@ -15,6 +15,7 @@* along with this program. If not, see <https://www.gnu.org/licenses/>. */ +#include <linux/random.h> #include <linux/sbitmap.h> int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
@@ -211,6 +212,11 @@ int sbitmap_queue_init_node(struct sbitmap_queue *sbq, unsigned int depth,return -ENOMEM ; } +if (depth && !round_robin) { +for_each_possible_cpu (i) + *per_cpu_ptr (sbq->alloc_hint, i) =prandom_u32 () % depth; + } + sbq->wake_batch =sbq_calc_wake_batch (depth);atomic_set (&sbq->wake_index,0 );
Signature · 1 line
2.9.3
PATCH v4 6/6 sbitmap: re-initialize allocation hints after resize
In reply to Omar SandovalFrom: Omar Sandoval <[email protected]>
After a struct sbitmap_queue is resized smaller, the allocation hints may still be set to bits beyond the new depth of the bitmap. This means that, for example, if the number of blk-mq tags is reduced through sysfs, more requests than the nominal queue depth may be in flight.
It's tempting to fix this at resize time by doing a one-time reinitialization of the hints, but this can race with __sbitmap_queue_get() updating the hint. Instead, check the hint before we use it. This caused no measurable performance difference in my synthetic benchmarks.
--- lib/sbitmap.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
lib/sbitmap.c +7 -2
@@ -246,10 +246,15 @@ EXPORT_SYMBOL_GPL(sbitmap_queue_resize);int __sbitmap_queue_get (struct sbitmap_queue *sbq) { -unsigned int hint; +unsigned int hint, depth;int nr; hint =this_cpu_read (*sbq->alloc_hint); + depth =READ_ONCE (sbq->sb.depth); +if (unlikely (hint >= depth)) { + hint = depth ?prandom_u32 () % depth :0 ; +this_cpu_write (*sbq->alloc_hint, hint); + } nr =sbitmap_get (&sbq->sb, hint, sbq->round_robin);if (nr ==-1 ) {
@@ -258,7 +263,7 @@ int __sbitmap_queue_get(struct sbitmap_queue *sbq) }else if (nr == hint ||unlikely (sbq->round_robin)) {/* Only update the hint if we used it. */ hint = nr +1 ; -if (hint >= sbq->sb.depth -1 ) +if (hint >= depth -1 ) hint =0 ;this_cpu_write (*sbq->alloc_hint, hint); }
Signature · 1 line
2.9.3
Quoted 5 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
This is v4 of the patch series to turn blk-mq's scalable tag allocation into a generic library, fixing a few bugs along the way. This is both cleanup and preparation for coming blk-mq work.
Thanks Omar, applied for 4.9.
Signature · 1 line
Jens Axboe
PATCH 1/1 sbitmap: don't update the allocation hint on clear after resize
From: Omar Sandoval <[email protected]>
If we have a bunch of high-numbered bits allocated and then we resize the struct sbitmap_queue, when those bits get cleared, we'll update the hint and then have to re-randomize it repeatedly. Avoid that by checking that the cleared bit is still a valid hint. No measurable performance difference in the common case.
---
Jens,
Small tweak to patch 6 that occurred to me after I sent the series out. Feel free to fold it in to patch 6 or apply it separately as you see fit.
Thanks!
lib/sbitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/sbitmap.c +1 -1
@@ -321,7 +321,7 @@ void sbitmap_queue_clear(struct sbitmap_queue *sbq, unsigned int nr, {sbitmap_clear_bit (&sbq->sb, nr);sbq_wake_up (sbq); -if (likely (!sbq->round_robin)) +if (likely (!sbq->round_robin && nr < sbq->sb.depth)) *per_cpu_ptr (sbq->alloc_hint, cpu) = nr; }EXPORT_SYMBOL_GPL (sbitmap_queue_clear);
Signature · 1 line
2.9.3
Re: [PATCH] sbitmap: don't update the allocation hint on clear after resize
Quoted 8 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
If we have a bunch of high-numbered bits allocated and then we resize the struct
sbitmap_queue, when those bits get cleared, we'll update the hint and then have to re-randomize it repeatedly. Avoid that by checking that the cleared bit is still a valid hint. No measurable performance difference in the common case.
Quoted 7 lines
Signed-off-by Omar Sandoval <[email protected]>---
Jens,Small tweak to patch 6 that occurred to me after I sent the series out. Feel free to fold it in to patch 6 or apply it separately as you see fit.
No worries. I can't fold since the series was already applied, so I just added the extra patch.
Signature · 1 line
Jens Axboe
Re: [PATCH] sbitmap: don't update the allocation hint on clear after resize
Quoted 13 lines · Jens Axboe
Quoted 12 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
If we have a bunch of high-numbered bits allocated and then we resize the struct
sbitmap_queue, when those bits get cleared, we'll update the hint and then have to re-randomize it repeatedly. Avoid that by checking that the cleared bit is still a valid hint. No measurable performance difference in the common case.Signed-off-by Omar Sandoval <[email protected]>---
Jens,
Quoted 7 lines
Quoted 3 lines
Small tweak to patch 6 that occurred to me after I sent the series out. Feel free to fold it in to patch 6 or apply it separately as you see fit.
No worries. I can't fold since the series was already applied, so I just added the extra patch.
Okay with me, thanks!
Signature · 1 line
Omar