[RFC PATCH v2 0/5] blk-iocost: BPF struct_ops cost model

[RFC PATCH v2 0/5] blk-iocost: BPF struct_ops cost model

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

This is v2 of the RFC.  It incorporates the feedback from the first
review round: the attachment model, pricing ownership and per-cgroup
state handling have all been reworked, and the interface,
registration, dispatch and configuration changes are now folded
into a single patch.  Thanks for the detailed review.

Why a pluggable model at all
----------------------------

When iocost landed in 2019, its commit message already promised that
"a later patch will also allow using bpf progs for cost models", and
the code has carried the split for it ever since: calc_vtime_cost()
is a dispatcher whose only implementation is calc_vtime_cost_builtin().
Seven years later the builtin linear model is still the only one.
This series fills that slot, following the TCP congestion control
model registration pattern: builtin algorithms remain the default
while new ones can be prototyped in BPF.

The measured problems
---------------------

The builtin model prices each IO with a binary sequential/random base
picked by a single per-cgroup cursor and a 16MB seek threshold, plus
a per-page cost.  On a virtio-blk device with the HDD autop profile,
a 4k IO costs ~24us when judged sequential and ~2.7ms when judged
random, a 112x spread, so a wrong judgement becomes a wrong price.
Three classes of mispricing, all measured:

 1. Heuristic rigidity.  Two legitimate sequential readers in one
    cgroup (a database with multiple tablespaces, a threaded backup)
    ping-pong the single cursor and are all priced random: a
    measured 89x overcharge collapses throughput under the same
    weight.
    Random IO within a hot window smaller than the 16MB threshold is
    priced sequential: measured 107x undercharge, an accounting
    escape for hotspot workloads.  No setting of the six builtin
    parameters seems able to fix this: telling the streams apart
    requires per-IO state tracking, which looks like logic rather
    than coefficients.

 2. Device nonlinearity.  SLC-cache phases, SMR band placement and
    shared controllers (multiple NVMe namespaces multiplexing one
    device) make the real cost of an identical IO vary by an order of
    magnitude over time or across namespaces.  A static
    6-parameter linear model has no way to express that.

 3. Unpriced operations.  Flush and zone append fall through to a
    cost of zero and bypass throttling entirely (fixed in a separate
    series already posted), but the same pattern extends to device
    quirks the builtin model was never taught.

Mispricing feeds directly into the control loop: vtime budgets,
surplus donation and the vrate feedback all consume the model's
output, so a wrong model can skew the whole controller.

How
---

A bound BPF model fully owns pricing for every IO on the device:
both the bio charging path and the request-level sizing path consult
it, there is no per-IO or per-path fallback to the builtin formula,
and the model prices every operation including flushes.  The builtin
cursor is not exposed; a model is expected to track its own stream
state.

    u64 calc_cost(u64 opf, u64 nbytes, sector_t sector,
		  struct blkcg *blkcg, u64 model_flags)

opf is the full bio->bi_opf (the operation must be extracted with a
mask, and the REQ_* flag bits, including PREFLUSH/FUA, are part of
it); model_flags carries iocost-specific metadata which is not
part of bio->bi_opf, such as whether the cost calculation is for
a merged request; the return value is vtime, clamped to 1 second
of device time per IO.  blkcg is passed so the model can key
per-cgroup state; state stored in BPF_MAP_TYPE_CGRP_STORAGE
follows the cgroup lifetime, and optional blkcg_online()/
blkcg_offline() callbacks mirror the css lifecycle for models
which want eager setup or teardown.

The registration and binding model follows the TCP congestion
control model registration pattern: registering a struct_ops makes
the model available by its name, while io.cost.model binds one
registered model to a device with "model=<name>" and restores the
builtin model with "model=linear".  Unregistering a model removes
it from the registry so it can no longer be selected by name;
devices already using the model keep using it until they are
switched back to the builtin model, at which point the reference
is released.  Sleepable models are rejected at
verification, since calc_cost() runs under RCU read lock.
Patch overview:

 1/5: the BPF struct_ops cost model support: Kconfig, ops
      definition, name registry, registration, io.cost.model
      binding, unified dispatch and verifier checks
 2/5: selftest with the 2x example model (the full builtin linear
      HDD formula at double cost) plus a runner and the selftest
      kernel config entries
 3/5: add an iocost_ioc_tick tracepoint emitting the per-period
      controller state, so model quality can be evaluated without
      drgn (existing events are state-change driven and silent in
      steady state)
 4/5: a second example model which replaces the single-cursor
      sequentiality heuristic with per-cgroup multi-stream detection
      keyed by the cgroup, the first consumer of the state interface
 5/5: document the model=<name> binding in cgroup-v2.rst

Does it work
------------

Mechanism, verified functionally (QEMU, virtio-blk with the HDD
profile, same 8s sequential-read workload from a 1%-weight cgroup,
builtin vs the 2x example model):

 - per-IO charge: 2882us -> 5722us, a factor of 1.985x; the
   completed IO count halves and total cost.usage is conserved,
   i.e. the model output drives both charging and budgeting
 - the same ratio held across four hosts and 4k/64k/1M block sizes
   in the v1 measurements (2.00-2.03x on flash-backed hosts, within
   2% of 2x on a real HDD behind a loaded host); the charging
   measurement is consistent with the v1 mechanism test, while v2
   additionally dispatches the request sizing path through the
   model
 - edge cases: binding an unknown model name fails with ENOENT;
   unregistering a bound model leaves the device correctly priced
   until it is switched back to the builtin model; the readback
   shows the bound model name; the selftest runner checks the
   write error and errno of every step, including the restoration

Payoff, demonstrated with the multi-stream example model (4/5) on
the same setup, 4k IOs at weight 1000, builtin vs the model:

 - two sequential readers in one cgroup: priced 1961us/op by builtin
   (both judged random by the single cursor) and 23us/op by the
   model (each stream keeps its own slot); the completed IO count
   rises by two orders of magnitude
 - random IO inside an 8M window: priced 24us/op by builtin
   (undercharge, an accounting escape) and 2607us/op by the model
 - single-stream sequential and whole-disk random pricing are
   unchanged, so the model fixes both directions of mispricing
   without introducing a new one

Non-interference, measured on enterprise NVMe: no measurable
overhead when the BPF model is not attached.

---
Changes in v2:
- struct_ops models are now registered by name and bound per
  device through io.cost.model; the previous ctrl=bpf selection
  mechanism, the system-wide single-instance limit and its mutex
  are gone
- a bound model fully owns pricing: the return-0 delegation to the
  builtin formula is gone, the request-level sizing path dispatches
  to the model too, and the builtin cursor is no longer exposed
- iocg_id is replaced by the blkcg kptr; per-cgroup state uses
  cgroup storage with its lifetime, plus optional
  blkcg_online/offline callbacks
- the full bio->bi_opf including PREFLUSH/FUA is preserved in
  opf, while model_flags carries iocost-specific metadata
- sleepable models are rejected in .check_member
- Kconfig depends on DEBUG_INFO_BTF
- tracepoint renamed to iocost_ioc_tick; the example model fixes
  the merged-bio stream advancement and the map exhaustion
  limitation (cgroup storage), the selftest checks real write
  errors and the selftest kernel config carries the new options
- the interface, registration, dispatch and configuration changes
  are folded into one patch

Tao Cui (5):
  blk-iocost: add BPF struct_ops cost model support
  selftests/bpf: add iocost cost model test
  blk-iocost: add iocost_ioc_tick tracepoint for per-period device
    summary
  selftests/bpf: add multi-stream sequentiality example model
  docs: cgroup-v2: document io.cost model=<name> binding

 Documentation/admin-guide/cgroup-v2.rst       |  11 +
 block/Kconfig                                 |   9 +
 block/Makefile                                |   1 +
 block/blk-cgroup.c                            |   3 +
 block/blk-iocost-bpf.c                        | 252 ++++++++++++++++++
 block/blk-iocost.c                            | 138 +++++++++-
 include/linux/blk-iocost.h                    |  82 ++++++
 include/trace/events/iocost.h                 |  40 +++
 tools/testing/selftests/bpf/config            |   2 +
 .../selftests/bpf/prog_tests/iocost_model.c   | 194 ++++++++++++++
 .../selftests/bpf/progs/iocost_model.c        | 117 ++++++++
 tools/testing/selftests/bpf/progs/iocost_ms.c | 137 ++++++++++
 12 files changed, 979 insertions(+), 7 deletions(-)
 create mode 100644 block/blk-iocost-bpf.c
 create mode 100644 include/linux/blk-iocost.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/iocost_model.c
 create mode 100644 tools/testing/selftests/bpf/progs/iocost_model.c
 create mode 100644 tools/testing/selftests/bpf/progs/iocost_ms.c

-- 
2.43.0

[RFC PATCH v2 1/5] blk-iocost: add BPF struct_ops cost model support

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Add the iocost_model_ops struct_ops: a bound BPF model fully replaces
the builtin linear model on a device.  calc_cost() receives the full
bio->bi_opf (including REQ_PREFLUSH and REQ_FUA), the IO size, the
start sector (sector_t), the issuing blkcg and the iocost-specific call
metadata (the merge-path indicator), and is called from both the
bio charging path and the request-level sizing path, so a model
owns pricing for every IO on the device.  The builtin cursor is
not exposed: a model is expected to track its own stream state.

The registration and binding model follows the TCP congestion
model registration pattern: registering a struct_ops makes
the model available by its name (char name[16], validated at
init_member), while io.cost.model
binds one registered model to a device with "model=<name>" and
unbinds with "model=linear" or "ctrl=auto/user".  Unregistering a
model removes it from the registry so it can no longer be selected
by name; devices already using the model continue to do so until
switched back to the builtin model.  References are taken with
bpf_struct_ops_get()/put() on the kdata and released when the
device switches back to the builtin model.

calc_cost() runs under RCU read lock; sleepable programs are rejected
in .check_member.  blkcg_online()/blkcg_offline() callbacks mirroring
the blkcg css lifecycle let models manage per-cgroup state.

Registered and bound models coexist with the builtin model: devices
which are not bound keep the builtin linear model unchanged.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/Kconfig              |   9 ++
 block/Makefile             |   1 +
 block/blk-cgroup.c         |   3 +
 block/blk-iocost-bpf.c     | 250 +++++++++++++++++++++++++++++++++++++
 block/blk-iocost.c         | 133 ++++++++++++++++++--
 include/linux/blk-iocost.h |  82 ++++++++++++
 6 files changed, 471 insertions(+), 7 deletions(-)
 create mode 100644 block/blk-iocost-bpf.c
 create mode 100644 include/linux/blk-iocost.h

diff --git a/block/Kconfig b/block/Kconfig
index 70e4a66d941f..91e808f86d28 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -231,4 +231,13 @@ config BLK_ERROR_INJECTION
 
 source "block/Kconfig.iosched"
 
+config BLK_CGROUP_IOCOST_BPF
+	bool "Enable BPF pluggable cost model support for the cost IO controller"
+	depends on BLK_CGROUP_IOCOST && BPF_SYSCALL && BPF_JIT && DEBUG_INFO_BTF
+	help
+	 Enabling this option registers the "iocost_model_ops" BPF
+	 struct_ops type, which allows a BPF program to fully replace
+	 the builtin linear cost model on a device it is bound to
+	 through io.cost.model.
+
 endif # BLOCK
