Received-SPF: pass (google.com: domain of linux-kernel+bounces-212468-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Yu Kuai <yukuai1@huaweicloud.com>
To: axboe@kernel.dk,
	tj@kernel.org,
	josef@toxicpanda.com,
	gregkh@linuxfoundation.org,
	lizefan.x@bytedance.com,
	hannes@cmpxchg.org
Cc: linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	cgroups@vger.kernel.org,
	yukuai3@huawei.com,
	yukuai1@huaweicloud.com,
	yi.zhang@huawei.com,
	yangerkun@huawei.com
Subject: [PATCH RFC -next 4/7] blk-iocost: factor out helpers to handle params from ioc_qos_write()
Date: Thu, 13 Jun 2024 09:49:34 +0800
Message-Id: <20240613014937.1326020-5-yukuai1@huaweicloud.com>
In-Reply-To: <20240613014937.1326020-1-yukuai1@huaweicloud.com>
References: <20240613014937.1326020-1-yukuai1@huaweicloud.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Yu Kuai <yukuai3@huawei.com>

Currently the procedures are:

1) parse input disk and open the disk;
2) get ioc, if this is the first writer, init iocost;
3) init qos params, by copying from ioc;
4) parse input qos params;
5) update qos params to ioc;

This patch just factor out step 3-5 into separate helpers, there are no
functional changes, and prepare for fulture optimizations:

- move step 3-4 before setp 2, and don't init iocost for invalid input
  qos params;
- add a new input, and support to free iocost after step 4;
- support to build iocost as kernel module;

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/blk-iocost.c | 155 ++++++++++++++++++++++++++-------------------
 1 file changed, 91 insertions(+), 64 deletions(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 690ca99dfaca..253143005086 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3206,45 +3206,24 @@ static const match_table_t qos_tokens = {
 	{ NR_QOS_PARAMS,	NULL		},
 };
 
-static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
-			     size_t nbytes, loff_t off)
-{
-	struct blkg_conf_ctx ctx;
-	struct gendisk *disk;
-	struct ioc *ioc;
+struct ioc_qos_params {
 	u32 qos[NR_QOS_PARAMS];
-	bool enable, user;
-	char *body, *p;
-	int ret;
-
-	blkg_conf_init(&ctx, input);
-
-	ret = blkg_conf_open_bdev(&ctx);
-	if (ret)
-		goto err;
-
-	body = ctx.body;
-	disk = ctx.bdev->bd_disk;
-	if (!queue_is_mq(disk->queue)) {
-		ret = -EOPNOTSUPP;
-		goto err;
-	}
-
-	ioc = q_to_ioc(disk->queue);
-	if (!ioc) {
-		ret = blk_iocost_init(disk);
-		if (ret)
-			goto err;
-		ioc = q_to_ioc(disk->queue);
-	}
+	bool enable;
+	bool user;
+};
 
-	blk_mq_freeze_queue(disk->queue);
-	blk_mq_quiesce_queue(disk->queue);
+static void ioc_qos_params_init(struct ioc *ioc, struct ioc_qos_params *params)
+{
+	memcpy(params->qos, ioc->params.qos, sizeof(params->qos));
+	params->enable = ioc->enabled;
+	params->user = ioc->user_qos_params;
+}
 
-	spin_lock_irq(&ioc->lock);
-	memcpy(qos, ioc->params.qos, sizeof(qos));
-	enable = ioc->enabled;
-	user = ioc->user_qos_params;
+static int ioc_qos_params_parse(struct blkg_conf_ctx *ctx,
+				struct ioc_qos_params *params)
+{
+	char *body = ctx->body;
+	char *p;
 
 	while ((p = strsep(&body, " \t\n"))) {
 		substring_t args[MAX_OPT_ARGS];
@@ -3258,17 +3237,17 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
 		switch (match_token(p, qos_ctrl_tokens, args)) {
 		case QOS_ENABLE:
 			if (match_u64(&args[0], &v))
-				goto einval;
-			enable = v;
+				return -EINVAL;
+			params->enable = v;
 			continue;
 		case QOS_CTRL:
 			match_strlcpy(buf, &args[0], sizeof(buf));
 			if (!strcmp(buf, "auto"))
-				user = false;
+				params->user = false;
 			else if (!strcmp(buf, "user"))
-				user = true;
+				params->user = true;
 			else
-				goto einval;
+				return -EINVAL;
 			continue;
 		}
 
@@ -3278,61 +3257,110 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
 		case QOS_WPPM:
 			if (match_strlcpy(buf, &args[0], sizeof(buf)) >=
 			    sizeof(buf))
-				goto einval;
+				return -EINVAL;
 			if (cgroup_parse_float(buf, 2, &v))
-				goto einval;
+				return -EINVAL;
 			if (v < 0 || v > 10000)
-				goto einval;
-			qos[tok] = v * 100;
+				return -EINVAL;
+			params->qos[tok] = v * 100;
 			break;
 		case QOS_RLAT:
 		case QOS_WLAT:
 			if (match_u64(&args[0], &v))
-				goto einval;
-			qos[tok] = v;
+				return -EINVAL;
+			params->qos[tok] = v;
 			break;
 		case QOS_MIN:
 		case QOS_MAX:
 			if (match_strlcpy(buf, &args[0], sizeof(buf)) >=
 			    sizeof(buf))
-				goto einval;
+				return -EINVAL;
 			if (cgroup_parse_float(buf, 2, &v))
-				goto einval;
+				return -EINVAL;
 			if (v < 0)
-				goto einval;
-			qos[tok] = clamp_t(s64, v * 100,
-					   VRATE_MIN_PPM, VRATE_MAX_PPM);
+				return -EINVAL;
+			params->qos[tok] = clamp_t(s64, v * 100,
+						   VRATE_MIN_PPM,
+						   VRATE_MAX_PPM);
 			break;
 		default:
-			goto einval;
+			return -EINVAL;
 		}
-		user = true;
+		params->user = true;
 	}
 
-	if (qos[QOS_MIN] > qos[QOS_MAX])
-		goto einval;
+	if (params->qos[QOS_MIN] > params->qos[QOS_MAX])
+		return -EINVAL;
+
+	return 0;
+}
 
-	if (enable && !ioc->enabled) {
+static void ioc_qos_params_update(struct gendisk *disk, struct ioc *ioc,
+				  struct ioc_qos_params *params)
+{
+	if (params->enable && !ioc->enabled) {
 		blk_stat_enable_accounting(disk->queue);
 		blk_queue_flag_set(QUEUE_FLAG_RQ_ALLOC_TIME, disk->queue);
 		ioc->enabled = true;
-	} else if (!enable && ioc->enabled) {
+	} else if (!params->enable && ioc->enabled) {
 		blk_stat_disable_accounting(disk->queue);
 		blk_queue_flag_clear(QUEUE_FLAG_RQ_ALLOC_TIME, disk->queue);
 		ioc->enabled = false;
 	}
 
-	if (user) {
-		memcpy(ioc->params.qos, qos, sizeof(qos));
+	if (params->user) {
+		memcpy(ioc->params.qos, params->qos, sizeof(params->qos));
 		ioc->user_qos_params = true;
 	} else {
 		ioc->user_qos_params = false;
 	}
 
 	ioc_refresh_params(ioc, true);
+}
+
+static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
+			     size_t nbytes, loff_t off)
+{
+	struct ioc_qos_params params;
+	struct blkg_conf_ctx ctx;
+	struct gendisk *disk;
+	struct ioc *ioc;
+	int ret;
+
+	blkg_conf_init(&ctx, input);
+
+	ret = blkg_conf_open_bdev(&ctx);
+	if (ret)
+		goto err;
+
+	disk = ctx.bdev->bd_disk;
+	if (!queue_is_mq(disk->queue)) {
+		ret = -EOPNOTSUPP;
+		goto err;
+	}
+
+	ioc = q_to_ioc(disk->queue);
+	if (!ioc) {
+		ret = blk_iocost_init(disk);
+		if (ret)
+			goto err;
+		ioc = q_to_ioc(disk->queue);
+	}
+
+	blk_mq_freeze_queue(disk->queue);
+	blk_mq_quiesce_queue(disk->queue);
+
+	spin_lock_irq(&ioc->lock);
+	ioc_qos_params_init(ioc, &params);
+
+	ret = ioc_qos_params_parse(&ctx, &params);
+	if (ret)
+		goto err_parse;
+
+	ioc_qos_params_update(disk, ioc, &params);
 	spin_unlock_irq(&ioc->lock);
 
-	if (enable)
+	if (params.enable)
 		wbt_disable_default(disk);
 	else
 		wbt_enable_default(disk);
@@ -3342,13 +3370,12 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
 
 	blkg_conf_exit(&ctx);
 	return nbytes;
-einval:
+
+err_parse:
 	spin_unlock_irq(&ioc->lock);
 
 	blk_mq_unquiesce_queue(disk->queue);
 	blk_mq_unfreeze_queue(disk->queue);
-
-	ret = -EINVAL;
 err:
 	blkg_conf_exit(&ctx);
 	return ret;
-- 
2.39.2