From: Ming Lei <ming.lei@redhat.com>
To: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>
Cc: linux-block@vger.kernel.org, Ming Lei <ming.lei@redhat.com>,
Yi Zhang <yi.zhang@redhat.com>
Subject: [PATCH 2/4] block: move wbt allocation into blk_alloc_queue
Date: Tue, 25 May 2021 16:04:40 +0800 [thread overview]
Message-ID: <20210525080442.1896417-3-ming.lei@redhat.com> (raw)
In-Reply-To: <20210525080442.1896417-1-ming.lei@redhat.com>
wbt_init() calls wbt_alloc() which adds allocated wbt instance into
q->rq_qos. This way is very dangerous because q->rq_qos is accessed in
IO fast path.
So far wbt_init() is called in the two code paths:
1) blk_register_queue(), when the block device has been exposed to
usespace, so IO may come when adding wbt into q->rq_qos
2) sysfs attribute store, in which normal IO is definitely allowed
Move wbt allocation into blk_alloc_queue() for avoiding to add wbt
instance dynamically to q->rq_qos. And before calling wbt_init(), the
wbt is disabled, so functionally it works as expected.
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-core.c | 6 ++++++
block/blk-mq-debugfs.c | 3 +++
block/blk-sysfs.c | 8 --------
block/blk-wbt.c | 21 ++++++---------------
block/blk-wbt.h | 10 +++++++---
5 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 689aac2625d2..e7d9eac5d4c1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -51,6 +51,7 @@
#include "blk-mq-sched.h"
#include "blk-pm.h"
#include "blk-rq-qos.h"
+#include "blk-wbt.h"
struct dentry *blk_debugfs_root;
@@ -579,12 +580,17 @@ struct request_queue *blk_alloc_queue(int node_id)
if (blkcg_init_queue(q))
goto fail_ref;
+ if (wbt_alloc(q))
+ goto fail_blkcg;
+
blk_queue_dma_alignment(q, 511);
blk_set_default_limits(&q->limits);
q->nr_requests = BLKDEV_MAX_RQ;
return q;
+fail_blkcg:
+ blkcg_exit_queue(q);
fail_ref:
percpu_ref_exit(&q->q_usage_counter);
fail_bdi:
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 2a75bc7401df..c1790f313479 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -948,6 +948,9 @@ void blk_mq_debugfs_register_rqos(struct rq_qos *rqos)
struct request_queue *q = rqos->q;
const char *dir_name = rq_qos_id_to_name(rqos->id);
+ if (!q->debugfs_dir)
+ return;
+
if (rqos->debugfs_dir || !rqos->ops->debugfs_attrs)
return;
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f89e2fc3963b..7fd4ffadcdfa 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -481,7 +481,6 @@ static ssize_t queue_wb_lat_show(struct request_queue *q, char *page)
static ssize_t queue_wb_lat_store(struct request_queue *q, const char *page,
size_t count)
{
- struct rq_qos *rqos;
ssize_t ret;
s64 val;
@@ -491,13 +490,6 @@ static ssize_t queue_wb_lat_store(struct request_queue *q, const char *page,
if (val < -1)
return -EINVAL;
- rqos = wbt_rq_qos(q);
- if (!rqos) {
- ret = wbt_init(q);
- if (ret)
- return ret;
- }
-
if (val == -1)
val = wbt_default_latency_nsec(q);
else if (val >= 0)
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index efff1232446f..5da48294e3e9 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -421,16 +421,14 @@ static void wbt_update_limits(struct rq_wb *rwb)
u64 wbt_get_min_lat(struct request_queue *q)
{
struct rq_qos *rqos = wbt_rq_qos(q);
- if (!rqos)
- return 0;
+
return RQWB(rqos)->min_lat_nsec;
}
void wbt_set_min_lat(struct request_queue *q, u64 val)
{
struct rq_qos *rqos = wbt_rq_qos(q);
- if (!rqos)
- return;
+
RQWB(rqos)->min_lat_nsec = val;
RQWB(rqos)->enable_state = WBT_STATE_ON_MANUAL;
wbt_update_limits(RQWB(rqos));
@@ -637,7 +635,7 @@ void wbt_enable_default(struct request_queue *q)
{
struct rq_qos *rqos = wbt_rq_qos(q);
/* Throttling already enabled? */
- if (rqos)
+ if (rwb_enabled(RQWB(rqos)))
return;
/* Queue not registered? Maybe shutting down... */
@@ -809,7 +807,7 @@ static struct rq_qos_ops wbt_rqos_ops = {
#endif
};
-static int wbt_alloc(struct request_queue *q)
+int wbt_alloc(struct request_queue *q)
{
struct rq_wb *rwb;
int i;
@@ -844,20 +842,13 @@ static int wbt_alloc(struct request_queue *q)
return 0;
}
-int wbt_init(struct request_queue *q)
+void wbt_init(struct request_queue *q)
{
- int ret = wbt_alloc(q);
- struct rq_wb *rwb;
-
- if (ret)
- return ret;
+ struct rq_wb *rwb = RQWB(wbt_rq_qos(q));
- rwb = RQWB(wbt_rq_qos(q));
rwb->enable_state = WBT_STATE_ON_DEFAULT;
rwb->min_lat_nsec = wbt_default_latency_nsec(q);
wbt_queue_depth_changed(&rwb->rqos);
wbt_set_write_cache(q, test_bit(QUEUE_FLAG_WC, &q->queue_flags));
-
- return 0;
}
diff --git a/block/blk-wbt.h b/block/blk-wbt.h
index 16bdc85b8df9..de07963f3544 100644
--- a/block/blk-wbt.h
+++ b/block/blk-wbt.h
@@ -87,7 +87,8 @@ static inline unsigned int wbt_inflight(struct rq_wb *rwb)
#ifdef CONFIG_BLK_WBT
-int wbt_init(struct request_queue *);
+int wbt_alloc(struct request_queue *q);
+void wbt_init(struct request_queue *);
void wbt_disable_default(struct request_queue *);
void wbt_enable_default(struct request_queue *);
@@ -103,9 +104,12 @@ u64 wbt_default_latency_nsec(struct request_queue *);
static inline void wbt_track(struct request *rq, enum wbt_flags flags)
{
}
-static inline int wbt_init(struct request_queue *q)
+static inline int wbt_alloc(struct request_queue *q)
+{
+ return 0;
+}
+static inline void wbt_init(struct request_queue *q)
{
- return -EINVAL;
}
static inline void wbt_disable_default(struct request_queue *q)
{
--
2.29.2
next prev parent reply other threads:[~2021-05-25 8:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-25 8:04 [PATCH 0/4] block: fix race between adding wbt and normal IO Ming Lei
2021-05-25 8:04 ` [PATCH 1/4] block: split wbt_init() into two parts Ming Lei
2021-06-04 0:32 ` Bart Van Assche
2021-05-25 8:04 ` Ming Lei [this message]
2021-06-04 0:44 ` [PATCH 2/4] block: move wbt allocation into blk_alloc_queue Bart Van Assche
2021-06-04 1:22 ` Ming Lei
2021-05-25 8:04 ` [PATCH 3/4] block: reuse wbt_set_min_lat for setting wbt->min_lat_nsec Ming Lei
2021-05-25 8:04 ` [PATCH 4/4] block: mark queue init done at the end of blk_register_queue Ming Lei
2021-06-04 0:03 ` [PATCH 0/4] block: fix race between adding wbt and normal IO Ming Lei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210525080442.1896417-3-ming.lei@redhat.com \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=yi.zhang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).