* [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
2022-03-02 12:39 ` John Garry
2022-03-02 12:14 ` [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit Ming Lei
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig
The current code always uses default queue map and hw queue index
for figuring out the numa node for hw queue, this way isn't correct
because blk-mq supports three queue maps, and the correct queue map
should be used for the specified hw queue.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a05ce7725031..5d25abf9e551 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3107,15 +3107,41 @@ void blk_mq_free_rq_map(struct blk_mq_tags *tags)
blk_mq_free_tags(tags);
}
+static enum hctx_type hctx_idx_to_type(struct blk_mq_tag_set *set,
+ unsigned int hctx_idx)
+{
+ int i;
+
+ for (i = 0; i < set->nr_maps; i++) {
+ unsigned int start = set->map[i].queue_offset;
+ unsigned int end = start + set->map[i].nr_queues;
+
+ if (hctx_idx >= start && hctx_idx < end)
+ break;
+ }
+
+ if (i >= set->nr_maps)
+ i = HCTX_TYPE_DEFAULT;
+
+ return i;
+}
+
+static int blk_mq_get_hctx_node(struct blk_mq_tag_set *set,
+ unsigned int hctx_idx)
+{
+ enum hctx_type type = hctx_idx_to_type(set, hctx_idx);
+
+ return blk_mq_hw_queue_to_node(&set->map[type], hctx_idx);
+}
+
static struct blk_mq_tags *blk_mq_alloc_rq_map(struct blk_mq_tag_set *set,
unsigned int hctx_idx,
unsigned int nr_tags,
unsigned int reserved_tags)
{
+ int node = blk_mq_get_hctx_node(set, hctx_idx);
struct blk_mq_tags *tags;
- int node;
- node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], hctx_idx);
if (node == NUMA_NO_NODE)
node = set->numa_node;
@@ -3164,10 +3190,9 @@ static int blk_mq_alloc_rqs(struct blk_mq_tag_set *set,
unsigned int hctx_idx, unsigned int depth)
{
unsigned int i, j, entries_per_page, max_order = 4;
+ int node = blk_mq_get_hctx_node(set, hctx_idx);
size_t rq_size, left;
- int node;
- node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], hctx_idx);
if (node == NUMA_NO_NODE)
node = set->numa_node;
@@ -3941,10 +3966,9 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
/* protect against switching io scheduler */
mutex_lock(&q->sysfs_lock);
for (i = 0; i < set->nr_hw_queues; i++) {
- int node;
+ int node = blk_mq_get_hctx_node(set, i);
struct blk_mq_hw_ctx *hctx;
- node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], i);
/*
* If the hw queue has been mapped to another numa node,
* we need to realloc the hctx. If allocation fails, fallback
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue
2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
@ 2022-03-02 12:39 ` John Garry
0 siblings, 0 replies; 12+ messages in thread
From: John Garry @ 2022-03-02 12:39 UTC (permalink / raw)
To: Ming Lei, Jens Axboe; +Cc: linux-block, Yu Kuai, Christoph Hellwig
On 02/03/2022 12:14, Ming Lei wrote:
> The current code always uses default queue map and hw queue index
> for figuring out the numa node for hw queue, this way isn't correct
> because blk-mq supports three queue maps, and the correct queue map
> should be used for the specified hw queue.
>
> Reviewed-by: Christoph Hellwig<hch@lst.de>
> Signed-off-by: Ming Lei<ming.lei@redhat.com>
FWIW:
Reviewed-by: John Garry <john.garry@huawei.com>
thanks
> ---
> block/blk-mq.c | 36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig
blk_mq_alloc_and_init_hctx() has already taken reuse into account, so
no need to do it outside, then we can simplify blk_mq_realloc_hw_ctxs().
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 5d25abf9e551..64fddb36c93c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3966,29 +3966,24 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
/* protect against switching io scheduler */
mutex_lock(&q->sysfs_lock);
for (i = 0; i < set->nr_hw_queues; i++) {
+ int old_node;
int node = blk_mq_get_hctx_node(set, i);
- struct blk_mq_hw_ctx *hctx;
+ struct blk_mq_hw_ctx *old_hctx = hctxs[i];
- /*
- * If the hw queue has been mapped to another numa node,
- * we need to realloc the hctx. If allocation fails, fallback
- * to use the previous one.
- */
- if (hctxs[i] && (hctxs[i]->numa_node == node))
- continue;
+ if (old_hctx) {
+ old_node = old_hctx->numa_node;
+ blk_mq_exit_hctx(q, set, old_hctx, i);
+ }
- hctx = blk_mq_alloc_and_init_hctx(set, q, i, node);
- if (hctx) {
- if (hctxs[i])
- blk_mq_exit_hctx(q, set, hctxs[i], i);
- hctxs[i] = hctx;
- } else {
- if (hctxs[i])
- pr_warn("Allocate new hctx on node %d fails,\
- fallback to previous one on node %d\n",
- node, hctxs[i]->numa_node);
- else
+ hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i, node);
+ if (!hctxs[i]) {
+ if (!old_hctx)
break;
+ pr_warn("Allocate new hctx on node %d fails, fallback to previous one on node %d\n",
+ node, old_node);
+ hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i,
+ old_node);
+ WARN_ON_ONCE(!hctxs[i]);
}
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
2022-03-02 12:14 ` [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
2022-03-03 15:20 ` John Garry
2022-03-02 12:14 ` [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx Ming Lei
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei
queue map can be changed when updating nr_hw_queues, so we need to
reconfigure queue's poll capability. Add one helper for doing this job.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 64fddb36c93c..57ae9df0f4dc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4010,6 +4010,17 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
mutex_unlock(&q->sysfs_lock);
}
+static void blk_mq_update_poll_flag(struct request_queue *q)
+{
+ struct blk_mq_tag_set *set = q->tag_set;
+
+ if (set->nr_maps > HCTX_TYPE_POLL &&
+ set->map[HCTX_TYPE_POLL].nr_queues)
+ blk_queue_flag_set(QUEUE_FLAG_POLL, q);
+ else
+ blk_queue_flag_clear(QUEUE_FLAG_POLL, q);
+}
+
int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
struct request_queue *q)
{
@@ -4044,9 +4055,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
q->tag_set = set;
q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
- if (set->nr_maps > HCTX_TYPE_POLL &&
- set->map[HCTX_TYPE_POLL].nr_queues)
- blk_queue_flag_set(QUEUE_FLAG_POLL, q);
+ blk_mq_update_poll_flag(q);
INIT_DELAYED_WORK(&q->requeue_work, blk_mq_requeue_work);
INIT_LIST_HEAD(&q->requeue_list);
@@ -4512,6 +4521,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
blk_mq_update_queue_map(set);
list_for_each_entry(q, &set->tag_list, tag_set_list) {
blk_mq_realloc_hw_ctxs(set, q);
+ blk_mq_update_poll_flag(q);
if (q->nr_hw_queues != set->nr_hw_queues) {
int i = prev_nr_hw_queues;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed
2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
@ 2022-03-03 15:20 ` John Garry
0 siblings, 0 replies; 12+ messages in thread
From: John Garry @ 2022-03-03 15:20 UTC (permalink / raw)
To: Ming Lei, Jens Axboe; +Cc: linux-block, Yu Kuai
On 02/03/2022 12:14, Ming Lei wrote:
> queue map can be changed when updating nr_hw_queues, so we need to
> reconfigure queue's poll capability. Add one helper for doing this job.
nvme_mpath_alloc_disk() seems to have similar code to this helper, so
maybe we can reuse this helper function there (obviously it would need
to made public)
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
Regardless of comment:
Reviewed-by: John Garry <john.garry@huawei.com>
> ---
> block/blk-mq.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 64fddb36c93c..57ae9df0f4dc 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4010,6 +4010,17 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
> mutex_unlock(&q->sysfs_lock);
> }
>
> +static void blk_mq_update_poll_flag(struct request_queue *q)
> +{
> + struct blk_mq_tag_set *set = q->tag_set;
> +
> + if (set->nr_maps > HCTX_TYPE_POLL &&
> + set->map[HCTX_TYPE_POLL].nr_queues)
> + blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> + else
> + blk_queue_flag_clear(QUEUE_FLAG_POLL, q);
> +}
> +
> int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> struct request_queue *q)
> {
> @@ -4044,9 +4055,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> q->tag_set = set;
>
> q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
> - if (set->nr_maps > HCTX_TYPE_POLL &&
> - set->map[HCTX_TYPE_POLL].nr_queues)
> - blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> + blk_mq_update_poll_flag(q);
>
> INIT_DELAYED_WORK(&q->requeue_work, blk_mq_requeue_work);
> INIT_LIST_HEAD(&q->requeue_list);
> @@ -4512,6 +4521,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
> blk_mq_update_queue_map(set);
> list_for_each_entry(q, &set->tag_list, tag_set_list) {
> blk_mq_realloc_hw_ctxs(set, q);
> + blk_mq_update_poll_flag(q);
> if (q->nr_hw_queues != set->nr_hw_queues) {
> int i = prev_nr_hw_queues;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
` (2 preceding siblings ...)
2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
2022-03-02 12:14 ` [PATCH V2 6/6] blk-mq: manage hctx map " Ming Lei
5 siblings, 0 replies; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig
q->queue_hw_ctx is really one blk-mq internal structure for retrieving
hctx via its index, not supposed to be used by drivers. Meantime drivers
can get the tags structure easily from tagset.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/mtip32xx/mtip32xx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index cba956881d55..2d43ab5b5cc0 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -160,9 +160,7 @@ static bool mtip_check_surprise_removal(struct driver_data *dd)
static struct mtip_cmd *mtip_cmd_from_tag(struct driver_data *dd,
unsigned int tag)
{
- struct blk_mq_hw_ctx *hctx = dd->queue->queue_hw_ctx[0];
-
- return blk_mq_rq_to_pdu(blk_mq_tag_to_rq(hctx->tags, tag));
+ return blk_mq_rq_to_pdu(blk_mq_tag_to_rq(dd->tags.tags[0], tag));
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
` (3 preceding siblings ...)
2022-03-02 12:14 ` [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
2022-03-03 9:08 ` kernel test robot
2022-03-02 12:14 ` [PATCH V2 6/6] blk-mq: manage hctx map " Ming Lei
5 siblings, 1 reply; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei
It is inevitable to cause use-after-free on q->queue_hw_ctx between
queue_for_each_hw_ctx() and blk_mq_update_nr_hw_queues(). And converting
to xarray can fix the uaf, meantime code gets cleaner.
Prepare for converting q->queue_hctx_ctx into xarray, one thing is that
xa_for_each() can only accept 'unsigned long' as index, so changes type
of hctx index of queue_for_each_hw_ctx() into 'unsigned long'.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-debugfs.c | 6 +++---
block/blk-mq-sched.c | 9 +++++----
block/blk-mq-sysfs.c | 10 ++++++----
block/blk-mq-tag.c | 2 +-
block/blk-mq.c | 30 ++++++++++++++++--------------
drivers/block/rnbd/rnbd-clt.c | 2 +-
6 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 3a790eb4995c..e2880f6deb34 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -707,7 +707,7 @@ static void debugfs_create_files(struct dentry *parent, void *data,
void blk_mq_debugfs_register(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
debugfs_create_files(q->debugfs_dir, q, blk_mq_debugfs_queue_attrs);
@@ -780,7 +780,7 @@ void blk_mq_debugfs_unregister_hctx(struct blk_mq_hw_ctx *hctx)
void blk_mq_debugfs_register_hctxs(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_debugfs_register_hctx(q, hctx);
@@ -789,7 +789,7 @@ void blk_mq_debugfs_register_hctxs(struct request_queue *q)
void blk_mq_debugfs_unregister_hctxs(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_debugfs_unregister_hctx(hctx);
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 55488ba97823..e6ad8f761474 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -515,7 +515,7 @@ static void blk_mq_exit_sched_shared_tags(struct request_queue *queue)
static void blk_mq_sched_tags_teardown(struct request_queue *q, unsigned int flags)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i) {
if (hctx->sched_tags) {
@@ -550,9 +550,10 @@ static int blk_mq_init_sched_shared_tags(struct request_queue *queue)
int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e)
{
- unsigned int i, flags = q->tag_set->flags;
+ unsigned int flags = q->tag_set->flags;
struct blk_mq_hw_ctx *hctx;
struct elevator_queue *eq;
+ unsigned long i;
int ret;
if (!e) {
@@ -618,7 +619,7 @@ int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e)
void blk_mq_sched_free_rqs(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
if (blk_mq_is_shared_tags(q->tag_set->flags)) {
blk_mq_free_rqs(q->tag_set, q->sched_shared_tags,
@@ -635,7 +636,7 @@ void blk_mq_sched_free_rqs(struct request_queue *q)
void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e)
{
struct blk_mq_hw_ctx *hctx;
- unsigned int i;
+ unsigned long i;
unsigned int flags = 0;
queue_for_each_hw_ctx(q, hctx, i) {
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 674786574075..8d8c71c77ff8 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -206,7 +206,7 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx)
void blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
lockdep_assert_held(&q->sysfs_dir_lock);
@@ -255,7 +255,8 @@ void blk_mq_sysfs_init(struct request_queue *q)
int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int ret, i;
+ unsigned long i;
+ int ret;
WARN_ON_ONCE(!q->kobj.parent);
lockdep_assert_held(&q->sysfs_dir_lock);
@@ -290,7 +291,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
void blk_mq_sysfs_unregister(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
mutex_lock(&q->sysfs_dir_lock);
if (!q->mq_sysfs_init_done)
@@ -306,7 +307,8 @@ void blk_mq_sysfs_unregister(struct request_queue *q)
int blk_mq_sysfs_register(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i, ret = 0;
+ unsigned long i;
+ int ret = 0;
mutex_lock(&q->sysfs_dir_lock);
if (!q->mq_sysfs_init_done)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 0fd409b8e86e..1850a4225e12 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -515,7 +515,7 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_tag_iter_fn *fn,
bt_for_each(NULL, q, btags, fn, priv, false);
} else {
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i) {
struct blk_mq_tags *tags = hctx->tags;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 57ae9df0f4dc..bffdd71c670d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -312,7 +312,7 @@ EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
void blk_mq_wake_waiters(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- unsigned int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
if (blk_mq_hw_queue_mapped(hctx))
@@ -1442,7 +1442,7 @@ static void blk_mq_timeout_work(struct work_struct *work)
container_of(work, struct request_queue, timeout_work);
unsigned long next = 0;
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
/* A deadlock might occur if a request is stuck requiring a
* timeout at the same time a queue freeze is waiting
@@ -2143,7 +2143,7 @@ static struct blk_mq_hw_ctx *blk_mq_get_sq_hctx(struct request_queue *q)
void blk_mq_run_hw_queues(struct request_queue *q, bool async)
{
struct blk_mq_hw_ctx *hctx, *sq_hctx;
- int i;
+ unsigned long i;
sq_hctx = NULL;
if (blk_mq_has_sqsched(q))
@@ -2171,7 +2171,7 @@ EXPORT_SYMBOL(blk_mq_run_hw_queues);
void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs)
{
struct blk_mq_hw_ctx *hctx, *sq_hctx;
- int i;
+ unsigned long i;
sq_hctx = NULL;
if (blk_mq_has_sqsched(q))
@@ -2209,7 +2209,7 @@ EXPORT_SYMBOL(blk_mq_delay_run_hw_queues);
bool blk_mq_queue_stopped(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
if (blk_mq_hctx_stopped(hctx))
@@ -2248,7 +2248,7 @@ EXPORT_SYMBOL(blk_mq_stop_hw_queue);
void blk_mq_stop_hw_queues(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_stop_hw_queue(hctx);
@@ -2266,7 +2266,7 @@ EXPORT_SYMBOL(blk_mq_start_hw_queue);
void blk_mq_start_hw_queues(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_start_hw_queue(hctx);
@@ -2286,7 +2286,7 @@ EXPORT_SYMBOL_GPL(blk_mq_start_stopped_hw_queue);
void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_start_stopped_hw_queue(hctx, async);
@@ -3446,7 +3446,7 @@ static void blk_mq_exit_hw_queues(struct request_queue *q,
struct blk_mq_tag_set *set, int nr_queue)
{
struct blk_mq_hw_ctx *hctx;
- unsigned int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i) {
if (i == nr_queue)
@@ -3637,7 +3637,8 @@ static void __blk_mq_free_map_and_rqs(struct blk_mq_tag_set *set,
static void blk_mq_map_swqueue(struct request_queue *q)
{
- unsigned int i, j, hctx_idx;
+ unsigned int j, hctx_idx;
+ unsigned long i;
struct blk_mq_hw_ctx *hctx;
struct blk_mq_ctx *ctx;
struct blk_mq_tag_set *set = q->tag_set;
@@ -3744,7 +3745,7 @@ static void blk_mq_map_swqueue(struct request_queue *q)
static void queue_set_hctx_shared(struct request_queue *q, bool shared)
{
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i) {
if (shared) {
@@ -3844,7 +3845,7 @@ static int blk_mq_alloc_ctxs(struct request_queue *q)
void blk_mq_release(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx, *next;
- int i;
+ unsigned long i;
queue_for_each_hw_ctx(q, hctx, i)
WARN_ON_ONCE(hctx && list_empty(&hctx->hctx_list));
@@ -4362,7 +4363,8 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
{
struct blk_mq_tag_set *set = q->tag_set;
struct blk_mq_hw_ctx *hctx;
- int i, ret;
+ int ret;
+ unsigned long i;
if (!set)
return -EINVAL;
@@ -4738,7 +4740,7 @@ void blk_mq_cancel_work_sync(struct request_queue *q)
{
if (queue_is_mq(q)) {
struct blk_mq_hw_ctx *hctx;
- int i;
+ unsigned long i;
cancel_delayed_work_sync(&q->requeue_work);
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index c08971de369f..58304f978e10 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -1343,7 +1343,7 @@ static inline void rnbd_init_hw_queue(struct rnbd_clt_dev *dev,
static void rnbd_init_mq_hw_queues(struct rnbd_clt_dev *dev)
{
- int i;
+ unsigned long i;
struct blk_mq_hw_ctx *hctx;
struct rnbd_queue *q;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
@ 2022-03-03 9:08 ` kernel test robot
2022-03-03 9:31 ` Ming Lei
0 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2022-03-03 9:08 UTC (permalink / raw)
To: Ming Lei, Jens Axboe; +Cc: kbuild-all, linux-block, Yu Kuai, Ming Lei
Hi Ming,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on v5.17-rc6 next-20220302]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031651.z0z86F6E-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
vim +282 block/blk-mq-sysfs.c
67aec14ce87fe2 Jens Axboe 2014-05-30 254
2d0364c8c1a97a Bart Van Assche 2017-04-26 255 int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
320ae51feed5c2 Jens Axboe 2013-10-24 256 {
320ae51feed5c2 Jens Axboe 2013-10-24 257 struct blk_mq_hw_ctx *hctx;
44849be579332c Ming Lei 2022-03-02 258 unsigned long i;
44849be579332c Ming Lei 2022-03-02 259 int ret;
320ae51feed5c2 Jens Axboe 2013-10-24 260
2d0364c8c1a97a Bart Van Assche 2017-04-26 261 WARN_ON_ONCE(!q->kobj.parent);
cecf5d87ff2035 Ming Lei 2019-08-27 262 lockdep_assert_held(&q->sysfs_dir_lock);
4593fdbe7a2f44 Akinobu Mita 2015-09-27 263
1db4909e76f64a Ming Lei 2018-11-20 264 ret = kobject_add(q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
320ae51feed5c2 Jens Axboe 2013-10-24 265 if (ret < 0)
4593fdbe7a2f44 Akinobu Mita 2015-09-27 266 goto out;
320ae51feed5c2 Jens Axboe 2013-10-24 267
1db4909e76f64a Ming Lei 2018-11-20 268 kobject_uevent(q->mq_kobj, KOBJ_ADD);
320ae51feed5c2 Jens Axboe 2013-10-24 269
320ae51feed5c2 Jens Axboe 2013-10-24 270 queue_for_each_hw_ctx(q, hctx, i) {
67aec14ce87fe2 Jens Axboe 2014-05-30 271 ret = blk_mq_register_hctx(hctx);
320ae51feed5c2 Jens Axboe 2013-10-24 272 if (ret)
f05d1ba7871a2c Bart Van Assche 2017-04-26 273 goto unreg;
320ae51feed5c2 Jens Axboe 2013-10-24 274 }
320ae51feed5c2 Jens Axboe 2013-10-24 275
4593fdbe7a2f44 Akinobu Mita 2015-09-27 276 q->mq_sysfs_init_done = true;
2d0364c8c1a97a Bart Van Assche 2017-04-26 277
4593fdbe7a2f44 Akinobu Mita 2015-09-27 278 out:
2d0364c8c1a97a Bart Van Assche 2017-04-26 279 return ret;
f05d1ba7871a2c Bart Van Assche 2017-04-26 280
f05d1ba7871a2c Bart Van Assche 2017-04-26 281 unreg:
f05d1ba7871a2c Bart Van Assche 2017-04-26 @282 while (--i >= 0)
f05d1ba7871a2c Bart Van Assche 2017-04-26 283 blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
f05d1ba7871a2c Bart Van Assche 2017-04-26 284
1db4909e76f64a Ming Lei 2018-11-20 285 kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
1db4909e76f64a Ming Lei 2018-11-20 286 kobject_del(q->mq_kobj);
f05d1ba7871a2c Bart Van Assche 2017-04-26 287 kobject_put(&dev->kobj);
f05d1ba7871a2c Bart Van Assche 2017-04-26 288 return ret;
2d0364c8c1a97a Bart Van Assche 2017-04-26 289 }
2d0364c8c1a97a Bart Van Assche 2017-04-26 290
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
2022-03-03 9:08 ` kernel test robot
@ 2022-03-03 9:31 ` Ming Lei
0 siblings, 0 replies; 12+ messages in thread
From: Ming Lei @ 2022-03-03 9:31 UTC (permalink / raw)
To: kernel test robot; +Cc: Jens Axboe, kbuild-all, linux-block, Yu Kuai
On Thu, Mar 03, 2022 at 05:08:47PM +0800, kernel test robot wrote:
> Hi Ming,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on axboe-block/for-next]
> [also build test WARNING on v5.17-rc6 next-20220302]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
> base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031651.z0z86F6E-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.2.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
Good catch, will fix it by the following way in V3:
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 6a7fb960e046..851faacc2f78 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -279,8 +279,8 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
return ret;
unreg:
- while (--i >= 0)
- blk_mq_unregister_hctx(xa_load(&q->hctx_table, i));
+ queue_for_each_hw_ctx(q, hctx, i) {
+ blk_mq_unregister_hctx(hctx);
kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
kobject_del(q->mq_kobj);
Thanks,
Ming
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 6/6] blk-mq: manage hctx map via xarray
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
` (4 preceding siblings ...)
2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
5 siblings, 0 replies; 12+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei
Firstly code becomes more clean by switching to xarray from plain array.
Secondly use-after-free on q->queue_hw_ctx can be fixed because
queue_for_each_hw_ctx() may be run when updating nr_hw_queues is
in-progress. With this patch, q->hctx_table is defined as xarray, and
this structure will share same lifetime with request queue, so
queue_for_each_hw_ctx() can use q->hctx_table to lookup hctx reliably.
Reported-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-sysfs.c | 2 +-
block/blk-mq-tag.c | 2 +-
block/blk-mq.c | 55 ++++++++++++++++++------------------------
block/blk-mq.h | 2 +-
include/linux/blk-mq.h | 3 +--
include/linux/blkdev.h | 2 +-
6 files changed, 29 insertions(+), 37 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 8d8c71c77ff8..6a7fb960e046 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -280,7 +280,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
unreg:
while (--i >= 0)
- blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
+ blk_mq_unregister_hctx(xa_load(&q->hctx_table, i));
kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
kobject_del(q->mq_kobj);
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 1850a4225e12..68ac23d0b640 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -498,7 +498,7 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_tag_iter_fn *fn,
void *priv)
{
/*
- * __blk_mq_update_nr_hw_queues() updates nr_hw_queues and queue_hw_ctx
+ * __blk_mq_update_nr_hw_queues() updates nr_hw_queues and hctx_table
* while the queue is frozen. So we can use q_usage_counter to avoid
* racing with it.
*/
diff --git a/block/blk-mq.c b/block/blk-mq.c
index bffdd71c670d..a15d12fb227c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -71,7 +71,8 @@ static int blk_mq_poll_stats_bkt(const struct request *rq)
static inline struct blk_mq_hw_ctx *blk_qc_to_hctx(struct request_queue *q,
blk_qc_t qc)
{
- return q->queue_hw_ctx[(qc & ~BLK_QC_T_INTERNAL) >> BLK_QC_T_SHIFT];
+ return xa_load(&q->hctx_table,
+ (qc & ~BLK_QC_T_INTERNAL) >> BLK_QC_T_SHIFT);
}
static inline struct request *blk_qc_to_rq(struct blk_mq_hw_ctx *hctx,
@@ -573,7 +574,7 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
* If not tell the caller that it should skip this queue.
*/
ret = -EXDEV;
- data.hctx = q->queue_hw_ctx[hctx_idx];
+ data.hctx = xa_load(&q->hctx_table, hctx_idx);
if (!blk_mq_hw_queue_mapped(data.hctx))
goto out_queue_exit;
cpu = cpumask_first_and(data.hctx->cpumask, cpu_online_mask);
@@ -3437,6 +3438,8 @@ static void blk_mq_exit_hctx(struct request_queue *q,
blk_mq_remove_cpuhp(hctx);
+ xa_erase(&q->hctx_table, hctx_idx);
+
spin_lock(&q->unused_hctx_lock);
list_add(&hctx->hctx_list, &q->unused_hctx_list);
spin_unlock(&q->unused_hctx_lock);
@@ -3476,8 +3479,15 @@ static int blk_mq_init_hctx(struct request_queue *q,
if (blk_mq_init_request(set, hctx->fq->flush_rq, hctx_idx,
hctx->numa_node))
goto exit_hctx;
+
+ if (xa_insert(&q->hctx_table, hctx_idx, hctx, GFP_KERNEL))
+ goto exit_flush_rq;
+
return 0;
+ exit_flush_rq:
+ if (set->ops->exit_request)
+ set->ops->exit_request(set, hctx->fq->flush_rq, hctx_idx);
exit_hctx:
if (set->ops->exit_hctx)
set->ops->exit_hctx(hctx, hctx_idx);
@@ -3856,7 +3866,7 @@ void blk_mq_release(struct request_queue *q)
kobject_put(&hctx->kobj);
}
- kfree(q->queue_hw_ctx);
+ xa_destroy(&q->hctx_table);
/*
* release .mq_kobj and sw queue's kobject now because
@@ -3946,45 +3956,28 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
struct request_queue *q)
{
int i, j, end;
- struct blk_mq_hw_ctx **hctxs = q->queue_hw_ctx;
-
- if (q->nr_hw_queues < set->nr_hw_queues) {
- struct blk_mq_hw_ctx **new_hctxs;
-
- new_hctxs = kcalloc_node(set->nr_hw_queues,
- sizeof(*new_hctxs), GFP_KERNEL,
- set->numa_node);
- if (!new_hctxs)
- return;
- if (hctxs)
- memcpy(new_hctxs, hctxs, q->nr_hw_queues *
- sizeof(*hctxs));
- q->queue_hw_ctx = new_hctxs;
- kfree(hctxs);
- hctxs = new_hctxs;
- }
/* protect against switching io scheduler */
mutex_lock(&q->sysfs_lock);
for (i = 0; i < set->nr_hw_queues; i++) {
int old_node;
int node = blk_mq_get_hctx_node(set, i);
- struct blk_mq_hw_ctx *old_hctx = hctxs[i];
+ struct blk_mq_hw_ctx *old_hctx = xa_load(&q->hctx_table, i);
if (old_hctx) {
old_node = old_hctx->numa_node;
blk_mq_exit_hctx(q, set, old_hctx, i);
}
- hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i, node);
- if (!hctxs[i]) {
+ if (!blk_mq_alloc_and_init_hctx(set, q, i, node)) {
+ struct blk_mq_hw_ctx *hctx;
+
if (!old_hctx)
break;
pr_warn("Allocate new hctx on node %d fails, fallback to previous one on node %d\n",
node, old_node);
- hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i,
- old_node);
- WARN_ON_ONCE(!hctxs[i]);
+ hctx = blk_mq_alloc_and_init_hctx(set, q, i, old_node);
+ WARN_ON_ONCE(!hctx);
}
}
/*
@@ -4001,12 +3994,10 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
}
for (; j < end; j++) {
- struct blk_mq_hw_ctx *hctx = hctxs[j];
+ struct blk_mq_hw_ctx *hctx = xa_load(&q->hctx_table, j);
- if (hctx) {
+ if (hctx)
blk_mq_exit_hctx(q, set, hctx, j);
- hctxs[j] = NULL;
- }
}
mutex_unlock(&q->sysfs_lock);
}
@@ -4046,6 +4037,8 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
INIT_LIST_HEAD(&q->unused_hctx_list);
spin_lock_init(&q->unused_hctx_lock);
+ xa_init(&q->hctx_table);
+
blk_mq_realloc_hw_ctxs(set, q);
if (!q->nr_hw_queues)
goto err_hctxs;
@@ -4075,7 +4068,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
return 0;
err_hctxs:
- kfree(q->queue_hw_ctx);
+ xa_destroy(&q->hctx_table);
q->nr_hw_queues = 0;
blk_mq_sysfs_deinit(q);
err_poll:
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 948791ea2a3e..2615bd58bad3 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -83,7 +83,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue_type(struct request_queue *
enum hctx_type type,
unsigned int cpu)
{
- return q->queue_hw_ctx[q->tag_set->map[type].mq_map[cpu]];
+ return xa_load(&q->hctx_table, q->tag_set->map[type].mq_map[cpu]);
}
static inline enum hctx_type blk_mq_get_hctx_type(unsigned int flags)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 3a41d50b85d3..7aa5c54901a9 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -917,8 +917,7 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)
}
#define queue_for_each_hw_ctx(q, hctx, i) \
- for ((i) = 0; (i) < (q)->nr_hw_queues && \
- ({ hctx = (q)->queue_hw_ctx[i]; 1; }); (i)++)
+ xa_for_each(&(q)->hctx_table, (i), (hctx))
#define hctx_for_each_ctx(hctx, ctx, i) \
for ((i) = 0; (i) < (hctx)->nr_ctx && \
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f757f9c2871f..a53ae40aaded 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -355,7 +355,7 @@ struct request_queue {
unsigned int queue_depth;
/* hw dispatch queues */
- struct blk_mq_hw_ctx **queue_hw_ctx;
+ struct xarray hctx_table;
unsigned int nr_hw_queues;
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread