[RFC,V4,2/6] blk-wbt: make wbt pluggable

Commit Message

Wang Jianchao Feb. 17, 2022, 3:13 a.m. UTC

This patch makes wbt pluggable through /sys/block/xxx/queue/qos.
Some queue_lock/unlock is added to protect rq_qos_by_id() in
wbt_rq_qos(). By default, wbt is enabled which is same with previous
code.

Signed-off-by: Wang Jianchao (Kuaishou) <[e-mail]>
---
 block/blk-mq-debugfs.c |  2 --
 block/blk-rq-qos.h     |  6 -----
 block/blk-wbt.c        | 51 +++++++++++++++++++++++++++++++++++-------
 block/blk-wbt.h        |  6 +++++
 4 files changed, 49 insertions(+), 16 deletions(-)

Comments

Christoph Hellwig Feb. 17, 2022, 8:50 a.m. UTC | #1

> +struct rq_qos *wbt_rq_qos(struct request_queue *q);
>  int wbt_init(struct request_queue *);


Please move the wb_lat sysfs attribute into blk-wbt.c as well, which
removes the need to expose these two functions.

Wang Jianchao Feb. 18, 2022, 3:41 a.m. UTC | #2

On 2022/2/17 4:50 下午, Christoph Hellwig wrote:
>> +struct rq_qos *wbt_rq_qos(struct request_queue *q);
>>  int wbt_init(struct request_queue *);
> 
> 
> Please move the wb_lat sysfs attribute into blk-wbt.c as well, which
> removes the need to expose these two functions.
> 

Given this patchset:
(1) Do we need to reserve the wb_lat sysfs when we turn off the wbt ?
(2) Do we need to disable wbt automatically when switch io scheduler
    to bfq ? Or just tell the user turn off the wbt by themselves ?

Thanks
Jianchao

Christoph Hellwig Feb. 22, 2022, 8:12 a.m. UTC | #3

On Fri, Feb 18, 2022 at 11:41:11AM +0800, Wang Jianchao wrote:
> 
> 
> On 2022/2/17 4:50 下午, Christoph Hellwig wrote:
> >> +struct rq_qos *wbt_rq_qos(struct request_queue *q);
> >>  int wbt_init(struct request_queue *);
> > 
> > 
> > Please move the wb_lat sysfs attribute into blk-wbt.c as well, which
> > removes the need to expose these two functions.
> > 
> 
> Given this patchset:
> (1) Do we need to reserve the wb_lat sysfs when we turn off the wbt ?
> (2) Do we need to disable wbt automatically when switch io scheduler
>     to bfq ? Or just tell the user turn off the wbt by themselves ?

I don't think this needs any changes to what is done there today,
i.e. keep the sysfs attribute around.

