From: Yu Kuai <[email protected]>
Yu Kuai (7):
kernfs: export pr_cont_kernfs_path()
cgroup: export cgroup_parse_float
block: export some API
blk-iocost: factor out helpers to handle params from ioc_qos_write()
blk-iocost: parse params before initializing iocost
blk-iocost: support to free iocost
blk-iocost: support to build iocost as kernel module
block/Kconfig | 2 +-
block/blk-cgroup.c | 4 +
block/blk-iocost.c | 223 ++++++++++++++++++++++++++------------
block/blk-rq-qos.c | 2 +
fs/kernfs/dir.c | 1 +
include/linux/blk_types.h | 2 +-
kernel/cgroup/cgroup.c | 1 +
7 files changed, 163 insertions(+), 72 deletions(-)
--
2.39.2
From: Yu Kuai <[email protected]>
This helper is used in iocost, prepare to build iocost as kernel module.
Signed-off-by: Yu Kuai <[email protected]>
---
fs/kernfs/dir.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 458519e416fe..84ad163a4281 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -279,6 +279,7 @@ void pr_cont_kernfs_path(struct kernfs_node *kn)
out:
spin_unlock_irqrestore(&kernfs_pr_cont_lock, flags);
}
+EXPORT_SYMBOL_GPL(pr_cont_kernfs_path);
/**
* kernfs_get_parent - determine the parent node and pin it
--
2.39.2
From: Yu Kuai <[email protected]>
The symbol is used by iocost, prepare to build iocost as kernel module.
Signed-off-by: Yu Kuai <[email protected]>
---
kernel/cgroup/cgroup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4abd817b0c7c..81b579495f8c 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6966,6 +6966,7 @@ int cgroup_parse_float(const char *input, unsigned dec_shift, s64 *v)
*v = whole * power_of_ten(dec_shift) + frac;
return 0;
}
+EXPORT_SYMBOL_GPL(cgroup_parse_float);
/*
* sock->sk_cgrp_data handling. For more info, see sock_cgroup_data
--
2.39.2
From: Yu Kuai <[email protected]>
On the one hand prevent to initialize iocost for invalid input, on the
ohter hand prepare to add a new input to free iocost.
Signed-off-by: Yu Kuai <[email protected]>
---
block/blk-iocost.c | 44 ++++++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 253143005086..96a6f2292623 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3214,9 +3214,20 @@ struct ioc_qos_params {
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;
+ if (ioc) {
+ memcpy(params->qos, ioc->params.qos, sizeof(params->qos));
+ params->enable = ioc->enabled;
+ params->user = ioc->user_qos_params;
+ } else {
+ params->qos[QOS_RPPM] = 0;
+ params->qos[QOS_RLAT] = 0;
+ params->qos[QOS_WPPM] = 0;
+ params->qos[QOS_WLAT] = 0;
+ params->qos[QOS_MIN] = VRATE_MIN_PPM;
+ params->qos[QOS_MAX] = VRATE_MAX_PPM;
+ params->enable = false;
+ params->user = false;
+ }
}
static int ioc_qos_params_parse(struct blkg_conf_ctx *ctx,
@@ -3309,7 +3320,16 @@ static void ioc_qos_params_update(struct gendisk *disk, struct ioc *ioc,
}
if (params->user) {
- memcpy(ioc->params.qos, params->qos, sizeof(params->qos));
+ if (params->qos[QOS_RPPM])
+ ioc->params.qos[QOS_RPPM] = params->qos[QOS_RPPM];
+ if (params->qos[QOS_RLAT])
+ ioc->params.qos[QOS_RLAT] = params->qos[QOS_RLAT];
+ if (params->qos[QOS_WPPM])
+ ioc->params.qos[QOS_RPPM] = params->qos[QOS_WPPM];
+ if (params->qos[QOS_WLAT])
+ ioc->params.qos[QOS_WLAT] = params->qos[QOS_WLAT];
+ ioc->params.qos[QOS_MIN] = params->qos[QOS_MIN];
+ ioc->params.qos[QOS_MAX] = params->qos[QOS_MAX];
ioc->user_qos_params = true;
} else {
ioc->user_qos_params = false;
@@ -3340,6 +3360,12 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
}
ioc = q_to_ioc(disk->queue);
+ ioc_qos_params_init(ioc, ¶ms);
+
+ ret = ioc_qos_params_parse(&ctx, ¶ms);
+ if (ret)
+ goto err;
+
if (!ioc) {
ret = blk_iocost_init(disk);
if (ret)
@@ -3351,11 +3377,6 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
blk_mq_quiesce_queue(disk->queue);
spin_lock_irq(&ioc->lock);
- ioc_qos_params_init(ioc, ¶ms);
-
- ret = ioc_qos_params_parse(&ctx, ¶ms);
- if (ret)
- goto err_parse;
ioc_qos_params_update(disk, ioc, ¶ms);
spin_unlock_irq(&ioc->lock);
@@ -3371,11 +3392,6 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
blkg_conf_exit(&ctx);
return nbytes;
-err_parse:
- spin_unlock_irq(&ioc->lock);
-
- blk_mq_unquiesce_queue(disk->queue);
- blk_mq_unfreeze_queue(disk->queue);
err:
blkg_conf_exit(&ctx);
return ret;
--
2.39.2
From: Yu Kuai <[email protected]>
These APIs are used in iocost, prepare to build iocost as kernel module.
Signed-off-by: Yu Kuai <[email protected]>
---
block/blk-cgroup.c | 4 ++++
block/blk-rq-qos.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 37e6cc91d576..6171e546ea9f 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -57,6 +57,7 @@ static struct blkcg_policy *blkcg_policy[BLKCG_MAX_POLS];
static LIST_HEAD(all_blkcgs); /* protected by blkcg_pol_mutex */
bool blkcg_debug_stats = false;
+EXPORT_SYMBOL_GPL(blkcg_debug_stats);
static DEFINE_RAW_SPINLOCK(blkg_stat_lock);
@@ -688,6 +689,7 @@ const char *blkg_dev_name(struct blkcg_gq *blkg)
return NULL;
return bdi_dev_name(blkg->q->disk->bdi);
}
+EXPORT_SYMBOL_GPL(blkg_dev_name);
/**
* blkcg_print_blkgs - helper for printing per-blkg data
@@ -815,6 +817,7 @@ int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx)
ctx->bdev = bdev;
return 0;
}
+EXPORT_SYMBOL_GPL(blkg_conf_open_bdev);
/**
* blkg_conf_prep - parse and prepare for per-blkg config update
@@ -2011,6 +2014,7 @@ void blkcg_schedule_throttle(struct gendisk *disk, bool use_memdelay)
current->use_memdelay = use_memdelay;
set_notify_resume(current);
}
+EXPORT_SYMBOL_GPL(blkcg_schedule_throttle);
/**
* blkcg_add_delay - add delay to this blkg
diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index dd7310c94713..c3fdf91ddf8d 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -332,6 +332,7 @@ int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
blk_mq_unfreeze_queue(q);
return -EBUSY;
}
+EXPORT_SYMBOL_GPL(rq_qos_add);
void rq_qos_del(struct rq_qos *rqos)
{
@@ -353,3 +354,4 @@ void rq_qos_del(struct rq_qos *rqos)
blk_mq_debugfs_unregister_rqos(rqos);
mutex_unlock(&q->debugfs_mutex);
}
+EXPORT_SYMBOL_GPL(rq_qos_del);
--
2.39.2
From: Yu Kuai <[email protected]>
Signed-off-by: Yu Kuai <[email protected]>
---
block/Kconfig | 2 +-
block/blk-iocost.c | 14 +++++++++++++-
include/linux/blk_types.h | 2 +-
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/block/Kconfig b/block/Kconfig
index dc12af58dbae..b94b93158e57 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -156,7 +156,7 @@ config BLK_CGROUP_FC_APPID
application specific identification into the FC frame.
config BLK_CGROUP_IOCOST
- bool "Enable support for cost model based cgroup IO controller"
+ tristate "Enable support for cost model based cgroup IO controller"
depends on BLK_CGROUP
select BLK_RQ_ALLOC_TIME
help
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 708a43a7c6a0..2a69db547045 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2850,6 +2850,7 @@ static void ioc_rqos_queue_depth_changed(struct rq_qos *rqos)
static void __ioc_exit(struct ioc *ioc)
{
+ module_put(THIS_MODULE);
blkcg_deactivate_policy(ioc->rqos.disk, &blkcg_policy_iocost);
spin_lock_irq(&ioc->lock);
@@ -2882,13 +2883,19 @@ static int blk_iocost_init(struct gendisk *disk)
struct ioc *ioc;
int i, cpu, ret;
+ if (!try_module_get(THIS_MODULE))
+ return -ENODEV;
+
ioc = kzalloc(sizeof(*ioc), GFP_KERNEL);
- if (!ioc)
+ if (!ioc) {
+ module_put(THIS_MODULE);
return -ENOMEM;
+ }
ioc->pcpu_stat = alloc_percpu(struct ioc_pcpu_stat);
if (!ioc->pcpu_stat) {
kfree(ioc);
+ module_put(THIS_MODULE);
return -ENOMEM;
}
@@ -2938,6 +2945,7 @@ static int blk_iocost_init(struct gendisk *disk)
rq_qos_del(&ioc->rqos);
err_free_ioc:
free_percpu(ioc->pcpu_stat);
+ module_put(THIS_MODULE);
kfree(ioc);
return ret;
}
@@ -3616,3 +3624,7 @@ static void __exit ioc_exit(void)
module_init(ioc_init);
module_exit(ioc_exit);
+
+MODULE_AUTHOR("Tejun Heo");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cost model based cgroup IO controller");
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 781c4500491b..8da12ebc7777 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -234,7 +234,7 @@ struct bio {
*/
struct blkcg_gq *bi_blkg;
struct bio_issue bi_issue;
-#ifdef CONFIG_BLK_CGROUP_IOCOST
+#if IS_ENABLED(CONFIG_BLK_CGROUP_IOCOST)
u64 bi_iocost_cost;
#endif
#endif
--
2.39.2
From: Yu Kuai <[email protected]>
Currently if iocost is initialized, it can never be freed until the disk
is deleted.
A new param "free" is added to the blk-io cgroup api "io.cost.qos", and
user can use it to free iocost. On the one hand prevent overhead from
fast path, on the other hand prepare to build iocost as kernel module.
Signed-off-by: Yu Kuai <[email protected]>
---
block/blk-iocost.c | 38 +++++++++++++++++++++++++++++++++-----
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 96a6f2292623..708a43a7c6a0 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -324,6 +324,7 @@ enum ioc_running {
enum {
QOS_ENABLE,
QOS_CTRL,
+ QOS_FREE,
NR_QOS_CTRL_PARAMS,
};
@@ -2847,11 +2848,9 @@ static void ioc_rqos_queue_depth_changed(struct rq_qos *rqos)
spin_unlock_irq(&ioc->lock);
}
-static void ioc_rqos_exit(struct rq_qos *rqos)
+static void __ioc_exit(struct ioc *ioc)
{
- struct ioc *ioc = rqos_to_ioc(rqos);
-
- blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iocost);
+ blkcg_deactivate_policy(ioc->rqos.disk, &blkcg_policy_iocost);
spin_lock_irq(&ioc->lock);
ioc->running = IOC_STOP;
@@ -2862,6 +2861,13 @@ static void ioc_rqos_exit(struct rq_qos *rqos)
kfree(ioc);
}
+static void ioc_rqos_exit(struct rq_qos *rqos)
+{
+ struct ioc *ioc = rqos_to_ioc(rqos);
+
+ __ioc_exit(ioc);
+}
+
static const struct rq_qos_ops ioc_rqos_ops = {
.throttle = ioc_rqos_throttle,
.merge = ioc_rqos_merge,
@@ -3193,6 +3199,7 @@ static int ioc_qos_show(struct seq_file *sf, void *v)
static const match_table_t qos_ctrl_tokens = {
{ QOS_ENABLE, "enable=%u" },
{ QOS_CTRL, "ctrl=%s" },
+ { QOS_FREE, "free" },
{ NR_QOS_CTRL_PARAMS, NULL },
};
@@ -3210,6 +3217,7 @@ struct ioc_qos_params {
u32 qos[NR_QOS_PARAMS];
bool enable;
bool user;
+ bool free;
};
static void ioc_qos_params_init(struct ioc *ioc, struct ioc_qos_params *params)
@@ -3228,6 +3236,8 @@ static void ioc_qos_params_init(struct ioc *ioc, struct ioc_qos_params *params)
params->enable = false;
params->user = false;
}
+
+ params->free = false;
}
static int ioc_qos_params_parse(struct blkg_conf_ctx *ctx,
@@ -3260,6 +3270,9 @@ static int ioc_qos_params_parse(struct blkg_conf_ctx *ctx,
else
return -EINVAL;
continue;
+ case QOS_FREE:
+ params->free = true;
+ continue;
}
tok = match_token(p, qos_tokens, args);
@@ -3338,6 +3351,15 @@ static void ioc_qos_params_update(struct gendisk *disk, struct ioc *ioc,
ioc_refresh_params(ioc, true);
}
+static void ioc_free(struct ioc *ioc)
+{
+ if (!ioc)
+ return;
+
+ rq_qos_del(&ioc->rqos);
+ __ioc_exit(ioc);
+}
+
static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
size_t nbytes, loff_t off)
{
@@ -3366,7 +3388,7 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
if (ret)
goto err;
- if (!ioc) {
+ if (!params.free && !ioc) {
ret = blk_iocost_init(disk);
if (ret)
goto err;
@@ -3376,6 +3398,11 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
blk_mq_freeze_queue(disk->queue);
blk_mq_quiesce_queue(disk->queue);
+ if (params.free) {
+ ioc_free(ioc);
+ goto out;
+ }
+
spin_lock_irq(&ioc->lock);
ioc_qos_params_update(disk, ioc, ¶ms);
@@ -3386,6 +3413,7 @@ static ssize_t ioc_qos_write(struct kernfs_open_file *of, char *input,
else
wbt_enable_default(disk);
+out:
blk_mq_unquiesce_queue(disk->queue);
blk_mq_unfreeze_queue(disk->queue);
--
2.39.2
From: Yu Kuai <[email protected]>
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 <[email protected]>
---
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, ¶ms);
+
+ ret = ioc_qos_params_parse(&ctx, ¶ms);
+ if (ret)
+ goto err_parse;
+
+ ioc_qos_params_update(disk, ioc, ¶ms);
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
On Thu, Jun 13, 2024 at 09:49:30AM +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> Yu Kuai (7):
> kernfs: export pr_cont_kernfs_path()
> cgroup: export cgroup_parse_float
> block: export some API
> blk-iocost: factor out helpers to handle params from ioc_qos_write()
> blk-iocost: parse params before initializing iocost
> blk-iocost: support to free iocost
> blk-iocost: support to build iocost as kernel module
No where do you say _why_ building this as a module is a good idea.
Why do this at all?
thanks,
greg k-h
On Thu, Jun 13, 2024 at 09:49:37AM +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> Signed-off-by: Yu Kuai <[email protected]>
> ---
> block/Kconfig | 2 +-
> block/blk-iocost.c | 14 +++++++++++++-
> include/linux/blk_types.h | 2 +-
> 3 files changed, 15 insertions(+), 3 deletions(-)
For obvious reasonse we can't take patches without any changelog text.
Nor can we review them :(
greg k-h
On Thu, Jun 13, 2024 at 09:49:31AM +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> This helper is used in iocost, prepare to build iocost as kernel module.
>
> Signed-off-by: Yu Kuai <[email protected]>
> ---
> fs/kernfs/dir.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 458519e416fe..84ad163a4281 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -279,6 +279,7 @@ void pr_cont_kernfs_path(struct kernfs_node *kn)
> out:
> spin_unlock_irqrestore(&kernfs_pr_cont_lock, flags);
> }
> +EXPORT_SYMBOL_GPL(pr_cont_kernfs_path);
This isn't the helper that needs to be exported, it's a include wrapper
that is using this for cgroups. Please document this much better and
perhaps, just fix up the cgroups code instead?
thanks,
greg k-h
Hi,
?? 2024/06/13 13:54, Greg KH д??:
> On Thu, Jun 13, 2024 at 09:49:30AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <[email protected]>
>>
>> Yu Kuai (7):
>> kernfs: export pr_cont_kernfs_path()
>> cgroup: export cgroup_parse_float
>> block: export some API
>> blk-iocost: factor out helpers to handle params from ioc_qos_write()
>> blk-iocost: parse params before initializing iocost
>> blk-iocost: support to free iocost
>> blk-iocost: support to build iocost as kernel module
>
> No where do you say _why_ building this as a module is a good idea.
Yes, we discussed this before and this is actually an general question.
Main advantages are:
- Flexibility and Maintainability, allows for dynamic loading and
unloading of modules at runtime without the need to recompile and
restart the kernel, for example fixing iocost CVE in our production
environment.
- Kernel Size and Resource Usage, modules are loaded only when their
specific functionality is required.
Thanks,
Kuai
>
> Why do this at all?
>
> thanks,
>
> greg k-h
>
> .
>
Hi,
?? 2024/06/13 13:55, Greg KH д??:
> On Thu, Jun 13, 2024 at 09:49:31AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <[email protected]>
>>
>> This helper is used in iocost, prepare to build iocost as kernel module.
>>
>> Signed-off-by: Yu Kuai <[email protected]>
>> ---
>> fs/kernfs/dir.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
>> index 458519e416fe..84ad163a4281 100644
>> --- a/fs/kernfs/dir.c
>> +++ b/fs/kernfs/dir.c
>> @@ -279,6 +279,7 @@ void pr_cont_kernfs_path(struct kernfs_node *kn)
>> out:
>> spin_unlock_irqrestore(&kernfs_pr_cont_lock, flags);
>> }
>> +EXPORT_SYMBOL_GPL(pr_cont_kernfs_path);
>
> This isn't the helper that needs to be exported, it's a include wrapper
> that is using this for cgroups. Please document this much better and
> perhaps, just fix up the cgroups code instead?
Thanks for the notice, I didn't think much of this, just export API that
is called from iocost. I'll look into this.
Thanks,
Kuai
>
> thanks,
>
> greg k-h
> .
>
On 6/12/24 10:54 PM, Greg KH wrote:
> On Thu, Jun 13, 2024 at 09:49:30AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <[email protected]>
>>
>> Yu Kuai (7):
>> kernfs: export pr_cont_kernfs_path()
>> cgroup: export cgroup_parse_float
>> block: export some API
>> blk-iocost: factor out helpers to handle params from ioc_qos_write()
>> blk-iocost: parse params before initializing iocost
>> blk-iocost: support to free iocost
>> blk-iocost: support to build iocost as kernel module
>
> No where do you say _why_ building this as a module is a good idea.
>
> Why do this at all?
With CONFIG_BLK_CGROUP_IOCOST=y (as in the Android kernel), the
blk-iocost kernel module causes a (small) runtime overhead, even if it
is not being used.
Thanks,
Bart.
Hi,
在 2024/06/14 0:15, Bart Van Assche 写道:
> On 6/12/24 10:54 PM, Greg KH wrote:
>> On Thu, Jun 13, 2024 at 09:49:30AM +0800, Yu Kuai wrote:
>>> From: Yu Kuai <[email protected]>
>>>
>>> Yu Kuai (7):
>>> kernfs: export pr_cont_kernfs_path()
>>> cgroup: export cgroup_parse_float
>>> block: export some API
>>> blk-iocost: factor out helpers to handle params from ioc_qos_write()
>>> blk-iocost: parse params before initializing iocost
>>> blk-iocost: support to free iocost
>>> blk-iocost: support to build iocost as kernel module
>>
>> No where do you say _why_ building this as a module is a good idea.
>>
>> Why do this at all?
>
> With CONFIG_BLK_CGROUP_IOCOST=y (as in the Android kernel), the
> blk-iocost kernel module causes a (small) runtime overhead, even if it
> is not being used.
I think this is not true... Because iocost is lazy initialized, and if
iocost is not initialized, there should not be such overhead.
Thanks,
Kuai
>
> Thanks,
>
> Bart.
>
>
> .
>
This cover letter is a little, erm, sparse.
Please explain why you want iocost as a module. This adds a whole
lot of infrastructure for no obvious reason.