diff --git a/block/Makefile b/block/Makefile
index e7bd320e3d69..ee5cebeea006 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_BLK_INLINE_ENCRYPTION)	+= blk-crypto.o blk-crypto-profile.o \
 					   blk-crypto-sysfs.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK)	+= blk-crypto-fallback.o
 obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED)	+= holder.o
+obj-$(CONFIG_BLK_CGROUP_IOCOST_BPF)	+= blk-iocost-bpf.o
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 2b5c29434e42..872871045351 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -32,6 +32,7 @@
 #include <linux/part_stat.h>
 #include "blk.h"
 #include "blk-cgroup.h"
+#include <linux/blk-iocost.h>
 #include "blk-ioprio.h"
 #include "blk-throttle.h"
 
@@ -1341,6 +1342,7 @@ void blkcg_unpin_online(struct cgroup_subsys_state *blkcg_css)
  */
 static void blkcg_css_offline(struct cgroup_subsys_state *css)
 {
+	iocost_notify_blkcg_offline(css_to_blkcg(css));
 	/* this prevents anyone from attaching or migrating to this blkcg */
 	wb_blkcg_offline(css);
 
@@ -1445,6 +1447,7 @@ blkcg_css_alloc(struct cgroup_subsys_state *parent_css)
 
 static int blkcg_css_online(struct cgroup_subsys_state *css)
 {
+	iocost_notify_blkcg_online(css_to_blkcg(css));
 	struct blkcg *parent = blkcg_parent(css_to_blkcg(css));
 
 	/*
diff --git a/block/blk-iocost-bpf.c b/block/blk-iocost-bpf.c
new file mode 100644
index 000000000000..aec6df279599
--- /dev/null
+++ b/block/blk-iocost-bpf.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * blk-iocost: BPF struct_ops plumbing for pluggable cost models.
+ *
+ * Registers the "iocost_model_ops" struct_ops type and maintains the
+ * name registry of registered models.  A registered model is bound to
+ * a device through io.cost.model; see include/linux/blk-iocost.h.
+ */
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/bpf.h>
+#include <linux/bpf_verifier.h>
+#include <linux/btf.h>
+#include <linux/blk-iocost.h>
+
+static DEFINE_MUTEX(iocost_bpf_reg_lock);
+static LIST_HEAD(iocost_bpf_models);
+
+/*
+ * The registry holds a bpf_struct_ops_get() reference obtained in .reg;
+ * .unreg drops it, so the kdata of an unregistered model stays alive
+ * while any device is still bound to it.
+ */
+struct iocost_bpf_model {
+	struct list_head	list;
+	const struct iocost_model_ops *ops;
+};
+
+/*
+ * Look up a registered model by name and acquire a reference on it.
+ * The registry lock is held across lookup and bpf_struct_ops_get() so
+ * the model cannot be unregistered in between.
+ */
+int iocost_bpf_model_get(const char *name,
+			 const struct iocost_model_ops **opsp)
+{
+	struct iocost_bpf_model *m;
+	int ret = -ENOENT;
+
+	mutex_lock(&iocost_bpf_reg_lock);
+	list_for_each_entry(m, &iocost_bpf_models, list) {
+		if (!strcmp(m->ops->name, name)) {
+			if (bpf_struct_ops_get(m->ops)) {
+				*opsp = m->ops;
+				ret = 0;
+			}
+			break;
+		}
+	}
+	mutex_unlock(&iocost_bpf_reg_lock);
+	return ret;
+}
+
+void iocost_bpf_model_put(const struct iocost_model_ops *ops)
+{
+	bpf_struct_ops_put(ops);
+}
+
+static struct iocost_bpf_model *
+iocost_bpf_model_lookup(const struct iocost_model_ops *ops)
+{
+	struct iocost_bpf_model *m;
+
+	list_for_each_entry(m, &iocost_bpf_models, list) {
+		if (m->ops == ops)
+			return m;
+	}
+	return NULL;
+}
+
+void iocost_notify_blkcg_online(struct blkcg *blkcg)
+{
+	struct iocost_bpf_model *m;
+
+	guard(mutex)(&iocost_bpf_reg_lock);
+	list_for_each_entry(m, &iocost_bpf_models, list) {
+		if (m->ops->blkcg_online)
+			m->ops->blkcg_online(blkcg);
+	}
+}
+
+void iocost_notify_blkcg_offline(struct blkcg *blkcg)
+{
+	struct iocost_bpf_model *m;
+
+	guard(mutex)(&iocost_bpf_reg_lock);
+	list_for_each_entry(m, &iocost_bpf_models, list) {
+		if (m->ops->blkcg_offline)
+			m->ops->blkcg_offline(blkcg);
+	}
+}
+
+static int bpf_iocost_model_init(struct btf *btf)
+{
+	s32 type_id;
+
+	type_id = btf_find_by_name_kind(btf, "iocost_model_ops", BTF_KIND_STRUCT);
+	if (type_id < 0)
+		return -EINVAL;
+	return 0;
+}
+
+static bool bpf_iocost_is_valid_access(int off, int size,
+				       enum bpf_access_type type,
+				       const struct bpf_prog *prog,
+				       struct bpf_insn_access_aux *info)
+{
+	return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
+}
+
+static const struct bpf_func_proto *
+bpf_iocost_get_func_proto(enum bpf_func_id func_id,
+			  const struct bpf_prog *prog)
+{
+	switch (func_id) {
+#ifdef CONFIG_CGROUPS
+	case BPF_FUNC_cgrp_storage_get:
+		return &bpf_cgrp_storage_get_proto;
+#endif
+	default:
+		return bpf_base_func_proto(func_id, prog);
+	}
+}
+
+static int bpf_iocost_check_member(const struct btf_type *t,
+				   const struct btf_member *member,
+				   const struct bpf_prog *prog)
+{
+	/* calc_cost() is called with RCU read lock held */
+	if (prog->sleepable)
+		return -EINVAL;
+	return 0;
+}
+
+static int bpf_iocost_init_member(const struct btf_type *t,
+				  const struct btf_member *member,
+				  void *kdata, const void *udata)
+{
+	struct iocost_model_ops *ops = kdata;
+	const struct iocost_model_ops *uops = udata;
+	u32 moff = __btf_member_bit_offset(t, member) / 8;
+
+	switch (moff) {
+	case offsetof(struct iocost_model_ops, name):
+		if (bpf_obj_name_cpy(ops->name, uops->name,
+				     sizeof(ops->name)) <= 0)
+			return -EINVAL;
+		return 1;
+	}
+
+	return 0;
+}
+
+static int bpf_iocost_validate(void *kdata)
+{
+	struct iocost_model_ops *ops = kdata;
+
+	return ops->calc_cost ? 0 : -EINVAL;
+}
+
+static int bpf_iocost_reg(void *kdata, struct bpf_link *link)
+{
+	struct iocost_model_ops *ops = kdata;
+	struct iocost_bpf_model *m;
+	int ret = 0;
+
+	if (!bpf_struct_ops_get(ops))
+		return -ENOENT;
+
+	m = kzalloc(sizeof(*m), GFP_KERNEL);
+	if (!m) {
+		bpf_struct_ops_put(ops);
+		return -ENOMEM;
+	}
+
+	mutex_lock(&iocost_bpf_reg_lock);
+	{
+		struct iocost_bpf_model *other;
+
+		list_for_each_entry(other, &iocost_bpf_models, list) {
+			if (!strcmp(other->ops->name, ops->name)) {
+				ret = -EEXIST;
+				break;
+			}
+		}
+	}
+	if (!ret) {
+		m->ops = ops;
+		list_add(&m->list, &iocost_bpf_models);
+	}
+	mutex_unlock(&iocost_bpf_reg_lock);
+
+	if (ret) {
+		bpf_struct_ops_put(ops);
+		kfree(m);
+	}
+	return ret;
+}
+
+static void bpf_iocost_unreg(void *kdata, struct bpf_link *link)
+{
+	struct iocost_model_ops *ops = kdata;
+	struct iocost_bpf_model *m;
+
+	mutex_lock(&iocost_bpf_reg_lock);
+	m = iocost_bpf_model_lookup(ops);
+	if (m) {
+		list_del(&m->list);
+		bpf_struct_ops_put(ops);
+		kfree(m);
+	}
+	mutex_unlock(&iocost_bpf_reg_lock);
+}
+
+static const struct bpf_verifier_ops bpf_iocost_verifier_ops = {
+	.get_func_proto = bpf_iocost_get_func_proto,
+	.is_valid_access = bpf_iocost_is_valid_access,
+};
+
+static u64 bpf_iocost_calc_cost_stub(u64 opf, u64 nbytes, u64 sector,
+				     struct blkcg *blkcg, u64 flags)
+{
+	return 0;
+}
+
+static struct iocost_model_ops __bpf_ops_iocost_model_ops = {
+	.calc_cost = bpf_iocost_calc_cost_stub,
+};
+
+static struct bpf_struct_ops bpf_iocost_model_ops = {
+	.verifier_ops = &bpf_iocost_verifier_ops,
+	.init = bpf_iocost_model_init,
+	.check_member = bpf_iocost_check_member,
+	.init_member = bpf_iocost_init_member,
+	.validate = bpf_iocost_validate,
+	.reg = bpf_iocost_reg,
+	.unreg = bpf_iocost_unreg,
+	.name = "iocost_model_ops",
+	.cfi_stubs = &__bpf_ops_iocost_model_ops,
+	.owner = THIS_MODULE,
+};
+
+static int __init bpf_iocost_init(void)
+{
+	return register_bpf_struct_ops(&bpf_iocost_model_ops, iocost_model_ops);
+}
+late_initcall(bpf_iocost_init);
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 2745bffcd5ee..182601ad783f 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -177,6 +177,7 @@
 #include <linux/timer.h>
 #include <linux/time64.h>
 #include <linux/parser.h>
+#include <linux/blk-iocost.h>
 #include <linux/sched/signal.h>
 #include <asm/local.h>
 #include <asm/local64.h>
@@ -445,6 +446,11 @@ struct ioc {
 	int				autop_idx;
 	bool				user_qos_params:1;
 	bool				user_cost_model:1;
+
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	/* bound BPF cost model, NULL = builtin linear model */
+	const struct iocost_model_ops	__rcu *model;
+#endif
 };
 
 struct iocg_pcpu_stat {
@@ -2571,10 +2577,28 @@ static void calc_vtime_cost_builtin(struct bio *bio, struct ioc_gq *iocg,
 
 static u64 calc_vtime_cost(struct bio *bio, struct ioc_gq *iocg, bool is_merge)
 {
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
 	u64 cost;
 
-	calc_vtime_cost_builtin(bio, iocg, is_merge, &cost);
-	return cost;
+	rcu_read_lock();
+	model = rcu_dereference(iocg->ioc->model);
+	if (model) {
+		cost = model->calc_cost(bio->bi_opf, bio->bi_iter.bi_size,
+					bio->bi_iter.bi_sector,
+					iocg_to_blkg(iocg)->blkcg,
+					is_merge ? IOCOST_COST_F_MERGE : 0);
+		rcu_read_unlock();
+		return min(cost, VTIME_PER_SEC);
+	}
+	rcu_read_unlock();
+#endif
+	{
+		u64 cost;
+
+		calc_vtime_cost_builtin(bio, iocg, is_merge, &cost);
+		return cost;
+	}
 }
 
 static void calc_size_vtime_cost_builtin(struct request *rq, struct ioc *ioc,
@@ -2596,10 +2620,28 @@ static void calc_size_vtime_cost_builtin(struct request *rq, struct ioc *ioc,
 
 static u64 calc_size_vtime_cost(struct request *rq, struct ioc *ioc)
 {
-	u64 cost;
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
 
-	calc_size_vtime_cost_builtin(rq, ioc, &cost);
-	return cost;
+	rcu_read_lock();
+	model = rcu_dereference(ioc->model);
+	if (model && rq->bio && rq->bio->bi_blkg) {
+		u64 cost;
+
+		cost = model->calc_cost(rq->cmd_flags, blk_rq_bytes(rq),
+					blk_rq_pos(rq),
+					rq->bio->bi_blkg->blkcg, 0);
+		rcu_read_unlock();
+		return min(cost, VTIME_PER_SEC);
+	}
+	rcu_read_unlock();
+#endif
+	{
+		u64 cost;
+
+		calc_size_vtime_cost_builtin(rq, ioc, &cost);
+		return cost;
+	}
 }
 
 enum over_budget_action {
@@ -2900,6 +2942,19 @@ static void ioc_rqos_exit(struct rq_qos *rqos)
 
 	timer_shutdown_sync(&ioc->timer);
 	free_percpu(ioc->pcpu_stat);
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	{
+		const struct iocost_model_ops *model;
+
+		spin_lock_irq(&ioc->lock);
+		model = rcu_dereference_protected(ioc->model,
+					lockdep_is_held(&ioc->lock));
+		rcu_assign_pointer(ioc->model, NULL);
+		spin_unlock_irq(&ioc->lock);
+		if (model)
+			iocost_bpf_model_put(model);
+	}
+#endif
 	kfree(ioc);
 }
 
@@ -3438,12 +3493,30 @@ static u64 ioc_cost_model_prfill(struct seq_file *sf,
 		return 0;
 
 	spin_lock_irq(&ioc->lock);
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	{
+		const struct iocost_model_ops *model =
+			rcu_dereference_protected(ioc->model,
+					lockdep_is_held(&ioc->lock));
+
+		seq_printf(sf, "%s ctrl=%s model=%s "
+			   "rbps=%llu rseqiops=%llu rrandiops=%llu "
+			   "wbps=%llu wseqiops=%llu wrandiops=%llu\n",
+			   dname, model ? "bpf" :
+				   ioc->user_cost_model ? "user" : "auto",
+			   model ? model->name : "linear",
+			   u[I_LCOEF_RBPS], u[I_LCOEF_RSEQIOPS],
+			   u[I_LCOEF_RRANDIOPS], u[I_LCOEF_WBPS],
+			   u[I_LCOEF_WSEQIOPS], u[I_LCOEF_WRANDIOPS]);
+	}
+#else
 	seq_printf(sf, "%s ctrl=%s model=linear "
 		   "rbps=%llu rseqiops=%llu rrandiops=%llu "
 		   "wbps=%llu wseqiops=%llu wrandiops=%llu\n",
 		   dname, ioc->user_cost_model ? "user" : "auto",
 		   u[I_LCOEF_RBPS], u[I_LCOEF_RSEQIOPS], u[I_LCOEF_RRANDIOPS],
 		   u[I_LCOEF_WBPS], u[I_LCOEF_WSEQIOPS], u[I_LCOEF_WRANDIOPS]);
+#endif
 	spin_unlock_irq(&ioc->lock);
 	return 0;
 }
@@ -3457,6 +3530,37 @@ static int ioc_cost_model_show(struct seq_file *sf, void *v)
 	return 0;
 }
 
+/*
+ * Bind @name (empty = builtin linear model) as the active cost model of
+ * @ioc.  The registry lookup and reference management happen outside
+ * ioc->lock; the pointer swap happens under it.
+ */
+static int ioc_bpf_model_bind(struct ioc *ioc, const char *name)
+{
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *new = NULL, *old;
+	int ret;
+
+	if (name[0]) {
+		ret = iocost_bpf_model_get(name, &new);
+		if (ret)
+			return ret;
+	}
+
+	spin_lock_irq(&ioc->lock);
+	old = rcu_dereference_protected(ioc->model,
+					lockdep_is_held(&ioc->lock));
+	rcu_assign_pointer(ioc->model, new);
+	spin_unlock_irq(&ioc->lock);
+
+	if (old)
+		iocost_bpf_model_put(old);
+	return 0;
+#else
+	return name[0] ? -ENOENT : 0;
+#endif
+}
+
 static const match_table_t cost_ctrl_tokens = {
 	{ COST_CTRL,		"ctrl=%s"	},
 	{ COST_MODEL,		"model=%s"	},
@@ -3482,6 +3586,7 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 	struct ioc *ioc;
 	u64 u[NR_I_LCOEFS];
 	bool user;
+	char bpf_model[IOCOST_MODEL_NAME_LEN];
 	char *body, *p;
 	int ret;
 
@@ -3512,6 +3617,7 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 	spin_lock_irq(&ioc->lock);
 	memcpy(u, ioc->params.i_lcoefs, sizeof(u));
 	user = ioc->user_cost_model;
+	bpf_model[0] = '\0';
 
 	ret = -EINVAL;
 
@@ -3533,11 +3639,16 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 				user = true;
 			else
 				goto unlock;
+			bpf_model[0] = '\0';
 			continue;
 		case COST_MODEL:
 			match_strlcpy(buf, &args[0], sizeof(buf));
-			if (strcmp(buf, "linear"))
-				goto unlock;
+			if (!strcmp(buf, "linear")) {
+				/* back to the builtin linear model */
+				bpf_model[0] = '\0';
+				continue;
+			}
+			match_strlcpy(bpf_model, &args[0], sizeof(bpf_model));
 			continue;
 		}
 
@@ -3563,6 +3674,14 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 unlock:
 	spin_unlock_irq(&ioc->lock);
 
+	/*
+	 * Bind the BPF model outside ioc->lock: the registry lookup
+	 * takes the registration mutex and the old model's reference
+	 * is dropped after the swap.
+	 */
+	if (!ret)
+		ret = ioc_bpf_model_bind(ioc, bpf_model);
+
 	blk_mq_unquiesce_queue(q);
 	blk_mq_unfreeze_queue(q, memflags);
 
diff --git a/include/linux/blk-iocost.h b/include/linux/blk-iocost.h
new file mode 100644
index 000000000000..3a0855efb610
--- /dev/null
+++ b/include/linux/blk-iocost.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_BLK_IOCOST_H
+#define _LINUX_BLK_IOCOST_H
+
+#include <linux/types.h>
+#include <linux/blk_types.h>
+
+#define IOCOST_MODEL_NAME_LEN	16
+
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+
+struct blkcg;
+
+/*
+ * Pluggable cost model interface for blk-iocost.
+ *
+ * A BPF struct_ops implementation registered against "iocost_model_ops"
+ * fully replaces the builtin linear model on the devices it is bound to
+ * through io.cost.model.  The model owns pricing for every IO on a bound
+ * device: it prices all operations, including flushes, and both the bio
+ * charging path and the request-level sizing path consult it.
+ *
+ * calc_cost() is called from the IO submission path with RCU read lock
+ * held and must not sleep.  It returns the cost of the IO in vtime
+ * units, where 1 second of device time equals VTIME_PER_SEC (2^37,
+ * available to BPF programs through vmlinux.h).  The returned value is
+ * clamped to 1 second of device time per IO.
+ *
+ * The model is passed the blkcg of the issuing cgroup so it can keep
+ * per-cgroup state.  blkcg_online()/blkcg_offline() are optional
+ * callbacks mirroring the blkcg css lifecycle: state created on online
+ * (or lazily on first use) must be released on offline.
+ *
+ * The registration and binding model follows the TCP congestion
+ * control framework: registering a struct_ops makes the model available
+ * by its name, while io.cost.model binds one registered model to a
+ * device.  Unregistering removes the name from the registry; devices
+ * already bound keep using it until switched back to the builtin
+ * model.
+ */
+
+#define IOCOST_MODEL_NAME_LEN	16
+
+/*
+ * iocost-specific call metadata for calc_cost()'s model_flags
+ * argument; everything else, including REQ_PREFLUSH/REQ_FUA, is
+ * already present in the opf argument
+ */
+#define IOCOST_COST_F_MERGE	(1ULL << 0)	/* called from merge path */
+
+struct iocost_model_ops {
+	u64 (*calc_cost)(u64 opf, u64 nbytes, sector_t sector,
+			 struct blkcg *blkcg, u64 model_flags);
+	void (*blkcg_online)(struct blkcg *blkcg);
+	void (*blkcg_offline)(struct blkcg *blkcg);
+
+	/* model name, used to select the model through io.cost.model */
+	char name[16];
+};
+
+int iocost_bpf_model_get(const char *name,
+			 const struct iocost_model_ops **opsp);
+void iocost_bpf_model_put(const struct iocost_model_ops *ops);
+void iocost_notify_blkcg_online(struct blkcg *blkcg);
+void iocost_notify_blkcg_offline(struct blkcg *blkcg);
+
+#else	/* CONFIG_BLK_CGROUP_IOCOST_BPF */
+
+struct blkcg;
+struct iocost_model_ops;
+
+static inline int iocost_bpf_model_get(const char *name,
+				       const struct iocost_model_ops **opsp)
+{
+	return -EOPNOTSUPP;
+}
+static inline void iocost_bpf_model_put(const struct iocost_model_ops *ops) { }
+static inline void iocost_notify_blkcg_online(struct blkcg *blkcg) { }
+static inline void iocost_notify_blkcg_offline(struct blkcg *blkcg) { }
+
+#endif	/* CONFIG_BLK_CGROUP_IOCOST_BPF */
+#endif	/* _LINUX_BLK_IOCOST_H */
-- 
2.43.0

[RFC PATCH v2 2/5] selftests/bpf: add iocost cost model test

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Add an example cost model implementing the full builtin linear HDD
formula at double cost, including flush pricing, and a runner which
registers it as a struct_ops and binds it to a device through
io.cost.model with "model=iocost_2x", verifying the readback and
restoring "model=linear" afterwards, checking the write error and
errno of every step including the restoration.  Binding an unknown
model name is verified to fail with ENOENT.  Under the same workload
the doubled model charges exactly twice the builtin model (verified
2882us -> 5722us per IO, completed IO count halved).

Per-cgroup stream state uses a CGRP_STORAGE map keyed by the cgroup
of the blkcg argument, so the model inherits the cgroup lifetime and
never leaks or reuses stale state.  opf carries the full
bio->bi_opf including REQ_* flag bits, so the operation must be
extracted with a mask, not compared for equality.

All writes go through write(2) rather than stdio, since the kernel's
rejection happens in the write() syscall, not in the userspace buffer
copy.  CONFIG_BLK_CGROUP_IOCOST and CONFIG_BLK_CGROUP_IOCOST_BPF are
added to the selftest kernel config: without them vmlinux.h does not
contain iocost_model_ops and the skeletons fail to build; the runtime
skip cannot avoid a build dependency.

The runner is skipped unless $IOCOST_TEST_DEV gives a major:minor of
a device with iocost enabled.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/testing/selftests/bpf/config            |   2 +
 .../selftests/bpf/prog_tests/iocost_model.c   | 193 ++++++++++++++++++
 .../selftests/bpf/progs/iocost_model.c        | 116 +++++++++++
 3 files changed, 311 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/iocost_model.c
 create mode 100644 tools/testing/selftests/bpf/progs/iocost_model.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 2f79688dcf7c..67a630cb5614 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -138,3 +138,5 @@ CONFIG_SMC_HS_CTRL_BPF=y
 CONFIG_DIBS=y
 CONFIG_DIBS_LO=y
 CONFIG_PM_WAKELOCKS=y
+CONFIG_BLK_CGROUP_IOCOST=y
+CONFIG_BLK_CGROUP_IOCOST_BPF=y
diff --git a/tools/testing/selftests/bpf/prog_tests/iocost_model.c b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
new file mode 100644
index 000000000000..e9344b13fba9
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include "iocost_model.skel.h"
+#include "iocost_ms.skel.h"
+
+/*
+ * Write a line to io.cost.model with write(2) and return the errno of
+ * the failed write, or 0 on success.  stdio is not used here on
+ * purpose: the kernel's rejection happens in the write() syscall,
+ * not in the userspace buffer copy, and every write, including
+ * the error paths of the callers below, is checked.
+ */
+static int write_cost_model(const char *buf)
+{
+	int fd, err = 0;
+	ssize_t n;
+
+	fd = open("/sys/fs/cgroup/io.cost.model", O_WRONLY);
+	if (fd < 0)
+		return errno;
+	n = write(fd, buf, strlen(buf));
+	if (n < 0)
+		err = errno;
+	close(fd);
+	return err;
+}
+
+/*
+ * Read back the io.cost.model line of dev and copy the model= value
+ * into @model.  Returns 0 on success.
+ */
+static int readback_model(const char *dev, char *model, size_t model_sz)
+{
+	char line[256], word[256], *m, *end;
+	FILE *fp;
+	int found = 0;
+
+	fp = fopen("/sys/fs/cgroup/io.cost.model", "r");
+	if (!fp)
+		return -1;
+	while (fgets(line, sizeof(line), fp)) {
+		if (sscanf(line, "%255s", word) == 1 && !strcmp(word, dev)) {
+			found = 1;
+			break;
+		}
+	}
+	fclose(fp);
+	if (!found)
+		return -1;
+
+	m = strstr(line, "model=");
+	if (!m)
+		return -1;
+	m += strlen("model=");
+	end = m;
+	while (*end && !isspace(*end))
+		end++;
+	snprintf(model, model_sz, "%.*s", (int)(end - m), m);
+	return 0;
+}
+
+/*
+ * Bind the named model to a device with "model=<name>", verify the
+ * readback and restore the builtin model.  Returns 0 on success.
+ */
+static int bind_model(const char *dev, const char *name)
+{
+	char buf[300], got[64];
+	int err;
+
+	snprintf(buf, sizeof(buf), "%s model=%s\n", dev, name);
+	err = write_cost_model(buf);
+	if (err) {
+		CHECK(false, "write_model", "write model=%s: %s\n", name,
+		      strerror(err));
+		return -1;
+	}
+	err = readback_model(dev, got, sizeof(got));
+	if (err || strcmp(got, name)) {
+		CHECK(false, "readback_model", "got model=%s want %s\n",
+		      err ? "(none)" : got, name);
+		return -1;
+	}
+
+	/* restore the builtin linear model; the write is checked too */
+	snprintf(buf, sizeof(buf), "%s model=linear\n", dev);
+	err = write_cost_model(buf);
+	if (err) {
+		CHECK(false, "restore_linear", "write model=linear: %s\n",
+		      strerror(err));
+		return -1;
+	}
+	return 0;
+}
+
+/*
+ * The dev argument must be present in io.cost.qos already, which
+ * means iocost is enabled for it.
+ */
+static int dev_has_iocost(const char *dev)
+{
+	char line[256], word[256];
+	FILE *fp;
+	int found = 0;
+
+	fp = fopen("/sys/fs/cgroup/io.cost.qos", "r");
+	if (!fp)
+		return 0;
+	while (fgets(line, sizeof(line), fp)) {
+		if (sscanf(line, "%255s", word) == 1 && !strcmp(word, dev)) {
+			found = 1;
+			break;
+		}
+	}
+	fclose(fp);
+	return found;
+}
+
+/*
+ * Bind the 2x example model and verify the io.cost.model readback.
+ * IO accounting itself is not checked here; it needs a device doing
+ * real IO under iocost and is covered by the kernel-side validation
+ * described in the cover letter.
+ *
+ * Requires root, cgroup v2 and a device with iocost support.  The
+ * device must be given as major:minor in $IOCOST_TEST_DEV, otherwise
+ * the test is skipped.
+ */
+void serial_test_iocost_model(void)
+{
+	struct iocost_model *skel;
+	char buf[300], *dev;
+	int err;
+
+	dev = getenv("IOCOST_TEST_DEV");
+	if (!dev || geteuid() != 0) {
+		test__skip();
+		return;
+	}
+	if (!ASSERT_TRUE(dev_has_iocost(dev), "iocost_mounted"))
+		return;
+
+	/*
+	 * negative: binding an unknown model name must be rejected,
+	 * so a typo cannot silently disable cost model updates
+	 */
+	snprintf(buf, sizeof(buf), "%s model=no_such_model\n", dev);
+	err = write_cost_model(buf);
+	ASSERT_EQ(err, ENOENT, "unknown_model_rejected");
+
+	skel = iocost_model__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_load"))
+		return;
+
+	/* attaching the struct_ops registers the model by name */
+	err = iocost_model__attach(skel);
+	if (ASSERT_OK(err, "attach"))
+		ASSERT_OK(bind_model(dev, "iocost_2x"), "bind_and_readback");
+
+	iocost_model__destroy(skel);
+}
+
+/*
+ * Same check for the multi-stream example model.  Only one model can
+ * be bound to a device at a time; both tests bind and restore, so
+ * they are serial and independent.
+ */
+void serial_test_iocost_model_streams(void)
+{
+	struct iocost_ms *skel;
+	char *dev;
+	int err;
+
+	dev = getenv("IOCOST_TEST_DEV");
+	if (!dev || geteuid() != 0) {
+		test__skip();
+		return;
+	}
+	if (!ASSERT_TRUE(dev_has_iocost(dev), "iocost_mounted"))
+		return;
+
+	skel = iocost_ms__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_load"))
+		return;
+
+	err = iocost_ms__attach(skel);
+	if (ASSERT_OK(err, "attach"))
+		ASSERT_OK(bind_model(dev, "iocost_ms"), "bind_and_readback");
+
+	iocost_ms__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/iocost_model.c b/tools/testing/selftests/bpf/progs/iocost_model.c
new file mode 100644
index 000000000000..f8ad417b3584
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iocost_model.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example iocost cost model: the builtin linear HDD formula with all
+ * costs doubled.
+ *
+ * The constants mirror what calc_lcoefs() derives from the AUTOP_HDD
+ * defaults (rbps=174019176 rseqiops=41708 rrandiops=370, w-side
+ * analog) in vtime units where 1s == 2^37, expressed with the same
+ * round-up divisions so they cannot drift from the kernel.  A device
+ * bound to this model through io.cost.model charges exactly twice the
+ * builtin model under the same workload, which makes it a convenient
+ * way to verify that accounting goes through the BPF path.
+ *
+ * The model implements the full linear formula itself, including
+ * flushes: there is no fallback to the builtin model, a dataless
+ * WRITE|REQ_PREFLUSH is priced as a one-page write.
+ */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* VTIME_PER_SEC comes from vmlinux.h (a BTF enum constant) */
+#define IOC_PAGE_SIZE		4096
+#define IOC_SECT_TO_PAGE_SHIFT	3	/* 512B sectors to 4kB pages */
+#define LCOEF_RANDIO_PAGES	4096	/* 16MB seek threshold */
+#define IOCOST_COST_F_MERGE	(1ULL << 0)	/* not in BTF: a plain macro */
+#define IOCOST_REQ_OP_MASK	0xff		/* REQ_OP_MASK, not in BTF */
+
+/*
+ * DIV64_U64_ROUND_UP / DIV_ROUND_UP_ULL equivalents, folded at
+ * compile time
+ */
+#define RU(x, y)		((x) / (y) + (((x) % (y)) ? 1 : 0))
+
+#define RBPS	174019176ULL
+#define RSEQIOPS	41708ULL
+#define RRANDIOPS	370ULL
+#define WBPS	178075866ULL
+#define WSEQIOPS	42705ULL
+#define WRANDIOPS	378ULL
+
+#define RPAGE	(RU(VTIME_PER_SEC, RU(RBPS, IOC_PAGE_SIZE)))
+#define RSEQIO	(RU(VTIME_PER_SEC, RSEQIOPS) - RPAGE)
+#define RRANDIO	(RU(VTIME_PER_SEC, RRANDIOPS) - RPAGE)
+#define WPAGE	(RU(VTIME_PER_SEC, RU(WBPS, IOC_PAGE_SIZE)))
+#define WSEQIO	(RU(VTIME_PER_SEC, WSEQIOPS) - WPAGE)
+#define WRANDIO	(RU(VTIME_PER_SEC, WRANDIOPS) - WPAGE)
+
+/*
+ * per-cgroup cursor storage: keyed by the cgroup, freed with it, so
+ * the model never leaks or reuses stale per-cgroup state
+ */
+struct {
+	__uint(type, BPF_MAP_TYPE_CGRP_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, __u64);
+} cursor_store SEC(".maps");
+
+SEC("struct_ops")
+u64 BPF_PROG(iocost_2x_calc_cost, u64 opf, u64 nbytes, u64 sector,
+	     struct blkcg *blkcg, u64 model_flags)
+{
+	u64 pages, seek_pages = 0, base, coef_page, randio, cost;
+
+	if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_READ) {
+		base = RSEQIO; coef_page = RPAGE; randio = RRANDIO;
+	} else if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_WRITE) {
+		base = WSEQIO; coef_page = WPAGE; randio = WRANDIO;
+	} else {
+		/*
+		 * a fully owning model must price every op; unknown
+		 * ops are priced as a single page write
+		 */
+		base = 0; coef_page = WPAGE; randio = 0;
+	}
+
+	/*
+	 * mirror the builtin single per-cgroup cursor: the model keeps
+	 * its own cursor keyed by the blkcg argument
+	 */
+	{
+		__u64 *cursor, cur;
+
+		cursor = bpf_cgrp_storage_get(&cursor_store,
+					      blkcg->css.cgroup, NULL,
+					      BPF_LOCAL_STORAGE_GET_F_CREATE);
+		if (!cursor)
+			return 2 * (base + RU(nbytes, IOC_PAGE_SIZE) * coef_page);
+		cur = *cursor;
+		seek_pages = sector > cur ? sector - cur : cur - sector;
+		seek_pages >>= IOC_SECT_TO_PAGE_SHIFT;
+		if (seek_pages > LCOEF_RANDIO_PAGES)
+			base = randio;
+		if (!(model_flags & IOCOST_COST_F_MERGE))
+			*cursor = sector + RU(nbytes, 512);
+	}
+
+	pages = RU(nbytes, IOC_PAGE_SIZE);
+	if (!pages)
+		pages = 1;	/* dataless flush: one page */
+	if (model_flags & IOCOST_COST_F_MERGE)
+		base = 0;
+
+	cost = 2 * (base + pages * coef_page);
+	return cost;
+}
+
+SEC(".struct_ops")
+struct iocost_model_ops iocost_2x = {
+	.calc_cost = (void *)iocost_2x_calc_cost,
+	.name = "iocost_2x",
+};
+
+char LICENSE[] SEC("license") = "GPL";
-- 
2.43.0

Re: [RFC PATCH v2 2/5] selftests/bpf: add iocost cost model test

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 14186 bytes --]

> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 2f79688dcf7c..67a630cb5614 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -138,3 +138,5 @@ CONFIG_SMC_HS_CTRL_BPF=y
>  CONFIG_DIBS=y
>  CONFIG_DIBS_LO=y
>  CONFIG_PM_WAKELOCKS=y
> +CONFIG_BLK_CGROUP_IOCOST=y
> +CONFIG_BLK_CGROUP_IOCOST_BPF=y
> diff --git a/tools/testing/selftests/bpf/prog_tests/iocost_model.c b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> new file mode 100644
> index 000000000000..e9344b13fba9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include "iocost_model.skel.h"
> +#include "iocost_ms.skel.h"

Does this commit build?  The new test includes iocost_ms.skel.h and
calls iocost_ms__open_and_load(), iocost_ms__attach(), and
iocost_ms__destroy() in serial_test_iocost_model_streams() below, but
this commit adds only progs/iocost_model.c under the progs directory.
progs/iocost_ms.c is added two commits later by b33a817eec7c.

The selftest Makefile derives skeleton generation rules from:

  TRUNNER_BPF_SRCS := $(notdir $(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))
  TRUNNER_BPF_SKELS := $(patsubst %.c,$(TRUNNER_OUTPUT)/%.skel.h, ...)

so iocost_ms.skel.h has no generation rule at this commit and make
fails with "No rule to make target '.../iocost_ms.skel.h', needed by
'.../iocost_model.test.o'".  Because prog_tests/*.o files all feed the
single test_progs binary, this breaks the build of the entire bpf
selftest suite.

The multi-stream model and test belong in the later commit, or
progs/iocost_ms.c belongs in this one.  As-is the series is not
bisectable.

[ ... ]

> +static int readback_model(const char *dev, char *model, size_t model_sz)
> +{
> +	char line[256], word[256], *m, *end;
> +	FILE *fp;
> +	int found = 0;
> +
> +	fp = fopen("/sys/fs/cgroup/io.cost.model", "r");
> +	if (!fp)
> +		return -1;
> +	while (fgets(line, sizeof(line), fp)) {
> +		if (sscanf(line, "%255s", word) == 1 && !strcmp(word, dev)) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	fclose(fp);
> +	if (!found)
> +		return -1;
> +
> +	m = strstr(line, "model=");
> +	if (!m)
> +		return -1;
> +	m += strlen("model=");
> +	end = m;
> +	while (*end && !isspace(*end))
> +		end++;

Does this build?  isspace() is used but <ctype.h> is never included.
The file includes only <test_progs.h>, <fcntl.h> and <unistd.h>.
test_progs.h includes stdio.h, unistd.h, errno.h, string.h, assert.h,
regex.h, stdlib.h, stdarg.h, time.h, signal.h and various linux/ and
sys/ headers, none of which declare isspace() in glibc.

tools/testing/selftests/bpf/Makefile builds userspace objects with
-Wall -Werror, so the implicit declaration is an error (and is an error
by default in GCC 14+ / C23 regardless).  The existing precedent in the
same directory, prog_tests/ctx_rewrite.c, is the only other prog_test
that calls isspace() and it explicitly includes <ctype.h>.

[ ... ]

> +/*
> + * The dev argument must be present in io.cost.qos already, which
> + * means iocost is enabled for it.
> + */
> +static int dev_has_iocost(const char *dev)
> +{
> +	char line[256], word[256];
> +	FILE *fp;
> +	int found = 0;
> +
> +	fp = fopen("/sys/fs/cgroup/io.cost.qos", "r");
> +	if (!fp)
> +		return 0;
> +	while (fgets(line, sizeof(line), fp)) {
> +		if (sscanf(line, "%255s", word) == 1 && !strcmp(word, dev)) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	fclose(fp);
> +	return found;
> +}

Does this helper match its comment?  The comment asserts that presence
in io.cost.qos "means iocost is enabled", but ioc_qos_prfill() prints
a line for every device that has the iocost policy activated, including
devices with enable=0:

block/blk-iocost.c:ioc_qos_prfill()
  seq_printf(sf, "%s enable=%d ctrl=%s rpct=...", dname, ioc->enabled, ...);

dev_has_iocost() only matches the leading dname and never looks at the
enable= field, so it returns 1 for a device where iocost is present but
disabled.  To match its own comment the helper should parse enable=1.

Relatedly, the commit message states "The runner is skipped unless
$IOCOST_TEST_DEV gives a major:minor of a device with iocost enabled",
but the code does not skip in that case:

> +void serial_test_iocost_model(void)
> +{
> +	struct iocost_model *skel;
> +	char buf[300], *dev;
> +	int err;
> +
> +	dev = getenv("IOCOST_TEST_DEV");
> +	if (!dev || geteuid() != 0) {
> +		test__skip();
> +		return;
> +	}
> +	if (!ASSERT_TRUE(dev_has_iocost(dev), "iocost_mounted"))
> +		return;

ASSERT_TRUE calls test__fail() if the condition is false, so setting
IOCOST_TEST_DEV to a device without iocost, or running where cgroup2 is
not mounted at /sys/fs/cgroup (fopen fails, dev_has_iocost returns 0),
produces a hard test failure rather than a skip.

[ ... ]

> +/*
> + * Write a line to io.cost.model with write(2) and return the errno of
> + * the failed write, or 0 on success.  stdio is not used here on
> + * purpose: the kernel's rejection happens in the write() syscall,
> + * not in the userspace buffer copy, and every write, including
> + * the error paths of the callers below, is checked.
> + */
> +static int write_cost_model(const char *buf)
> +{
> +	int fd, err = 0;
> +	ssize_t n;
> +
> +	fd = open("/sys/fs/cgroup/io.cost.model", O_WRONLY);
> +	if (fd < 0)
> +		return errno;
> +	n = write(fd, buf, strlen(buf));
> +	if (n < 0)
> +		err = errno;
> +	close(fd);
> +	return err;
> +}

[ ... ]

> +	/*
> +	 * negative: binding an unknown model name must be rejected,
> +	 * so a typo cannot silently disable cost model updates
> +	 */
> +	snprintf(buf, sizeof(buf), "%s model=no_such_model\n", dev);
> +	err = write_cost_model(buf);
> +	ASSERT_EQ(err, ENOENT, "unknown_model_rejected");

Can this negative test distinguish the two failure modes?
write_cost_model() returns the open() errno and the write() errno
through the same channel, so its callers cannot tell them apart.  ENOENT
is exactly what open() returns when /sys/fs/cgroup/io.cost.model does
not exist (cgroup2 mounted elsewhere, or the io controller not enabled
in the root cgroup's subtree_control).  The assertion then passes
without the kernel ever having parsed the model name, i.e. it can report
success for the opposite of what it means to prove.

The helper's own comment says it returns "the errno of the failed
write", which does not match the open() path.  Distinguishing the two
(e.g. a separate return code for open failure, or asserting the file is
openable first) would make the negative test meaningful.

> diff --git a/tools/testing/selftests/bpf/progs/iocost_model.c b/tools/testing/selftests/bpf/progs/iocost_model.c
> new file mode 100644
> index 000000000000..f8ad417b3584
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iocost_model.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Example iocost cost model: the builtin linear HDD formula with all
> + * costs doubled.
> + *
> + * The constants mirror what calc_lcoefs() derives from the AUTOP_HDD
> + * defaults (rbps=174019176 rseqiops=41708 rrandiops=370, w-side
> + * analog) in vtime units where 1s == 2^37, expressed with the same
> + * round-up divisions so they cannot drift from the kernel.  A device
> + * bound to this model through io.cost.model charges exactly twice the
> + * builtin model under the same workload, which makes it a convenient
> + * way to verify that accounting goes through the BPF path.

Does this claim hold for arbitrary devices?  "A device bound to this
model through io.cost.model charges exactly twice the builtin model
under the same workload" is only true for a rotational device still on
ctrl=auto.

The builtin model reads ioc->params.lcoefs[], which calc_lcoefs()
derives from autop[ioc_autop_idx()] or from user-supplied io.cost.model
coefficients.  ioc_autop_idx() returns AUTOP_HDD only when
blk_queue_rot(disk->queue); otherwise it returns AUTOP_SSD_QD1 or
AUTOP_SSD_DFL, and when ioc->user_cost_model is set it keeps whatever
the user pinned.

This model hardcodes the AUTOP_HDD i_lcoefs (rbps=174019176 etc., from
block/blk-iocost.c), so on the non-rotational $IOCOST_TEST_DEV that
most testers will use, the ratio to the builtin is arbitrary rather than
2x.  The comment should state the rotational/ctrl=auto precondition (or
the code should not claim exactness).

Secondly, "expressed with the same round-up divisions so they cannot
drift from the kernel" overstates the coupling: only the RU() divisions
mirror calc_lcoefs().  RBPS/RSEQIOPS/RRANDIOPS/WBPS/WSEQIOPS/WRANDIOPS,
IOC_PAGE_SIZE, IOC_SECT_TO_PAGE_SHIFT and LCOEF_RANDIO_PAGES are
hardcoded copies of kernel values and will drift silently if the kernel
changes them.

[ ... ]

> +SEC("struct_ops")
> +u64 BPF_PROG(iocost_2x_calc_cost, u64 opf, u64 nbytes, u64 sector,
> +	     struct blkcg *blkcg, u64 model_flags)
> +{
> +	u64 pages, seek_pages = 0, base, coef_page, randio, cost;
> +
> +	if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_READ) {
> +		base = RSEQIO; coef_page = RPAGE; randio = RRANDIO;
> +	} else if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_WRITE) {
> +		base = WSEQIO; coef_page = WPAGE; randio = WRANDIO;
> +	} else {
> +		/*
> +		 * a fully owning model must price every op; unknown
> +		 * ops are priced as a single page write
> +		 */
> +		base = 0; coef_page = WPAGE; randio = 0;
> +	}
> +
> +	/*
> +	 * mirror the builtin single per-cgroup cursor: the model keeps
> +	 * its own cursor keyed by the blkcg argument
> +	 */

Does the keying match the builtin?  The comment says this mirrors "the
builtin single per-cgroup cursor", but the builtin cursor is not
per-cgroup - it is per blkg, i.e. per (cgroup, device).  The cursor is
'sector_t cursor;' in struct ioc_gq, and ioc_gq is allocated by
blkg_to_iocg(blkg).

cursor_store is keyed by blkcg->css.cgroup only, so if two devices are
bound to this model the same cgroup shares one cursor between them and
every alternating IO looks random.

> +	{
> +		__u64 *cursor, cur;
> +
> +		cursor = bpf_cgrp_storage_get(&cursor_store,
> +					      blkcg->css.cgroup, NULL,
> +					      BPF_LOCAL_STORAGE_GET_F_CREATE);
> +		if (!cursor)
> +			return 2 * (base + RU(nbytes, IOC_PAGE_SIZE) * coef_page);
> +		cur = *cursor;
> +		seek_pages = sector > cur ? sector - cur : cur - sector;
> +		seek_pages >>= IOC_SECT_TO_PAGE_SHIFT;

Does this handle a freshly created cgroup?  bpf_cgrp_storage_get() with
BPF_LOCAL_STORAGE_GET_F_CREATE returns a zero-filled value
(bpf_selem_alloc() allocates with __GFP_ZERO), so a new entry has
cur == 0, which this code treats as a real cursor position rather than
'no previous IO'.

The builtin only computes a seek distance when the cursor is non-zero:

block/blk-iocost.c:calc_vtime_cost_builtin()
  if (iocg->cursor) {
      seek_pages = abs(bio->bi_iter.bi_sector - iocg->cursor);
      seek_pages >>= IOC_SECT_TO_PAGE_SHIFT;
  }

Without this guard, the first bio of every cgroup landing past sector
32768 is priced with randio (368,221,632) instead of seqio (60,266).

> +		if (seek_pages > LCOEF_RANDIO_PAGES)
> +			base = randio;
> +		if (!(model_flags & IOCOST_COST_F_MERGE))
> +			*cursor = sector + RU(nbytes, 512);
> +	}

Does this cursor update match the builtin?  The cursor is advanced for
every non-merge call, including calls the builtin model never accounts
for.

calc_vtime_cost_builtin() returns cost 0 both for
'!bio->bi_iter.bi_size' and for any op that is not REQ_OP_READ or
REQ_OP_WRITE, and ioc_rqos_throttle() then does 'if (!abs_cost) return;'
before 'iocg->cursor = bio_end_sector(bio);'.  So the builtin leaves the
cursor untouched for flush and discard bios.

This model instead writes 'sector + RU(nbytes, 512)' for them.  A
dataless flush from blkdev_issue_flush() is 'bio_init(&bio, bdev, NULL,
0, REQ_OP_WRITE | REQ_PREFLUSH)': bi_sector == 0 and bi_size == 0.
It matches the REQ_OP_WRITE arm, so this line sets *cursor = 0 + 0 = 0.
REQ_OP_FLUSH hits the 'unknown op' arm and does the same.

The next read/write in that cgroup is then compared against cursor 0, so
seek_pages == sector >> 3 and any sector beyond 32768 (16 MiB into the
device) trips 'seek_pages > LCOEF_RANDIO_PAGES' and is priced with
randio instead of seqio.  With the AUTOP_HDD constants in this file that
is RRANDIO = 368,221,632 instead of RSEQIO = 60,266.  For a 4 KiB read,
roughly 113x instead of the 2x the file comment claims.

On an fsync-heavy workload a flush precedes almost every IO, so a cgroup
bound to this model is throttled orders of magnitude harder than under
the builtin model it is supposed to double.  A large discard has the
mirror-image effect: 'sector + RU(nbytes, 512)' jumps the cursor by the
whole discard length, so the following sequential read looks random.

Matching the builtin requires only advancing the cursor for the ops that
were actually priced (READ/WRITE with nbytes != 0).

> +
> +	pages = RU(nbytes, IOC_PAGE_SIZE);
> +	if (!pages)
> +		pages = 1;	/* dataless flush: one page */

Does the page count match the builtin?  The page count rounds up where
the builtin truncates, so the cost is not 2x for bios that are not a
whole number of 4 KiB pages.

The builtin uses:

block/blk-iocost.c:calc_vtime_cost_builtin()
  u64 pages = max_t(u64, bio_sectors(bio) >> IOC_SECT_TO_PAGE_SHIFT, 1);

which is max(nbytes >> 12, 1) - a truncating shift.  RU(nbytes,
IOC_PAGE_SIZE) is a round-up.  For a 6 KiB bio the builtin charges 1
page and this model charges 2, i.e. 2*(base + 2*coef_page) instead of
2*(base + 1*coef_page).  Bios whose size is not a multiple of 4096 are
routine (writeback of a partial tail, and merged bios on the
ioc_rqos_merge() path).

Note the file comment claims the constants use "the same round-up
divisions so they cannot drift from the kernel" - the page count is the
one division the kernel deliberately does not round up.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34481298417

[RFC PATCH v2 3/5] blk-iocost: add iocost_ioc_tick tracepoint for per-period device summary

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The existing iocost tracepoints are state-change driven: vrate_adj
fires only when the adjustment logic runs, inuse_* only on surplus
state transitions, activate/idle only on cgroup state changes.  In a
steady state none of them fire.  The only other way to observe the
controller (period length, vrate, busy level, active cgroup count,
device utilization) is iocost_monitor.py, which reads kernel memory
through drgn and is not usable in most production environments.

Add iocost_ioc_tick, emitted once per period from the tail of
ioc_timer_fn() with the overall controller state: period_us, vrate,
busy_level, active iocg count, usage percentage and running state.
It fires every period the controller runs, including steady states,
plus one final tick before the controller goes idle, which makes
dormancy (e.g. a device saturated entirely by uncharged IO) directly
visible.

At the default period this is a couple of events per second per
device; the cost is zero while the static key is off.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/blk-iocost.c            |  5 +++++
 include/trace/events/iocost.h | 40 +++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 182601ad783f..bccdbd2496d8 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2244,6 +2244,7 @@ static void ioc_timer_fn(struct timer_list *timer)
 	struct ioc_now now;
 	LIST_HEAD(surpluses);
 	int nr_debtors, nr_shortages = 0, nr_lagging = 0;
+	int nr_active = 0;
 	u64 usage_us_sum = 0;
 	u32 ppm_rthr;
 	u32 ppm_wthr;
@@ -2280,6 +2281,8 @@ static void ioc_timer_fn(struct timer_list *timer)
 		u64 vdone, vtime, usage_us;
 		u32 hw_active, hw_inuse;
 
+		nr_active++;
+
 		/*
 		 * Collect unused and wind vtime closer to vnow to prevent
 		 * iocgs from accumulating a large amount of budget.
@@ -2466,6 +2469,8 @@ static void ioc_timer_fn(struct timer_list *timer)
 		ioc_refresh_vrate(ioc, &now);
 	}
 
+	trace_iocost_ioc_tick(ioc, nr_active, usage_us_sum);
+
 	spin_unlock_irq(&ioc->lock);
 }
 
diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
index e772b1bc60d6..2b9ff348a4f5 100644
--- a/include/trace/events/iocost.h
+++ b/include/trace/events/iocost.h
@@ -178,6 +178,46 @@ TRACE_EVENT(iocost_ioc_vrate_adj,
 	)
 );
 
+/*
+ * Periodic per-device summary, emitted once per period from the tail of
+ * ioc_timer_fn().  Unlike the state-change events above, this fires every
+ * period the controller is running, including steady states, and carries
+ * the overall controller state so basic monitoring doesn't require drgn.
+ */
+TRACE_EVENT(iocost_ioc_tick,
+
+	TP_PROTO(struct ioc *ioc, int nr_active, u64 usage_us_sum),
+
+	TP_ARGS(ioc, nr_active, usage_us_sum),
+
+	TP_STRUCT__entry (
+		__string(devname, ioc_name(ioc))
+		__field(u32, period_us)
+		__field(u64, vrate)
+		__field(int, busy_level)
+		__field(int, nr_active)
+		__field(u32, usage_pct)
+		__field(int, running)
+	),
+
+	TP_fast_assign(
+		__assign_str(devname);
+		__entry->period_us = ioc->period_us;
+		__entry->vrate = ioc->vtime_base_rate;
+		__entry->busy_level = ioc->busy_level;
+		__entry->nr_active = nr_active;
+		__entry->usage_pct = ioc->period_us ?
+			div_u64(usage_us_sum * 100, ioc->period_us) : 0;
+		__entry->running = ioc->running;
+	),
+
+	TP_printk("[%s] period=%uus vrate=%llu busy=%d active=%d usage=%u%% running=%d",
+		__get_str(devname), __entry->period_us, __entry->vrate,
+		__entry->busy_level, __entry->nr_active, __entry->usage_pct,
+		__entry->running
+	)
+);
+
 TRACE_EVENT(iocost_iocg_forgive_debt,
 
 	TP_PROTO(struct ioc_gq *iocg, const char *path, struct ioc_now *now,
-- 
2.43.0

Re: [RFC PATCH v2 3/5] blk-iocost: add iocost_ioc_tick tracepoint for per-period device summary

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 8158 bytes --]

> blk-iocost: add iocost_ioc_tick tracepoint for per-period device summary
>
> The existing iocost tracepoints are state-change driven: vrate_adj
> fires only when the adjustment logic runs, inuse_* only on surplus
> state transitions, activate/idle only on cgroup state changes.  In a
> steady state none of them fire.  The only other way to observe the
> controller (period length, vrate, busy level, active cgroup count,
> device utilization) is iocost_monitor.py, which reads kernel memory
> through drgn and is not usable in most production environments.
>
> Add iocost_ioc_tick, emitted once per period from the tail of
> ioc_timer_fn() with the overall controller state: period_us, vrate,
> busy_level, active iocg count, usage percentage and running state.
> It fires every period the controller runs, including steady states,
> plus one final tick before the controller goes idle, which makes
> dormancy (e.g. a device saturated entirely by uncharged IO) directly
> visible.
>
> At the default period this is a couple of events per second per
> device; the cost is zero while the static key is off.

Both halves of this last sentence overstate the case.

"a couple of events per second per device": the default period comes from
ioc_refresh_period_us(), and because the autop[] tables leave QOS_RPPM/QOS_WPPM
at 0, multi is always 2, so period_us = 2 * latency target:

  - AUTOP_HDD:      2 * 250000us = 500ms  ->  2 events/s
  - AUTOP_SSD_QD1:  2 *  25000us =  50ms  -> 20 events/s
  - AUTOP_SSD_DFL:  2 *  25000us =  50ms  -> 20 events/s
  - AUTOP_SSD_FAST: 2 *   5000us =  10ms  -> 100 events/s

Only the rotational default matches "a couple per second"; the SSD defaults -
the common case - are 20-100 events/s per device, 10-50x higher.

"the cost is zero while the static key is off": the nr_active++ added at
block/blk-iocost.c:2284 runs unconditionally on every active iocg in every
period, outside the static key. The cost is negligible, but it is not zero,
and the claim as written invites the reader to believe no work was added to
the timer path.

> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 182601ad783f..bccdbd2496d8 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c

[ ... ]

> @@ -2280,6 +2281,8 @@ static void ioc_timer_fn(struct timer_list *timer)
>  		u64 vdone, vtime, usage_us;
>  		u32 hw_active, hw_inuse;
>
> +		nr_active++;
> +
>  		/*
>  		 * Collect unused and wind vtime closer to vnow to prevent
>  		 * iocgs from accumulating a large amount of budget.

[ ... ]

> @@ -2466,6 +2469,8 @@ static void ioc_timer_fn(struct timer_list *timer)
>  		ioc_refresh_vrate(ioc, &now);
>  	}
>
> +	trace_iocost_ioc_tick(ioc, nr_active, usage_us_sum);
> +
>  	spin_unlock_irq(&ioc->lock);
>  }

Is the tracepoint emitted at the right point in the function? It is called
after the period-rollover bookkeeping, so on the one tick the changelog
specifically advertises, the reported state has already been wiped.

The changelog says: "plus one final tick before the controller goes idle,
which makes dormancy (e.g. a device saturated entirely by uncharged IO)
directly visible." That final tick is taken through the else branch at
block/blk-iocost.c:2464-2467, which executes this:

	} else {
		ioc->busy_level = 0;
		ioc->vtime_err = 0;
		ioc->running = IOC_IDLE;
	}

	ioc_refresh_vrate(ioc, &now);

immediately before the trace call. The event therefore always reports busy=0
for the go-idle tick, even though the busy_level computed earlier in this
very period (from rq_wait_pct / missed_ppm, lines 2401-2420) is the
interesting number in the named scenario: a device saturated by uncharged IO
drives rq_wait_pct up, so busy_level was incremented and ioc_adjust_base_vrate()
already slowed vrate down - and none of that is visible in the emitted record,
which reads active=0 usage=0% busy=0 running=0.

Same ordering issue for running: it is set to IOC_IDLE at line 2466 before the
trace, so the field describes the state entered *after* the period rather than
the state the period ran in. Placing the trace_iocost_ioc_tick() call before
the `if (ioc->running != IOC_STOP)` block (or capturing busy_level/running into
locals right after ioc_adjust_base_vrate()) would emit the period's own state.

> diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
> index e772b1bc60d6..2b9ff348a4f5 100644
> --- a/include/trace/events/iocost.h
> +++ b/include/trace/events/iocost.h

[ ... ]

> +/*
> + * Periodic per-device summary, emitted once per period from the tail of
> + * ioc_timer_fn().  Unlike the state-change events above, this fires every
> + * period the controller is running, including steady states, and carries
> + * the overall controller state so basic monitoring doesn't require drgn.
> + */
> +TRACE_EVENT(iocost_ioc_tick,
> +
> +	TP_PROTO(struct ioc *ioc, int nr_active, u64 usage_us_sum),
> +
> +	TP_ARGS(ioc, nr_active, usage_us_sum),
> +
> +	TP_STRUCT__entry (
> +		__string(devname, ioc_name(ioc))
> +		__field(u32, period_us)
> +		__field(u64, vrate)
> +		__field(int, busy_level)
> +		__field(int, nr_active)
> +		__field(u32, usage_pct)
> +		__field(int, running)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(devname);
> +		__entry->period_us = ioc->period_us;
> +		__entry->vrate = ioc->vtime_base_rate;
> +		__entry->busy_level = ioc->busy_level;
> +		__entry->nr_active = nr_active;
> +		__entry->usage_pct = ioc->period_us ?
> +			div_u64(usage_us_sum * 100, ioc->period_us) : 0;
> +		__entry->running = ioc->running;
> +	),

Does usage_pct normalise against the right divisor? It uses the *nominal*
period length read at the very end of ioc_timer_fn(), which is neither the
interval the usage was accumulated over nor necessarily the period_us that
was in effect during it.

Two concrete problems:

1. Wrong divisor. usage_us_sum is accumulated over the real interval
[ioc->period_at, now.now]. ioc_timer_fn() itself never treats that as equal to
ioc->period_us - the donation loop a few lines above deliberately uses the
measured duration:

    usage_dur = max_t(u64, now.now - ioc->period_at, 1);

and ioc_forgive_debts() (block/blk-iocost.c:2119) does the same for its own
percentage:

    dur = now->now - ioc->dfgv_period_at;
    usage_pct = div64_u64(100 * ioc->dfgv_usage_us_sum, dur);

ioc_start_period() re-arms with a jiffies timer (ioc->timer.expires = jiffies +
usecs_to_jiffies(ioc->period_us)), so a period routinely runs longer than
period_us - which is exactly why the code above measures instead of assuming.
Every microsecond of timer/softirq lateness inflates the reported usage
percentage proportionally, and the consumer cannot correct for it: the
tracepoint is emitted *after* ioc_start_period() has already overwritten
ioc->period_at with now.now, and the event carries neither now.now nor the old
period_at, so the actual duration is unrecoverable from the trace.

2. Stale-vs-new period_us. ioc_refresh_params(ioc, false) runs before the
tracepoint (block/blk-iocost.c:2452) and, when the autop index changes, calls
ioc_refresh_period_us() which rewrites ioc->period_us (e.g. AUTOP_SSD_DFL
50000us -> AUTOP_SSD_FAST 10000us). On such a period the event divides the old
period's usage by the new period length, a 5x error, and also reports the
freshly reset vtime_base_rate (ioc_refresh_params_disk() sets it back to
VTIME_PER_USEC).

Using the measured duration - captured before ioc_start_period() clobbers
ioc->period_at - would make the field mean what the changelog says it means
("device utilization").

Aside on the same line: the `ioc->period_us ?` guard is unreachable.
ioc_refresh_period_us() ends with `period_us = clamp_t(u32, period_us,
MIN_PERIOD, MAX_PERIOD)` (MIN_PERIOD = USEC_PER_MSEC) and runs from
blk_iocost_init()'s ioc_refresh_params_disk(ioc, true, disk) before
timer_setup() at block/blk-iocost.c:3001, so ioc->period_us is never 0 when
ioc_timer_fn() executes.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34481298417

[RFC PATCH v2 4/5] selftests/bpf: add multi-stream sequentiality example model

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Add a second example cost model which replaces the builtin
single-cursor sequentiality heuristic with a per-cgroup table of
stream slots: an IO is sequential iff its sector matches the expected
next sector of any tracked stream.  Interleaved sequential readers in
one cgroup keep their own slots instead of ping-ponging a single
cursor, and random IO inside a hot window rarely matches a moving
expectation.  Merged bios skip the base cost but still advance the
matched stream position, so a merge at the expected sector does not
make the following new IO look random.

Stream state lives in a CGRP_STORAGE map keyed by the cgroup of the
blkcg argument, following the cgroup lifetime; there is no
fixed-size registry to exhaust.

Measured (QEMU, virtio-blk with the HDD profile, 4k IOs, w=1000):
two sequential readers in one cgroup are priced 1961us/op by the
builtin model (judged random) and 23us/op by this model (judged
sequential), the completed IO count rises from 4495 to 207505;
random IO inside an 8M window is priced 24us/op by builtin
(undercharge) and 2607us/op by this model; single-stream sequential
and whole-disk random pricing are unchanged.
---
 tools/testing/selftests/bpf/progs/iocost_ms.c | 136 ++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/iocost_ms.c

diff --git a/tools/testing/selftests/bpf/progs/iocost_ms.c b/tools/testing/selftests/bpf/progs/iocost_ms.c
new file mode 100644
index 000000000000..3cc57c03d3c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iocost_ms.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example multi-stream sequentiality detection cost model.
+ *
+ * The builtin model keeps a single cursor per cgroup, so two
+ * interleaved sequential readers in one cgroup are all priced random
+ * (measured 89x overcharge, 12.9x throughput collapse), while random
+ * IO inside a hot window smaller than the 16MB seek threshold is
+ * priced sequential (measured 107x undercharge).  This model replaces
+ * the single cursor with a per-cgroup table of stream slots: an IO is
+ * sequential iff its sector matches the expected next sector of any
+ * tracked stream.  Interleaved streams keep their own slots, and
+ * windowed random IO rarely matches a moving expectation.
+ *
+ * Stream state lives in a CGRP_STORAGE map, so it is created and
+ * freed with the cgroup.  The model implements the full builtin
+ * linear formula itself, including flush pricing.
+ */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* VTIME_PER_SEC comes from vmlinux.h (a BTF enum constant) */
+#define IOC_PAGE_SIZE		4096
+#define IOC_SECT_TO_PAGE_SHIFT	3	/* 512B sectors to 4kB pages */
+#define LCOEF_RANDIO_PAGES	4096	/* 16MB seek threshold */
+#define IOCOST_COST_F_MERGE	(1ULL << 0)	/* not in BTF: a plain macro */
+#define IOCOST_REQ_OP_MASK	0xff		/* REQ_OP_MASK, not in BTF */
+
+/*
+ * DIV64_U64_ROUND_UP / DIV_ROUND_UP_ULL equivalents, folded at
+ * compile time
+ */
+#define RU(x, y)		((x) / (y) + (((x) % (y)) ? 1 : 0))
+
+#define RBPS	174019176ULL
+#define RSEQIOPS	41708ULL
+#define RRANDIOPS	370ULL
+#define WBPS	178075866ULL
+#define WSEQIOPS	42705ULL
+#define WRANDIOPS	378ULL
+
+#define RPAGE	(RU(VTIME_PER_SEC, RU(RBPS, IOC_PAGE_SIZE)))
+#define RSEQIO	(RU(VTIME_PER_SEC, RSEQIOPS) - RPAGE)
+#define RRANDIO	(RU(VTIME_PER_SEC, RRANDIOPS) - RPAGE)
+#define WPAGE	(RU(VTIME_PER_SEC, RU(WBPS, IOC_PAGE_SIZE)))
+#define WSEQIO	(RU(VTIME_PER_SEC, WSEQIOPS) - WPAGE)
+#define WRANDIO	(RU(VTIME_PER_SEC, WRANDIOPS) - WPAGE)
+
+#define NSLOTS	4
+
+struct streams {
+	__u64 expected[NSLOTS];	/* next expected sector, per stream */
+	__u64 stamp[NSLOTS];	/* LRU stamp, 0 = empty */
+};
+
+/*
+ * per-cgroup stream table: keyed by the cgroup, freed with it
+ */
+struct {
+	__uint(type, BPF_MAP_TYPE_CGRP_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct streams);
+} stream_tab SEC(".maps");
+
+SEC("struct_ops")
+u64 BPF_PROG(iocost_ms_calc_cost, u64 opf, u64 nbytes, u64 sector,
+	     struct blkcg *blkcg, u64 model_flags)
+{
+	struct streams *s;
+	u64 pages, base, coef_page, randio, advance, now;
+	u32 i, victim = 0, found = 0xFFFFFFFF;
+
+	if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_READ) {
+		base = RSEQIO; coef_page = RPAGE; randio = RRANDIO;
+	} else if ((opf & IOCOST_REQ_OP_MASK) == REQ_OP_WRITE) {
+		base = WSEQIO; coef_page = WPAGE; randio = WRANDIO;
+	} else {
+		/*
+		 * a fully owning model must price every op; unknown
+		 * ops are priced as a single page write
+		 */
+		base = 0; coef_page = WPAGE; randio = 0;
+	}
+	advance = RU(nbytes, 512);	/* sectors */
+
+	s = bpf_cgrp_storage_get(&stream_tab, blkcg->css.cgroup, NULL,
+				 BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!s)
+		return base + RU(nbytes, IOC_PAGE_SIZE) * coef_page;
+
+	now = bpf_ktime_get_ns();
+	for (i = 0; i < NSLOTS; i++) {
+		if (s->expected[i] == sector && s->stamp[i]) {
+			found = i;
+			break;
+		}
+	}
+	if (found != 0xFFFFFFFF) {
+		/* sequential: keep the seq base from the op branch */
+		s->expected[found] = sector + advance;
+		s->stamp[found] = now;
+	} else {
+		base = randio;
+		for (i = 1; i < NSLOTS; i++) {
+			if (s->stamp[i] < s->stamp[victim])
+				victim = i;
+		}
+		s->expected[victim] = sector + advance;
+		s->stamp[victim] = now;
+	}
+
+	pages = RU(nbytes, IOC_PAGE_SIZE);
+	if (!pages)
+		pages = 1;	/* dataless flush: one page */
+	if (model_flags & IOCOST_COST_F_MERGE) {
+		/*
+		 * merged bios skip the base cost but still advance
+		 * the stream position above, so a merge at the
+		 * expected sector does not make the following new IO
+		 * look random
+		 */
+		base = 0;
+	}
+
+	return base + pages * coef_page;
+}
+
+SEC(".struct_ops")
+struct iocost_model_ops iocost_ms = {
+	.calc_cost = (void *)iocost_ms_calc_cost,
+	.name = "iocost_ms",
+};
+
+char LICENSE[] SEC("license") = "GPL";
-- 
2.43.0

[RFC PATCH v2 5/5] docs: cgroup-v2: document io.cost model=<name> binding

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Document the named-model binding of io.cost.model in the
io.cost.model section of the cgroup v2 documentation: the binding
and restore syntax, the unknown-name rejection, the full-replacement
semantics of a bound model and the unregister lifetime.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 Documentation/admin-guide/cgroup-v2.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 8d2603751c51..a6910ad7c2d0 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2124,6 +2124,17 @@ IO Interface Files
 	parameters are written to, "ctrl" become "user" and the
 	automatic changes are disabled.
 
+	When CONFIG_BLK_CGROUP_IOCOST_BPF is enabled, "model" also accepts
+	the name of a registered iocost_model_ops BPF struct_ops model:
+	"model=<name>" binds the model to the device and the model fully
+	replaces the builtin linear pricing, for every operation including
+	flushes; "model=linear" (or "ctrl=auto/user") restores the builtin
+	model.  Writing an unknown name fails with ENOENT.  Unregistering
+	a model removes its name; devices already bound keep using it
+	until switched back to the builtin model, following the lifetime
+	model of TCP congestion control.  See include/linux/blk-iocost.h
+	for the model interface.
+
 	When "model" is "linear", the following model parameters are
 	defined.
 
-- 
2.43.0

Re: [RFC PATCH v2 0/5] blk-iocost: BPF struct_ops cost model

[Tao Cui]

在 2026/9/10 20:58, Tao Cui 写道:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> This is v2 of the RFC.  It incorporates the feedback from the first
> review round: the attachment model, pricing ownership and per-cgroup
> state handling have all been reworked, and the interface,
> registration, dispatch and configuration changes are now folded
> into a single patch.  Thanks for the detailed review.
> 

Thanks for running the automated review.

I've gone through the reports. Some point out real issues or places that
can be improved, while others need a closer look. I'll sort through
them and address the valid ones in the next revision, including the
documentation updates where appropriate.

Thanks.

> Why a pluggable model at all
> ----------------------------
> 
> When iocost landed in 2019, its commit message already promised that
> "a later patch will also allow using bpf progs for cost models", and
> the code has carried the split for it ever since: calc_vtime_cost()
> is a dispatcher whose only implementation is calc_vtime_cost_builtin().
> Seven years later the builtin linear model is still the only one.
> This series fills that slot, following the TCP congestion control
> model registration pattern: builtin algorithms remain the default
> while new ones can be prototyped in BPF.
> 
> The measured problems
> ---------------------
> 
> The builtin model prices each IO with a binary sequential/random base
> picked by a single per-cgroup cursor and a 16MB seek threshold, plus
> a per-page cost.  On a virtio-blk device with the HDD autop profile,
> a 4k IO costs ~24us when judged sequential and ~2.7ms when judged
> random, a 112x spread, so a wrong judgement becomes a wrong price.
> Three classes of mispricing, all measured:
> 
>  1. Heuristic rigidity.  Two legitimate sequential readers in one
>     cgroup (a database with multiple tablespaces, a threaded backup)
>     ping-pong the single cursor and are all priced random: a
>     measured 89x overcharge collapses throughput under the same
>     weight.
>     Random IO within a hot window smaller than the 16MB threshold is
>     priced sequential: measured 107x undercharge, an accounting
>     escape for hotspot workloads.  No setting of the six builtin
>     parameters seems able to fix this: telling the streams apart
>     requires per-IO state tracking, which looks like logic rather
>     than coefficients.
> 
>  2. Device nonlinearity.  SLC-cache phases, SMR band placement and
>     shared controllers (multiple NVMe namespaces multiplexing one
>     device) make the real cost of an identical IO vary by an order of
>     magnitude over time or across namespaces.  A static
>     6-parameter linear model has no way to express that.
> 
>  3. Unpriced operations.  Flush and zone append fall through to a
>     cost of zero and bypass throttling entirely (fixed in a separate
>     series already posted), but the same pattern extends to device
>     quirks the builtin model was never taught.
> 
> Mispricing feeds directly into the control loop: vtime budgets,
> surplus donation and the vrate feedback all consume the model's
> output, so a wrong model can skew the whole controller.
> 
> How
> ---
> 
> A bound BPF model fully owns pricing for every IO on the device:
> both the bio charging path and the request-level sizing path consult
> it, there is no per-IO or per-path fallback to the builtin formula,
> and the model prices every operation including flushes.  The builtin
> cursor is not exposed; a model is expected to track its own stream
> state.
> 
>     u64 calc_cost(u64 opf, u64 nbytes, sector_t sector,
> 		  struct blkcg *blkcg, u64 model_flags)
> 
> opf is the full bio->bi_opf (the operation must be extracted with a
> mask, and the REQ_* flag bits, including PREFLUSH/FUA, are part of
> it); model_flags carries iocost-specific metadata which is not
> part of bio->bi_opf, such as whether the cost calculation is for
> a merged request; the return value is vtime, clamped to 1 second
> of device time per IO.  blkcg is passed so the model can key
> per-cgroup state; state stored in BPF_MAP_TYPE_CGRP_STORAGE
> follows the cgroup lifetime, and optional blkcg_online()/
> blkcg_offline() callbacks mirror the css lifecycle for models
> which want eager setup or teardown.
> 
> The registration and binding model follows the TCP congestion
> control model registration pattern: registering a struct_ops makes
> the model available by its name, while io.cost.model binds one
> registered model to a device with "model=<name>" and restores the
> builtin model with "model=linear".  Unregistering a model removes
> it from the registry so it can no longer be selected by name;
> devices already using the model keep using it until they are
> switched back to the builtin model, at which point the reference
> is released.  Sleepable models are rejected at
> verification, since calc_cost() runs under RCU read lock.
> Patch overview:
> 
>  1/5: the BPF struct_ops cost model support: Kconfig, ops
>       definition, name registry, registration, io.cost.model
>       binding, unified dispatch and verifier checks
>  2/5: selftest with the 2x example model (the full builtin linear
>       HDD formula at double cost) plus a runner and the selftest
>       kernel config entries
>  3/5: add an iocost_ioc_tick tracepoint emitting the per-period
>       controller state, so model quality can be evaluated without
>       drgn (existing events are state-change driven and silent in
>       steady state)
>  4/5: a second example model which replaces the single-cursor
>       sequentiality heuristic with per-cgroup multi-stream detection
>       keyed by the cgroup, the first consumer of the state interface
>  5/5: document the model=<name> binding in cgroup-v2.rst
> 
> Does it work
> ------------
> 
> Mechanism, verified functionally (QEMU, virtio-blk with the HDD
> profile, same 8s sequential-read workload from a 1%-weight cgroup,
> builtin vs the 2x example model):
> 
>  - per-IO charge: 2882us -> 5722us, a factor of 1.985x; the
>    completed IO count halves and total cost.usage is conserved,
>    i.e. the model output drives both charging and budgeting
>  - the same ratio held across four hosts and 4k/64k/1M block sizes
>    in the v1 measurements (2.00-2.03x on flash-backed hosts, within
>    2% of 2x on a real HDD behind a loaded host); the charging
>    measurement is consistent with the v1 mechanism test, while v2
>    additionally dispatches the request sizing path through the
>    model
>  - edge cases: binding an unknown model name fails with ENOENT;
>    unregistering a bound model leaves the device correctly priced
>    until it is switched back to the builtin model; the readback
>    shows the bound model name; the selftest runner checks the
>    write error and errno of every step, including the restoration
> 
> Payoff, demonstrated with the multi-stream example model (4/5) on
> the same setup, 4k IOs at weight 1000, builtin vs the model:
> 
>  - two sequential readers in one cgroup: priced 1961us/op by builtin
>    (both judged random by the single cursor) and 23us/op by the
>    model (each stream keeps its own slot); the completed IO count
>    rises by two orders of magnitude
>  - random IO inside an 8M window: priced 24us/op by builtin
>    (undercharge, an accounting escape) and 2607us/op by the model
>  - single-stream sequential and whole-disk random pricing are
>    unchanged, so the model fixes both directions of mispricing
>    without introducing a new one
> 
> Non-interference, measured on enterprise NVMe: no measurable
> overhead when the BPF model is not attached.
> 
> ---
> Changes in v2:
> - struct_ops models are now registered by name and bound per
>   device through io.cost.model; the previous ctrl=bpf selection
>   mechanism, the system-wide single-instance limit and its mutex
>   are gone
> - a bound model fully owns pricing: the return-0 delegation to the
>   builtin formula is gone, the request-level sizing path dispatches
>   to the model too, and the builtin cursor is no longer exposed
> - iocg_id is replaced by the blkcg kptr; per-cgroup state uses
>   cgroup storage with its lifetime, plus optional
>   blkcg_online/offline callbacks
> - the full bio->bi_opf including PREFLUSH/FUA is preserved in
>   opf, while model_flags carries iocost-specific metadata
> - sleepable models are rejected in .check_member
> - Kconfig depends on DEBUG_INFO_BTF
> - tracepoint renamed to iocost_ioc_tick; the example model fixes
>   the merged-bio stream advancement and the map exhaustion
>   limitation (cgroup storage), the selftest checks real write
>   errors and the selftest kernel config carries the new options
> - the interface, registration, dispatch and configuration changes
>   are folded into one patch
> 
> Tao Cui (5):
>   blk-iocost: add BPF struct_ops cost model support
>   selftests/bpf: add iocost cost model test
>   blk-iocost: add iocost_ioc_tick tracepoint for per-period device
>     summary
>   selftests/bpf: add multi-stream sequentiality example model
>   docs: cgroup-v2: document io.cost model=<name> binding
> 
>  Documentation/admin-guide/cgroup-v2.rst       |  11 +
>  block/Kconfig                                 |   9 +
>  block/Makefile                                |   1 +
>  block/blk-cgroup.c                            |   3 +
>  block/blk-iocost-bpf.c                        | 252 ++++++++++++++++++
>  block/blk-iocost.c                            | 138 +++++++++-
>  include/linux/blk-iocost.h                    |  82 ++++++
>  include/trace/events/iocost.h                 |  40 +++
>  tools/testing/selftests/bpf/config            |   2 +
>  .../selftests/bpf/prog_tests/iocost_model.c   | 194 ++++++++++++++
>  .../selftests/bpf/progs/iocost_model.c        | 117 ++++++++
>  tools/testing/selftests/bpf/progs/iocost_ms.c | 137 ++++++++++
>  12 files changed, 979 insertions(+), 7 deletions(-)
>  create mode 100644 block/blk-iocost-bpf.c
>  create mode 100644 include/linux/blk-iocost.h
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/iocost_model.c
>  create mode 100644 tools/testing/selftests/bpf/progs/iocost_model.c
>  create mode 100644 tools/testing/selftests/bpf/progs/iocost_ms.c
>