Patch

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 8b6d557e1ad6..a5d78b094234 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -826,8 +826,6 @@  void blk_mq_debugfs_unregister_sched(struct request_queue *q)
 static const char *rq_qos_id_to_name(enum rq_qos_id id)
 {
 	switch (id) {
-	case RQ_QOS_WBT:
-		return "wbt";
 	case RQ_QOS_LATENCY:
 		return "latency";
 	case RQ_QOS_COST:
diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
index 586c3f5ec152..171a83d6de45 100644
--- a/block/blk-rq-qos.h
+++ b/block/blk-rq-qos.h
@@ -14,7 +14,6 @@ 
 struct blk_mq_debugfs_attr;
 
 enum rq_qos_id {
-	RQ_QOS_WBT,
 	RQ_QOS_LATENCY,
 	RQ_QOS_COST,
 	RQ_QOS_IOPRIO,
@@ -96,11 +95,6 @@  static inline struct rq_qos *rq_qos_by_id(struct request_queue *q, int id)
 	return rqos;
 }
 
-static inline struct rq_qos *wbt_rq_qos(struct request_queue *q)
-{
-	return rq_qos_id(q, RQ_QOS_WBT);
-}
-
 static inline struct rq_qos *blkcg_rq_qos(struct request_queue *q)
 {
 	return rq_qos_id(q, RQ_QOS_LATENCY);
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 0c119be0e813..8aa85303bbbc 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -31,6 +31,13 @@ 
 #define CREATE_TRACE_POINTS
 #include <trace/events/wbt.h>
 
+static struct rq_qos_ops wbt_rqos_ops;
+
+struct rq_qos *wbt_rq_qos(struct request_queue *q)
+{
+	return rq_qos_by_id(q, wbt_rqos_ops.id);
+}
+
 static inline void wbt_clear_state(struct request *rq)
 {
 	rq->wbt_flags = 0;
@@ -628,9 +635,13 @@  static void wbt_requeue(struct rq_qos *rqos, struct request *rq)
 
 void wbt_set_write_cache(struct request_queue *q, bool write_cache_on)
 {
-	struct rq_qos *rqos = wbt_rq_qos(q);
+	struct rq_qos *rqos;
+
+	spin_lock_irq(&q->queue_lock);
+	rqos = wbt_rq_qos(q);
 	if (rqos)
 		RQWB(rqos)->wc = write_cache_on;
+	spin_unlock_irq(&q->queue_lock);
 }
 
 /*
@@ -638,14 +649,20 @@  void wbt_set_write_cache(struct request_queue *q, bool write_cache_on)
  */
 void wbt_enable_default(struct request_queue *q)
 {
-	struct rq_qos *rqos = wbt_rq_qos(q);
+	struct rq_qos *rqos;
+
+	spin_lock_irq(&q->queue_lock);
+	rqos = wbt_rq_qos(q);
 
 	/* Throttling already enabled? */
 	if (rqos) {
 		if (RQWB(rqos)->enable_state == WBT_STATE_OFF_DEFAULT)
 			RQWB(rqos)->enable_state = WBT_STATE_ON_DEFAULT;
+
+		spin_unlock_irq(&q->queue_lock);
 		return;
 	}
+	spin_unlock_irq(&q->queue_lock);
 
 	/* Queue not registered? Maybe shutting down... */
 	if (!blk_queue_registered(q))
@@ -692,6 +709,7 @@  static void wbt_exit(struct rq_qos *rqos)
 	struct rq_wb *rwb = RQWB(rqos);
 	struct request_queue *q = rqos->q;
 
+	rq_qos_deactivate(rqos);
 	blk_stat_remove_callback(q, rwb->cb);
 	blk_stat_free_callback(rwb->cb);
 	kfree(rwb);
@@ -702,15 +720,21 @@  static void wbt_exit(struct rq_qos *rqos)
  */
 void wbt_disable_default(struct request_queue *q)
 {
-	struct rq_qos *rqos = wbt_rq_qos(q);
+	struct rq_qos *rqos;
 	struct rq_wb *rwb;
+
+	spin_lock_irq(&q->queue_lock);
+	rqos = wbt_rq_qos(q);
 	if (!rqos)
-		return;
+		goto out;
+
 	rwb = RQWB(rqos);
 	if (rwb->enable_state == WBT_STATE_ON_DEFAULT) {
 		blk_stat_deactivate(rwb->cb);
 		rwb->enable_state = WBT_STATE_OFF_DEFAULT;
 	}
+out:
+	spin_unlock_irq(&q->queue_lock);
 }
 EXPORT_SYMBOL_GPL(wbt_disable_default);
 
@@ -803,6 +827,7 @@  static const struct blk_mq_debugfs_attr wbt_debugfs_attrs[] = {
 #endif
 
 static struct rq_qos_ops wbt_rqos_ops = {
+	.name = "blk-wbt",
 	.throttle = wbt_wait,
 	.issue = wbt_issue,
 	.track = wbt_track,
@@ -811,6 +836,7 @@  static struct rq_qos_ops wbt_rqos_ops = {
 	.cleanup = wbt_cleanup,
 	.queue_depth_changed = wbt_queue_depth_changed,
 	.exit = wbt_exit,
+	.init = wbt_init,
 #ifdef CONFIG_BLK_DEBUG_FS
 	.debugfs_attrs = wbt_debugfs_attrs,
 #endif
@@ -834,9 +860,6 @@  int wbt_init(struct request_queue *q)
 	for (i = 0; i < WBT_NUM_RWQ; i++)
 		rq_wait_init(&rwb->rq_wait[i]);
 
-	rwb->rqos.id = RQ_QOS_WBT;
-	rwb->rqos.ops = &wbt_rqos_ops;
-	rwb->rqos.q = q;
 	rwb->last_comp = rwb->last_issue = jiffies;
 	rwb->win_nsec = RWB_WINDOW_NSEC;
 	rwb->enable_state = WBT_STATE_ON_DEFAULT;
@@ -846,7 +869,7 @@  int wbt_init(struct request_queue *q)
 	/*
 	 * Assign rwb and add the stats callback.
 	 */
-	rq_qos_add(q, &rwb->rqos);
+	rq_qos_activate(q, &rwb->rqos, &wbt_rqos_ops);
 	blk_stat_add_callback(q, rwb->cb);
 
 	rwb->min_lat_nsec = wbt_default_latency_nsec(q);
@@ -856,3 +879,15 @@  int wbt_init(struct request_queue *q)
 
 	return 0;
 }
+
+static __init int wbt_mod_init(void)
+{
+	return rq_qos_register(&wbt_rqos_ops);
+}
+
+static __exit void wbt_mod_exit(void)
+{
+	return rq_qos_unregister(&wbt_rqos_ops);
+}
+module_init(wbt_mod_init);
+module_exit(wbt_mod_exit);
diff --git a/block/blk-wbt.h b/block/blk-wbt.h
index 2eb01becde8c..e0d051cb00f7 100644
--- a/block/blk-wbt.h
+++ b/block/blk-wbt.h
@@ -88,6 +88,7 @@  static inline unsigned int wbt_inflight(struct rq_wb *rwb)
 
 #ifdef CONFIG_BLK_WBT
 
+struct rq_qos *wbt_rq_qos(struct request_queue *q);
 int wbt_init(struct request_queue *);
 void wbt_disable_default(struct request_queue *);
 void wbt_enable_default(struct request_queue *);
@@ -101,6 +102,11 @@  u64 wbt_default_latency_nsec(struct request_queue *);
 
 #else
 
+static inline struct rq_qos *wbt_rq_qos(struct request_queue *q)
+{
+	return NULL;
+}
+
 static inline void wbt_track(struct request *rq, enum wbt_flags flags)
 {
 }