[RFC PATCH v7 0/4] blk-iocost: BPF struct_ops cost model

[RFC PATCH v7 0/4] blk-iocost: BPF struct_ops cost model

[Tao Cui]

This is v7 of the RFC.  The attachment model is reworked: the
struct_ops instance is now per-device, following the hid_bpf_ops
pattern.  The name registry, the binding through io.cost.model
writes, the lifecycle list and the refcounting are gone.

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: 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, and 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
---

Attachment follows the hid_bpf_ops model: the target device is set
in the dev member of the struct_ops from userspace before load, as
a major:minor in the usual userspace encoding (decoded with
new_decode_dev() on the kernel side); loading attaches the model to
that device and switches it away from the builtin linear model,
detaching the struct_ops restores the builtin model, and the
struct_ops core owns the program lifetime.  There is no registry
and no bound-state bookkeeping; attaching a second model to a device
with one already attached fails with -EBUSY.

A bound model fully owns pricing for every IO on the device: it is
called from the bio charging path and prices every operation
including flushes.  The completion-time request sizing which feeds
the latency met/missed accounting uses the transfer cost
coefficients the struct_ops carries (vtime per page for reads and
writes), so the builtin latency tracking and vrate adjustment
follow the model's pricing while it is attached; extending the
model to the QoS side itself is left for a later interface.  The
builtin cursor is not exposed; a model is expected to track its own
stream state.  Model state keyed by the blkcg alone is shared
across every device the model is attached to, unlike the builtin
cursor which is per (cgroup, device); iocg_init()/iocg_free()
callbacks, invoked from the iocg policy init and free paths with
the same per (cgroup, device) lifetime, let models manage their own
per-cgroup state.

While a model is attached, "model" reads back "bpf" and "ctrl"
keeps describing the builtin coefficients, which are inert then:
coefficient writes are stored and take effect again after detach,
and the automatic profile stepping does not switch profiles.
Writing "ctrl=bpf" or "model=bpf" is accepted as a no-op so a saved
configuration still parses; re-attaching the model requires loading
the struct_ops again, not writing to this file.

    u64 calc_cost(struct bio *bio, u64 model_flags)

The model reads whatever it needs from the bio itself: the
operation flags (the operation must be extracted with a mask, and
the REQ_* flag bits, including PREFLUSH/FUA, are part of it), the
size, the start sector and the issuing cgroup through
bio->bi_blkg.  model_flags carries iocost-specific metadata which
is not a property of the bio, 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.  Per-cgroup state can be stored in
BPF_MAP_TYPE_CGRP_STORAGE keyed by the cgroup of bi_blkg; it
follows the cgroup lifetime.  Sleepable models are rejected at
verification, since calc_cost() runs under RCU read lock.

Patch overview:

 1/4: the BPF struct_ops cost model support: Kconfig, ops
      definition, per-device attachment, unified dispatch and
      verifier checks
 2/4: selftests with two example models (the full builtin linear
      HDD formula at double cost, and a multi-stream sequentiality
      model) plus a runner and the selftest kernel config entries
 3/4: 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/4: document the attachment in cgroup-v2.rst

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

Mechanism, verified functionally on a virtio-blk device (HDD
profile, 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
 - attachment: model=bpf readback while attached (ctrl keeps
   describing the inert builtin coefficients), EBUSY for a second
   model on the same device, a second device attaches an independent
   model, detaching restores the builtin readback
 - edge cases: binding an unknown device fails and nothing is
   applied; the selftest runner checks the write error and errno
   of every step, including the restoration

Workload-shape verification (same setup, 4k IOs at weight 1000,
builtin vs the 2x example model):

 - flush-heavy workload (read/write/fsync alternating): priced
   1.99x the builtin, i.e. flushes no longer reset the cursor
   and misjudge the following IO as random
 - non-page-multiple IO (6 KiB): priced ~2x, matching the
   builtin's truncating page count
 - first IO from a high LBA (past 16 MiB): priced 2.01x, i.e.
   a fresh cgroup's zero cursor no longer misjudges the first IO
   as random

Payoff, demonstrated with the multi-stream example model (2/4) 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 v7:
- the attachment model is reworked along the lines of the v1 review
  feedback: the struct_ops instance is per-device following
  hid_bpf_ops, with the device in the dev member set from
  userspace before load, binding in .reg, unbinding in .unreg and
  the struct_ops core owning the lifetime; the name registry, the
  model-name handling in io.cost.model writes, the lifecycle list
  and the refcounting are removed
- calc_cost() receives the bio itself and reads the operation
  flags, size, sector and issuing cgroup from it; only the merge
  indicator stays in the separate flags argument
- the cgroup callbacks are bound to the iocg policy init/free
  paths instead of the blkcg css, giving the model the builtin
  cursor's per (cgroup, device) lifetime and dropping the global
  mutex from the cgroup online/offline paths
- the struct_ops carries the transfer cost coefficients (vtime per
  page for reads and writes), used by the completion-time request
  sizing so the builtin latency tracking and vrate adjustment
  follow the model's pricing while it is attached
- the tick tracepoint is emitted at the end of the period it
  reports, before the refresh, and the temporary snapshots are
  dropped
- the two selftest patches are combined

Link: https://lore.kernel.org/r/20260908100143.47598-1-cui.tao@linux.dev # v1
Link: https://lore.kernel.org/r/20260910125817.223354-1-cui.tao@linux.dev # v2
Link: https://lore.kernel.org/r/20260914073356.791518-1-cui.tao@linux.dev # v3
Link: https://lore.kernel.org/r/20260916072302.1068871-1-cui.tao@linux.dev # v4
Link: https://lore.kernel.org/r/20260918031751.1255420-1-cui.tao@linux.dev # v5
Link: https://lore.kernel.org/r/20260918055001.1273840-1-cui.tao@linux.dev # v6

Tao Cui (4):
  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
  docs: cgroup-v2: document the iocost BPF cost model attachment

 Documentation/admin-guide/cgroup-v2.rst       |  17 ++
 block/Kconfig                                 |  10 +
 block/Makefile                                |   1 +
 block/blk-iocost-bpf.c                        | 165 ++++++++++
 block/blk-iocost.c                            | 283 +++++++++++++++++-
 include/linux/blk-iocost.h                    |  96 ++++++
 include/trace/events/iocost.h                 |  45 +++
 tools/testing/selftests/bpf/config            |   2 +
 .../selftests/bpf/prog_tests/iocost_model.c   | 180 +++++++++++
 .../selftests/bpf/progs/iocost_model.c        | 138 +++++++++
 tools/testing/selftests/bpf/progs/iocost_ms.c | 159 ++++++++++
 11 files changed, 1092 insertions(+), 4 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 v7 1/4] blk-iocost: add BPF struct_ops cost model support

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Add the iocost_model_ops struct_ops.  Attachment follows the
hid_bpf_ops model: the struct_ops instance is per-device, the target
device is set in the dev member from userspace before load, .reg
attaches the model to that device and switches it away from the
builtin linear model, and .unreg detaches it and restores the builtin
model.  The struct_ops core owns the program lifetime, so there is no
name registry and no bound-state bookkeeping.

calc_cost() receives the bio itself and reads whatever it needs
from it (operation flags, size, sector, the issuing cgroup); the
merge indicator stays in the separate flags
argument as it is not a property of the bio.  It is called from the
bio charging path, so the model owns pricing for every IO on the
device.  The completion-time request sizing uses the transfer cost
coefficients carried in the struct_ops (vtime per page for reads and
writes) while a model is attached, so the builtin latency tracking
and vrate adjustment follow the model's pricing; letting a model take
over the QoS side is left for a later extension.

Attaching to a partition's device number is rejected: the model
prices the whole queue.  Enabling iocost implicitly through the
attach disables wbt, matching io.cost.qos.

The cgroup callbacks are bound to the iocg policy init/free paths,
one (cgroup, device) pair per invocation, matching the builtin
cursor's lifetime, instead of the blkcg css lifecycle, which also
drops the mutex from the cgroup online/offline paths.

calc_cost() runs under RCU read lock; sleepable programs are
rejected in .check_member.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/Kconfig              |  10 ++
 block/Makefile             |   1 +
 block/blk-iocost-bpf.c     | 171 +++++++++++++++++++++++
 block/blk-iocost.c         | 275 ++++++++++++++++++++++++++++++++++++-
 include/linux/blk-iocost.h | 103 ++++++++++++++
 5 files changed, 556 insertions(+), 4 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..1cafc1bd0dda 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -231,4 +231,14 @@ 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 the device it is attached
+	 to.  The struct_ops is attached per device, following the
+	 hid_bpf_ops 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-iocost-bpf.c b/block/blk-iocost-bpf.c
new file mode 100644
index 000000000000..2306c5fd6d8d
--- /dev/null
+++ b/block/blk-iocost-bpf.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * blk-iocost: BPF struct_ops plumbing for pluggable cost models.
+ *
+ * Registers the "iocost_model_ops" struct_ops type.  Attachment is
+ * per-device and follows the hid_bpf_ops model: the target device is
+ * set in the ops from userspace before load, .reg attaches the model
+ * to that device and switches it away from the builtin linear model,
+ * .unreg detaches it and restores the builtin model, and the struct_ops
+ * core owns the program lifetime.  There is no name registry and no
+ * separate bound-state bookkeeping.
+ */
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/bpf.h>
+#include <linux/bpf_verifier.h>
+#include <linux/btf.h>
+#include <linux/blk-iocost.h>
+
+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);
+}
+
+/*
+ * No iocost-specific helpers; bpf_base_func_proto already covers the
+ * cgroup storage helpers under CONFIG_CGROUPS.
+ */
+static const struct bpf_func_proto *
+bpf_iocost_get_func_proto(enum bpf_func_id func_id,
+			  const struct bpf_prog *prog)
+{
+	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, bdev_file):
+		/*
+		 * kernel-private: the open bdev file pinning the queue;
+		 * reject a userspace value instead of copying it
+		 */
+		if (uops->bdev_file)
+			return -EINVAL;
+		ops->bdev_file = NULL;
+		return 1;
+	case offsetof(struct iocost_model_ops, q):
+		/* kernel-private: the queue of the attached device */
+		if (uops->q)
+			return -EINVAL;
+		ops->q = NULL;
+		return 1;
+	case offsetof(struct iocost_model_ops, dev):
+		/*
+		 * copy it and return 1 to indicate that the member is
+		 * handled here, or the verifier rejects the map if the
+		 * userspace value is nonzero
+		 */
+		ops->dev = uops->dev;
+		return 1;
+	case offsetof(struct iocost_model_ops, read_vtime_per_page):
+		ops->read_vtime_per_page = uops->read_vtime_per_page;
+		return 1;
+	case offsetof(struct iocost_model_ops, write_vtime_per_page):
+		ops->write_vtime_per_page = uops->write_vtime_per_page;
+		return 1;
+	}
+
+	return 0;
+}
+
+/*
+ * kvalue is zeroed at map allocation and function members are only
+ * written when the BPF side provides a prog, so a model which did
+ * not implement calc_cost leaves it NULL.  The dispatch would call
+ * it on every bio, so reject it here.
+ */
+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;
+
+	if (!ops->dev)
+		return -EINVAL;
+
+	return ioc_bpf_attach(ops);
+}
+
+static void bpf_iocost_unreg(void *kdata, struct bpf_link *link)
+{
+	ioc_bpf_unreg(kdata);
+}
+
+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(struct bio *bio, u64 flags)
+{
+	return 0;
+}
+
+static void bpf_iocost_iocg_init_stub(struct blkcg *blkcg,
+				      struct request_queue *q)
+{ }
+static void bpf_iocost_iocg_free_stub(struct blkcg *blkcg,
+				      struct request_queue *q)
+{ }
+
+static struct iocost_model_ops __bpf_ops_iocost_model_ops = {
+	.calc_cost = bpf_iocost_calc_cost_stub,
+	.iocg_init = bpf_iocost_iocg_init_stub,
+	.iocg_free = bpf_iocost_iocg_free_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..21e4f8cbd9f2 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
+	/* attached BPF cost model, NULL = builtin linear model */
+	const struct iocost_model_ops	__rcu *model;
+#endif
 };
 
 struct iocg_pcpu_stat {
@@ -803,8 +809,16 @@ static int ioc_autop_idx(struct ioc *ioc, struct gendisk *disk)
 	if (idx < AUTOP_SSD_DFL)
 		return AUTOP_SSD_DFL;
 
-	/* if user is overriding anything, maintain what was there */
-	if (ioc->user_qos_params || ioc->user_cost_model)
+	/* if user is overriding anything, maintain what was there; the
+	 * same while a BPF model is attached: the builtin coefficients
+	 * are inert then, so stepping the profile is pointless
+	 */
+	if (ioc->user_qos_params || ioc->user_cost_model
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	    || rcu_dereference_protected(ioc->model,
+					 lockdep_is_held(&ioc->lock))
+#endif
+	   )
 		return idx;
 
 	/* step up/down based on the vrate */
@@ -2572,7 +2586,19 @@ 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)
 {
 	u64 cost;
-
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
+
+	rcu_read_lock();
+	model = rcu_dereference(iocg->ioc->model);
+	if (model) {
+		cost = model->calc_cost(bio,
+				is_merge ? IOCOST_COST_F_MERGE : 0);
+		rcu_read_unlock();
+		return min(cost, VTIME_PER_SEC);
+	}
+	rcu_read_unlock();
+#endif
 	calc_vtime_cost_builtin(bio, iocg, is_merge, &cost);
 	return cost;
 }
@@ -2594,10 +2620,41 @@ static void calc_size_vtime_cost_builtin(struct request *rq, struct ioc *ioc,
 	}
 }
 
+/*
+ * Called from the request completion path, where no ioc->lock is
+ * held; the model pointer is read under RCU, matching the bio-side
+ * calc_vtime_cost().
+ */
 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;
+
+	rcu_read_lock();
+	model = rcu_dereference(ioc->model);
+	if (model && (req_op(rq) == REQ_OP_READ ||
+		      req_op(rq) == REQ_OP_WRITE)) {
+		unsigned int pages =
+			blk_rq_stats_sectors(rq) >> IOC_SECT_TO_PAGE_SHIFT;
+		u64 coeff = req_op(rq) == REQ_OP_READ ?
+			model->read_vtime_per_page :
+			model->write_vtime_per_page;
+
+		rcu_read_unlock();
+		/* sub-page IO: nothing to transfer-price */
+		if (!pages)
+			return 0;
+		/* zero transfer cost is a legal model; guard the division */
+		if (!coeff)
+			return 0;
+		/* pages * coeff can wrap and dodge the clamp below */
+		if (coeff > VTIME_PER_SEC || pages > VTIME_PER_SEC / coeff)
+			return VTIME_PER_SEC;
+		return min(pages * coeff, VTIME_PER_SEC);
+	}
+	rcu_read_unlock();
+#endif
 	calc_size_vtime_cost_builtin(rq, ioc, &cost);
 	return cost;
 }
@@ -2891,6 +2948,9 @@ static void ioc_rqos_queue_depth_changed(struct rq_qos *rqos)
 static void ioc_rqos_exit(struct rq_qos *rqos)
 {
 	struct ioc *ioc = rqos_to_ioc(rqos);
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
+#endif
 
 	blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iocost);
 
@@ -2900,6 +2960,15 @@ 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
+	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)
+		ioc_bpf_detach((struct iocost_model_ops *)model);
+#endif
 	kfree(ioc);
 }
 
@@ -3022,6 +3091,9 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
 	struct ioc_now now;
 	struct blkcg_gq *tblkg;
 	unsigned long flags;
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
+#endif
 
 	ioc_now(ioc, &now);
 
@@ -3048,6 +3120,19 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
 	spin_lock_irqsave(&ioc->lock, flags);
 	weight_updated(iocg, &now);
 	spin_unlock_irqrestore(&ioc->lock, flags);
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	/*
+	 * the model pointer and the ops behind it are RCU-protected:
+	 * a concurrent detach publishes NULL and the struct_ops image
+	 * survives it by a grace period, so the callback is safe
+	 * inside the read-side critical section
+	 */
+	rcu_read_lock();
+	model = rcu_dereference(ioc->model);
+	if (model && model->iocg_init)
+		model->iocg_init(blkg->blkcg, ioc->rqos.disk->queue);
+	rcu_read_unlock();
+#endif
 }
 
 static void iocg_release(struct rcu_head *rcu)
@@ -3066,8 +3151,18 @@ static void ioc_pd_free(struct blkg_policy_data *pd)
 	struct blkcg_gq *blkg = pd_to_blkg(pd);
 	struct ioc *ioc = iocg->ioc;
 	unsigned long flags;
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
+#endif
 
 	if (ioc) {
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+		rcu_read_lock();
+		model = rcu_dereference(ioc->model);
+		if (model && model->iocg_free)
+			model->iocg_free(blkg->blkcg, ioc->rqos.disk->queue);
+		rcu_read_unlock();
+#endif
 		spin_lock_irqsave(&ioc->lock, flags);
 
 		if (!list_empty(&iocg->active_list)) {
@@ -3433,17 +3528,34 @@ static u64 ioc_cost_model_prfill(struct seq_file *sf,
 	const char *dname = blkg_dev_name(pd->blkg);
 	struct ioc *ioc = pd_to_iocg(pd)->ioc;
 	u64 *u = ioc->params.i_lcoefs;
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	const struct iocost_model_ops *model;
+#endif
 
 	if (!dname)
 		return 0;
 
 	spin_lock_irq(&ioc->lock);
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+	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, ioc->user_cost_model ? "user" : "auto",
+		   model ? "bpf" : "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 +3569,141 @@ static int ioc_cost_model_show(struct seq_file *sf, void *v)
 	return 0;
 }
 
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+/*
+ * Attach a BPF cost model to the device named by ops->dev: resolve the
+ * queue, make sure iocost is on it, and publish the model.  Attaching
+ * switches the device away from the builtin linear model; detaching
+ * restores it.  The struct_ops core holds the program alive for the
+ * whole registered period, so no extra reference is taken on the ops.
+ */
+int ioc_bpf_attach(struct iocost_model_ops *ops)
+{
+	struct block_device *bdev;
+	struct request_queue *q;
+	struct ioc *ioc;
+	const struct iocost_model_ops *old;
+	struct file *bdevf;
+	int ret;
+
+	bdevf = bdev_file_open_by_dev(new_decode_dev(ops->dev),
+				      BLK_OPEN_READ, NULL, NULL);
+	if (IS_ERR(bdevf))
+		return PTR_ERR(bdevf);
+	bdev = file_bdev(bdevf);
+
+	if (bdev_is_partition(bdev)) {
+		fput(bdevf);
+		return -EINVAL;
+	}
+
+	q = bdev->bd_queue;
+	if (!queue_is_mq(q)) {
+		fput(bdevf);
+		return -EOPNOTSUPP;
+	}
+
+	mutex_lock(&q->rq_qos_mutex);
+	ioc = q_to_ioc(q);
+	if (!ioc) {
+		ret = blk_iocost_init(bdev->bd_disk);
+		if (ret) {
+			mutex_unlock(&q->rq_qos_mutex);
+			fput(bdevf);
+			return ret;
+		}
+		ioc = q_to_ioc(q);
+	}
+
+	/*
+	 * Stay under rq_qos_mutex until the model is published:
+	 * ioc_rqos_exit() frees the ioc under this mutex, so holding
+	 * it keeps the ioc alive through the publish below.  The open
+	 * bdev file pins the queue for as long as the model is
+	 * attached; it is released by the .unreg side of the detach.
+	 */
+	spin_lock_irq(&ioc->lock);
+	old = rcu_dereference_protected(ioc->model,
+					lockdep_is_held(&ioc->lock));
+	if (old) {
+		spin_unlock_irq(&ioc->lock);
+		mutex_unlock(&q->rq_qos_mutex);
+		fput(bdevf);
+		return -EBUSY;
+	}
+	if (!ioc->enabled) {
+		/*
+		 * the controller must run for the model to be consulted:
+		 * enable it like io.cost.qos enable=1 does
+		 */
+		blk_stat_enable_accounting(q);
+		blk_queue_flag_set(QUEUE_FLAG_RQ_ALLOC_TIME, q);
+		ioc->enabled = true;
+		ioc_refresh_params(ioc, true);
+	}
+	rcu_assign_pointer(ioc->model, ops);
+	ops->q = q;
+	ops->bdev_file = bdevf;
+	spin_unlock_irq(&ioc->lock);
+	/* match io.cost.qos: running iocost disables wbt */
+	wbt_disable_default(bdev->bd_disk);
+	mutex_unlock(&q->rq_qos_mutex);
+
+	return 0;
+}
+
+/*
+ * Detach a model.  The caller holds q->rq_qos_mutex, which serializes
+ * this against ioc_bpf_attach(), against ioc_rqos_exit() freeing the
+ * ioc, and against a concurrent .unreg, so the ops->q/model clearing
+ * is idempotent.  The bdev file is not released here: it pins the
+ * queue for the .unreg side, which may still be about to lock it.
+ */
+void ioc_bpf_detach(struct iocost_model_ops *ops)
+{
+	struct request_queue *q = ops->q;
+	struct ioc *ioc;
+
+	if (!q)
+		return;
+
+	ioc = q_to_ioc(q);
+	/* pairs with the lockless READ_ONCE() in ioc_bpf_unreg() */
+	WRITE_ONCE(ops->q, NULL);
+
+	if (!ioc)
+		return;
+
+	spin_lock_irq(&ioc->lock);
+	if (rcu_dereference_protected(ioc->model,
+				      lockdep_is_held(&ioc->lock)) == ops)
+		rcu_assign_pointer(ioc->model, NULL);
+	spin_unlock_irq(&ioc->lock);
+}
+
+/*
+ * The .unreg side of the detach: serialize against the queue
+ * teardown, then drop the bdev file pinning the queue.  ops->q is
+ * stable here: only ioc_bpf_detach() clears it, the file pin keeps
+ * the queue alive until it is dropped below, and .unreg runs once.
+ */
+void ioc_bpf_unreg(struct iocost_model_ops *ops)
+{
+	struct request_queue *q = READ_ONCE(ops->q);
+	struct file *bdevf = ops->bdev_file;
+
+	ops->bdev_file = NULL;
+	if (q)
+		mutex_lock(&q->rq_qos_mutex);
+	ioc_bpf_detach(ops);
+	if (q)
+		mutex_unlock(&q->rq_qos_mutex);
+	if (bdevf)
+		fput(bdevf);
+}
+
+#endif
+
 static const match_table_t cost_ctrl_tokens = {
 	{ COST_CTRL,		"ctrl=%s"	},
 	{ COST_MODEL,		"model=%s"	},
@@ -3531,11 +3778,31 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 				user = false;
 			else if (!strcmp(buf, "user"))
 				user = true;
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+			else if (!strcmp(buf, "bpf")) {
+				/*
+				 * readback value while a BPF model is
+				 * attached; attaching is done by loading
+				 * the struct_ops, not through this file
+				 */
+				continue;
+			}
+#endif
 			else
 				goto unlock;
 			continue;
 		case COST_MODEL:
 			match_strlcpy(buf, &args[0], sizeof(buf));
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+			if (!strcmp(buf, "bpf")) {
+				/*
+				 * readback value while a BPF model is
+				 * attached; attaching is done by loading
+				 * the struct_ops, not through this file
+				 */
+				continue;
+			}
+#endif
 			if (strcmp(buf, "linear"))
 				goto unlock;
 			continue;
diff --git a/include/linux/blk-iocost.h b/include/linux/blk-iocost.h
new file mode 100644
index 000000000000..ce120a8d6007
--- /dev/null
+++ b/include/linux/blk-iocost.h
@@ -0,0 +1,103 @@
+/* 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>
+#include <linux/blkdev.h>
+
+#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
+
+struct bio;
+struct blkcg;
+struct request_queue;
+
+/*
+ * Pluggable cost model interface for blk-iocost.
+ *
+ * A BPF struct_ops implementation is attached to one device, identified
+ * by the dev member set from userspace before load, following the
+ * hid_bpf_ops model: attaching the struct_ops switches the device to
+ * the BPF model, detaching it restores the builtin linear model, and
+ * the struct_ops core owns the lifetime of the program.  The model
+ * then owns pricing for every charged IO on the device: it prices all
+ * operations, including flushes, from the bio charging path.
+ *
+ * calc_cost() is called from the IO submission path with RCU read lock
+ * held and must not sleep.  It receives the bio itself so the model
+ * can read whatever it needs (operation flags, size, sector, the
+ * issuing cgroup through bio->bi_blkg).  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 struct_ops also carries the transfer cost coefficients, vtime
+ * per page for reads and writes: while a model is attached, the
+ * builtin latency tracking and vrate adjustment use these instead of
+ * the builtin linear coefficients for the completion-time request
+ * sizing, so the whole controller follows the model's pricing.  Letting
+ * a model take over the QoS side entirely (latency tracking, vrate
+ * control) is left for a later extension.
+ *
+ * The cgroup callbacks are bound to the iocg policy lifetime, one
+ * (cgroup, device) pair per invocation, matching the builtin cursor:
+ * state created in init (or lazily on first use) must be released in
+ * free.
+ */
+
+/*
+ * iocost-specific call metadata for calc_cost()'s model_flags
+ * argument; the merge indicator is not a property of the bio.
+ * An enum so the value is exported through BTF and BPF models can
+ * use it from vmlinux.h.
+ */
+enum {
+	IOCOST_COST_F_MERGE	= 1 << 0,	/* called from merge path */
+};
+
+struct iocost_model_ops {
+	/*
+	 * target device (major:minor), set from userspace before load;
+	 * must stay the first member so userspace can write it through
+	 * the struct_ops map's initial value
+	 */
+	dev_t dev;
+	/* kernel-private: the open bdev file pinning the queue */
+	struct file *bdev_file;
+
+	/* vtime per page, used by the builtin sizing and vrate logic */
+	u64 read_vtime_per_page;
+	u64 write_vtime_per_page;
+
+	u64 (*calc_cost)(struct bio *bio, u64 model_flags);
+	/*
+	 * per-(cgroup, device) lifecycle: both callbacks run inside
+	 * an RCU read-side critical section (see below) and must not
+	 * sleep; IRQs may be enabled or disabled, so per-CPU state
+	 * must not rely on the IRQs-off guarantee.  iocg_init() is
+	 * delivered for
+	 * cgroups which appear on the device while the model is
+	 * attached; cgroups which already exist when the model is
+	 * attached never see an init, so iocg_free() must tolerate
+	 * freeing state it never initialized.  iocg_free() is only
+	 * delivered while the model is attached: detaching does not
+	 * flush state created by iocg_init(), so models must keep
+	 * their per-cgroup state reclaimable by other means
+	 */
+	void (*iocg_init)(struct blkcg *blkcg, struct request_queue *q);
+	void (*iocg_free)(struct blkcg *blkcg, struct request_queue *q);
+
+	/* private: */
+
+	/* queue of the attached device, NULL = not attached */
+	struct request_queue	*q;
+};
+
+int ioc_bpf_attach(struct iocost_model_ops *ops);
+void ioc_bpf_detach(struct iocost_model_ops *ops);
+void ioc_bpf_unreg(struct iocost_model_ops *ops);
+
+#else	/* CONFIG_BLK_CGROUP_IOCOST_BPF */
+
+#endif	/* CONFIG_BLK_CGROUP_IOCOST_BPF */
+#endif	/* _LINUX_BLK_IOCOST_H */
-- 
2.43.0

Re: [RFC PATCH v7 1/4] blk-iocost: add BPF struct_ops cost model support

[bot+bpf-ci]

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

> diff --git a/block/blk-iocost-bpf.c b/block/blk-iocost-bpf.c
> new file mode 100644
> index 000000000000..2306c5fd6d8d
> --- /dev/null
> +++ b/block/blk-iocost-bpf.c

[ ... ]

> +static int bpf_iocost_reg(void *kdata, struct bpf_link *link)
> +{
> +	struct iocost_model_ops *ops = kdata;
> +
> +	if (!ops->dev)
> +		return -EINVAL;
> +
> +	return ioc_bpf_attach(ops);
> +}

Does this guard against multiple registrations of the same struct_ops?

bpf_struct_ops_link_create() allows several links from one BPF_F_LINK map.
bpf_struct_ops_valid_to_reg() only checks the map type, BPF_F_LINK flag,
and the READY state.  The state stays READY after a link is created, so
every bpf_link_create() on the map calls ->reg(st_map->kvalue.data, link)
on the same kdata.

hid_bpf_reg(), which the commit message names as the model, guards this
case:

    /* prevent multiple attach of the same struct_ops */
    if (ops->hdev)
        return -EINVAL;

While the first link is attached, a second link on the same device is
refused by the ioc->model check in ioc_bpf_attach() with -EBUSY.  That
check stops working once the device goes away:

1. link1 = bpf_link_create(map). ioc_bpf_attach() opens f1 =
   bdev_file_open_by_dev(dev) and stores ops->q = q1 and ops->bdev_file =
   f1.
2. The disk is removed, for example a null_blk device deleted through
   configfs. null_del_dev() calls ida_free(&nullb_indexes, nullb->index)
   and then del_gendisk(). __del_gendisk() -> rq_qos_exit() ->
   ioc_rqos_exit() clears ioc->model and calls ioc_bpf_detach(), which does
   WRITE_ONCE(ops->q, NULL) but deliberately leaves ops->bdev_file = f1.
   The old disk and queue stay alive because f1 pins them. __del_gendisk()
   also calls bdev_unhash(), so the old bdev can no longer be looked up by
   dev_t.
3. A new null_blk device is created. It gets the same index, and so the
   same dev_t, because null_add_dev() sets disk->first_minor = nullb->index
   and the index was already freed.
4. link2 = bpf_link_create(map). bpf_iocost_reg() sees ops->dev != 0.
   ioc_bpf_attach() opens f2 on the new disk and finds no model on the new
   ioc. It then runs:
       rcu_assign_pointer(ioc->model, ops);
       ops->q = q;
       ops->bdev_file = bdevf;
   This overwrites f1, and nothing ever releases f1. The dead gendisk,
   request_queue, bdev and the driver module reference held through the
   open file are leaked, so null_blk can no longer be unloaded.
5. Closing link1 then runs bpf_iocost_unreg() -> ioc_bpf_unreg(). It reads
   q = new queue and bdevf = f2, detaches the model from the new device and
   fputs f2. link2 is still alive but its model has been silently removed.
   When link2 is released, ops->q and ops->bdev_file are both NULL and
   nothing happens.

The map's dev cannot be changed after the first update, because
map_update_elem requires state INIT, so dev_t reuse is the trigger.  Every
step can be reached by a user with CAP_BPF, CAP_PERFMON and the ability to
add and remove block devices.

> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 2745bffcd5ee..21e4f8cbd9f2 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c

[ ... ]

> +#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
> +/*
> + * Attach a BPF cost model to the device named by ops->dev: resolve the
> + * queue, make sure iocost is on it, and publish the model.  Attaching
> + * switches the device away from the builtin linear model; detaching
> + * restores it.  The struct_ops core holds the program alive for the
> + * whole registered period, so no extra reference is taken on the ops.
> + */
> +int ioc_bpf_attach(struct iocost_model_ops *ops)
> +{
> +	struct block_device *bdev;
> +	struct request_queue *q;
> +	struct ioc *ioc;
> +	const struct iocost_model_ops *old;
> +	struct file *bdevf;
> +	int ret;
> +
> +	bdevf = bdev_file_open_by_dev(new_decode_dev(ops->dev),
> +				      BLK_OPEN_READ, NULL, NULL);
> +	if (IS_ERR(bdevf))
> +		return PTR_ERR(bdevf);
> +	bdev = file_bdev(bdevf);
> +
> +	if (bdev_is_partition(bdev)) {
> +		fput(bdevf);
> +		return -EINVAL;
> +	}
> +
> +	q = bdev->bd_queue;
> +	if (!queue_is_mq(q)) {
> +		fput(bdevf);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	mutex_lock(&q->rq_qos_mutex);
> +	ioc = q_to_ioc(q);
> +	if (!ioc) {
> +		ret = blk_iocost_init(bdev->bd_disk);

Does this check that the disk is still live?

ioc_bpf_attach() takes q->rq_qos_mutex and, if the queue has no ioc, calls
blk_iocost_init() to add one. It never checks that the disk is still live.
The only liveness check is the one bdev_file_open_by_dev() does under
open_mutex at open time, and __del_gendisk() does not wait for existing
openers.

The blkcg config path handles the same race explicitly in
blkg_conf_open_bdev():

    mutex_lock(&bdev->bd_queue->rq_qos_mutex);
    if (!disk_live(bdev->bd_disk)) {
        blkdev_put_no_open(bdev);
        mutex_unlock(&bdev->bd_queue->rq_qos_mutex);
        return -ENODEV;
    }

Without that check, this interleaving is possible:

1. bpf_iocost_reg() -> ioc_bpf_attach(): bdev_file_open_by_dev() succeeds
   while the disk is live.
2. __del_gendisk() on another CPU: bdev_unhash, GD_DEAD, blk_unregister_queue,
   device_del, blk_mq_freeze_queue_wait, then rq_qos_exit(q). rq_qos_exit()
   unlinks every rqos under rq_qos_mutex and runs ioc_rqos_exit(), so
   q->rq_qos no longer has an ioc.
3. ioc_bpf_attach() then gets rq_qos_mutex. q_to_ioc(q) is NULL, so
   blk_iocost_init() runs on the dead disk. rq_qos_add() only checks for a
   duplicate id, so it links a new rqos, and blkcg_activate_policy() succeeds
   because the blkgs are only destroyed later in disk_release(). The model is
   then published and blk_stat accounting is turned on.

Nothing tears down an rqos added after rq_qos_exit(). disk_release() only
calls blkcg_exit_disk(), and blk_free_queue() does not look at q->rq_qos.
The consequences depend on who owns the queue:

- GD_OWNS_QUEUE: the ioc and its pcpu_stat leak.
- Queue not owned by the disk, e.g. SCSI sd, where __del_gendisk()
  unfreezes the queue and it is reused for the next disk on sd rebind: the
  stale ioc stays on q->rq_qos with ioc->rqos.disk pointing at the freed
  old gendisk. The following paths then dereference it:
  - the next rq_qos_exit(), via ioc_rqos_exit() ->
    blkcg_deactivate_policy(rqos->disk, ...)
  - ioc_rqos_queue_depth_changed() -> ioc_refresh_params() ->
    ioc_refresh_params_disk(ioc, force, ioc->rqos.disk)
  - an io.cost.qos write on the new disk, where q_to_ioc() returns the
    stale ioc

[ ... ]

> @@ -3531,11 +3778,31 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
>  				user = false;
>  			else if (!strcmp(buf, "user"))
>  				user = true;
> +#ifdef CONFIG_BLK_CGROUP_IOCOST_BPF
> +			else if (!strcmp(buf, "bpf")) {
> +				/*
> +				 * readback value while a BPF model is
> +				 * attached; attaching is done by loading
> +				 * the struct_ops, not through this file
> +				 */
> +				continue;
> +			}
> +#endif
>  			else
>  				goto unlock;

Is ctrl=bpf meant to be accepted here?

The COST_CTRL case now accepts "ctrl=bpf" as a silent no-op. Before this
patch the write failed with -EINVAL. The comment justifies this as a
"readback value while a BPF model is attached", and the documentation
(da89d4c2165e) says ctrl=bpf is accepted "so a saved configuration still
parses". But the read side never produces ctrl=bpf. ioc_cost_model_prfill()
still prints only

    dname, ioc->user_cost_model ? "user" : "auto",
    model ? "bpf" : "linear",

so only model= can read back as "bpf". The same docs commit also says
"ctrl keeps describing the builtin coefficients" and lists ctrl as "auto"
or "user". So no saved configuration can contain ctrl=bpf, and the stated
reason for accepting it does not hold.

The branch adds a write-only io.cost.model token with no read-side
counterpart and no effect. As user ABI it would have to be kept forever.
It is also accepted when no BPF model is attached, so a mistyped config is
taken silently where it used to be rejected.

The model=bpf branch in COST_MODEL is consistent with the readback and is
not affected.


---
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/35962141400

[RFC PATCH v7 2/4] 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, and a test which attaches it to one device:
the dev member of the struct_ops is written through the map's
initial value before load, as hid_bpf tests do with hid_id, and
attaching the struct_ops attaches the model to the device.  The test
verifies the model=bpf readback while attached, that a second model
on the same device fails with -EBUSY, and that detaching restores
the builtin model.

Under the same workload the doubled model charges 1.99x the
builtin model (measured 2882us -> 5722us per IO, completed IO count
halved).  The example also sets the transfer cost coefficients so
the builtin sizing follows the doubled pricing.

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

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.

Requires root, cgroup v2 and a device given as major:minor in
$IOCOST_TEST_DEV.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 tools/testing/selftests/bpf/config            |   2 +
 .../selftests/bpf/prog_tests/iocost_model.c   | 182 ++++++++++++++++++
 .../selftests/bpf/progs/iocost_model.c        | 139 +++++++++++++
 tools/testing/selftests/bpf/progs/iocost_ms.c | 159 +++++++++++++++
 4 files changed, 482 insertions(+)
 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

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index d292cb60a5a4..6e005145d3a8 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -140,3 +140,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..156c75367af0
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <fcntl.h>
+#include <sys/sysmacros.h>
+#include <unistd.h>
+#include "iocost_model.skel.h"
+#include "iocost_ms.skel.h"
+
+/*
+ * 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 && *end != ' ')
+		end++;
+	snprintf(model, model_sz, "%.*s", (int)(end - m), m);
+	return 0;
+}
+
+/*
+ * Attach the example model to one device, given as major:minor in
+ * $IOCOST_TEST_DEV: the dev member is written through the struct_ops
+ * map's initial value before load, as hid_bpf_ops does with hid_id,
+ * and loading attaches the model to the device.  Detaching the
+ * struct_ops restores the builtin model.
+ *
+ * Requires root, cgroup v2 and a device with iocost support.
+ */
+void serial_test_iocost_model(void)
+{
+	struct iocost_model *skel, *second;
+	unsigned int maj, min;
+	__u64 *ops_dev, *sdev;
+	int err;
+	char model[32], *dev;
+
+	dev = getenv("IOCOST_TEST_DEV");
+	if (!dev || geteuid() != 0 || sscanf(dev, "%u:%u", &maj, &min) != 2) {
+		test__skip();
+		return;
+	}
+
+	skel = iocost_model__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	/* dev is the first member of struct iocost_model_ops */
+	ops_dev = bpf_map__initial_value(skel->maps.iocost_2x, NULL);
+	if (!ASSERT_OK_PTR(ops_dev, "initial_value")) {
+		iocost_model__destroy(skel);
+		return;
+	}
+	*ops_dev = makedev(maj, min);
+
+	err = iocost_model__load(skel);
+	if (!ASSERT_OK(err, "skel_load")) {
+		iocost_model__destroy(skel);
+		return;
+	}
+
+	err = iocost_model__attach(skel);
+	if (ASSERT_OK(err, "attach")) {
+		/*
+		 * attached: the read path reports model=bpf until the
+		 * struct_ops is detached; ctrl keeps describing the
+		 * builtin coefficients
+		 */
+		err = readback_model(dev, model, sizeof(model));
+		if (ASSERT_OK(err, "readback"))
+			ASSERT_EQ(strcmp(model, "bpf"), 0, "model_bpf");
+
+		/* a second model on the same device fails with -EBUSY */
+		second = iocost_model__open();
+		if (ASSERT_OK_PTR(second, "second_open")) {
+			sdev = bpf_map__initial_value(
+					second->maps.iocost_2x, NULL);
+			if (!ASSERT_OK_PTR(sdev, "second_initial_value"))
+				goto out_destroy;
+			*sdev = makedev(maj, min);
+			err = iocost_model__load(second);
+			if (ASSERT_OK(err, "second_load")) {
+				struct bpf_link *l2;
+
+				/*
+				 * the kernel rejects attaching a second
+				 * model to the device with EBUSY
+				 */
+				l2 = bpf_map__attach_struct_ops(
+						second->maps.iocost_2x);
+				if (!ASSERT_ERR_PTR(l2, "second_ebusy"))
+					bpf_link__destroy(l2);
+				else
+					ASSERT_EQ(libbpf_get_error(l2), -EBUSY,
+						  "second_ebusy_errno");
+			}
+out_destroy:
+			iocost_model__destroy(second);
+		}
+
+		iocost_model__detach(skel);
+
+		err = readback_model(dev, model, sizeof(model));
+		if (ASSERT_OK(err, "readback_after_detach"))
+			ASSERT_EQ(strcmp(model, "linear"), 0, "model_linear");
+	}
+
+	iocost_model__destroy(skel);
+}
+/*
+ * Same check for the multi-stream example model.  Only one model can
+ * be attached to a device at a time; both tests attach and detach, so
+ * they are serial and independent.
+ */
+void serial_test_iocost_model_streams(void)
+{
+	struct iocost_ms *skel;
+	unsigned int maj, min;
+	__u64 *ops_dev;
+	int err;
+	char model[32], *dev;
+
+	dev = getenv("IOCOST_TEST_DEV");
+	if (!dev || geteuid() != 0 || sscanf(dev, "%u:%u", &maj, &min) != 2) {
+		test__skip();
+		return;
+	}
+
+	skel = iocost_ms__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	ops_dev = bpf_map__initial_value(skel->maps.iocost_ms, NULL);
+	if (!ASSERT_OK_PTR(ops_dev, "initial_value")) {
+		iocost_ms__destroy(skel);
+		return;
+	}
+	*ops_dev = makedev(maj, min);
+
+	err = iocost_ms__load(skel);
+	if (!ASSERT_OK(err, "skel_load")) {
+		iocost_ms__destroy(skel);
+		return;
+	}
+
+	err = iocost_ms__attach(skel);
+	if (ASSERT_OK(err, "attach")) {
+		err = readback_model(dev, model, sizeof(model));
+		if (ASSERT_OK(err, "readback"))
+			ASSERT_EQ(strcmp(model, "bpf"), 0, "model_bpf");
+
+		iocost_ms__detach(skel);
+
+		err = readback_model(dev, model, sizeof(model));
+		if (ASSERT_OK(err, "readback_after_detach"))
+			ASSERT_EQ(strcmp(model, "linear"), 0, "model_linear");
+	}
+
+	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..369818bda1cf
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iocost_model.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example iocost cost model: the builtin linear HDD formula with all
+ * costs doubled, for one device given by the dev member of the
+ * struct_ops.
+ *
+ * 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.  On a rotational device
+ * still on ctrl=auto, a device with this model attached charges
+ * twice the builtin model under the same workload, so the
+ * doubled cost is a direct check that accounting goes through the
+ * BPF path.  On a non-rotational device, or one with user-pinned
+ * coefficients, the ratio to the builtin model is arbitrary.
+ *
+ * A zero cursor means "no previous IO".  The cursor advances for
+ * every priced bio with a non-zero size (READ/WRITE), merged ones
+ * included, truncating to whole sectors like the builtin, so flushes
+ * and discards leave it alone and merged streams do not drift past
+ * the 16MB seek threshold.
+ *
+ * The model implements the full linear formula itself, including
+ * flushes: there is no fallback to the builtin model, a dataless
+ * A dataless WRITE|REQ_PREFLUSH keeps the write base: the op is still
+ * WRITE, so it carries WSEQIO (or WRANDIO after a seek) plus one page.
+ */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/*
+ * VTIME_PER_SEC, IOC_PAGE_SIZE/SHIFT, IOC_SECT_TO_PAGE_SHIFT and
+ * IOCOST_COST_F_MERGE come from vmlinux.h (BTF enum constants)
+ */
+#define LCOEF_RANDIO_PAGES	4096	/* 16MB seek threshold */
+#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
+ * per-cgroup state follows the cgroup lifetime
+ */
+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, struct bio *bio, u64 model_flags)
+{
+	u64 opf = bio->bi_opf, nbytes = bio->bi_iter.bi_size;
+	u64 sector = bio->bi_iter.bi_sector;
+	struct blkcg *blkcg = bio->bi_blkg->blkcg;
+	u64 pages, seek_pages = 0, base, coef_page, randio, cost;
+	__u64 *cursor, cur;
+	int priced;
+
+	/* builtin truncates: max(sectors >> IOC_SECT_TO_PAGE_SHIFT, 1) */
+	pages = nbytes >> IOC_PAGE_SHIFT;
+	if (!pages)
+		pages = 1;
+
+	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 cursor semantics: seek distance is only
+	 * computed against a non-zero cursor, and the cursor is
+	 * advanced for bios the builtin prices (READ/WRITE with a
+	 * non-zero size), merged ones included, so flushes and
+	 * discards leave it alone
+	 */
+	priced = (opf & IOCOST_REQ_OP_MASK) == REQ_OP_READ ||
+		 (opf & IOCOST_REQ_OP_MASK) == REQ_OP_WRITE;
+	cursor = bpf_cgrp_storage_get(&cursor_store,
+				      blkcg->css.cgroup, NULL,
+				      BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!cursor) {
+		if (model_flags & IOCOST_COST_F_MERGE)
+			base = 0;
+		return 2 * (base + pages * coef_page);
+	}
+	cur = *cursor;
+	if (cur && priced) {
+		seek_pages = sector > cur ? sector - cur
+					   : cur - sector;
+		seek_pages >>= IOC_SECT_TO_PAGE_SHIFT;
+		if (seek_pages > LCOEF_RANDIO_PAGES)
+			base = randio;
+	}
+	if (priced && nbytes)
+		*cursor = sector + (nbytes >> 9);
+
+	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 = {
+	.read_vtime_per_page = 2 * RPAGE,
+	.write_vtime_per_page = 2 * WPAGE,
+	.calc_cost = (void *)iocost_2x_calc_cost,
+};
+
+char LICENSE[] SEC("license") = "GPL";
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..aaa2e4489c1b
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iocost_ms.c
@@ -0,0 +1,159 @@
+// 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, IOC_PAGE_SIZE/SHIFT, IOC_SECT_TO_PAGE_SHIFT and
+ * IOCOST_COST_F_MERGE come from vmlinux.h (BTF enum constants)
+ */
+#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, struct bio *bio, u64 model_flags)
+{
+	u64 opf = bio->bi_opf, nbytes = bio->bi_iter.bi_size;
+	u64 sector = bio->bi_iter.bi_sector;
+	struct blkcg *blkcg = bio->bi_blkg->blkcg;
+	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 per-page writes
+		 */
+		base = 0; coef_page = WPAGE; randio = 0;
+	}
+	advance = RU(nbytes, 512);	/* sectors */
+
+	/* only bios the builtin prices participate in stream tracking */
+	if (!(((opf & IOCOST_REQ_OP_MASK) == REQ_OP_READ ||
+	       (opf & IOCOST_REQ_OP_MASK) == REQ_OP_WRITE) && nbytes)) {
+		pages = nbytes >> IOC_PAGE_SHIFT;
+		if (!pages)
+			pages = 1;
+		return base + pages * coef_page;
+	}
+
+	s = bpf_cgrp_storage_get(&stream_tab, blkcg->css.cgroup, NULL,
+				 BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!s) {
+		/* no storage: price per page, truncating like the builtin */
+		pages = nbytes >> IOC_PAGE_SHIFT;
+		if (!pages)
+			pages = 1;
+		return base + pages * coef_page;
+	}
+
+	/*
+	 * Slot access is lockless, mirroring the builtin single-cursor
+	 * update in ioc_rqos_throttle(): concurrent CPUs submitting for
+	 * the same cgroup can race on slot updates; mispricing is
+	 * bounded and acceptable for an example model.
+	 */
+	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;
+	}
+
+	/* builtin truncates: max(sectors >> IOC_SECT_TO_PAGE_SHIFT, 1) */
+	pages = nbytes >> IOC_PAGE_SHIFT;
+	if (!pages)
+		pages = 1;
+	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 = {
+	.read_vtime_per_page = RPAGE,
+	.write_vtime_per_page = WPAGE,
+	.calc_cost = (void *)iocost_ms_calc_cost,
+};
+
+char LICENSE[] SEC("license") = "GPL";
-- 
2.43.0

Re: [RFC PATCH v7 2/4] selftests/bpf: add iocost cost model test

[bot+bpf-ci]

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

> 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..156c75367af0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <fcntl.h>
> +#include <sys/sysmacros.h>
> +#include <unistd.h>
> +#include "iocost_model.skel.h"
> +#include "iocost_ms.skel.h"
> +
> +/*

[ ... ]

> +/*
> + * Attach the example model to one device, given as major:minor in
> + * $IOCOST_TEST_DEV: the dev member is written through the struct_ops
> + * map's initial value before load, as hid_bpf_ops does with hid_id,
> + * and loading attaches the model to the device.  Detaching the
> + * struct_ops restores the builtin model.

Is this description accurate?  The comment says "loading attaches the
model to the device", but the code and commit message indicate otherwise.

For non-link SEC(".struct_ops") maps, the kernel only calls .reg from
bpf_struct_ops_map_update_elem(), and libbpf issues that BPF_MAP_UPDATE_ELEM
only from bpf_map__attach_struct_ops().  So the attach happens during
iocost_model__attach(), not during iocost_model__load().

The test itself relies on this distinction.  It calls
iocost_model__load(second) at line 103 while the first model is attached and
expects the load to succeed (line 103, ASSERT_OK(err, "second_load")).  The
-EBUSY failure is expected only from bpf_map__attach_struct_ops() at line
110.  The commit message also says "attaching the struct_ops attaches the
model to the device".

Should the comment say "attaching" rather than "loading"?

> + * Requires root, cgroup v2 and a device with iocost support.
> + */
> +void serial_test_iocost_model(void)
> +{
> +	struct iocost_model *skel, *second;
> +	unsigned int maj, min;
> +	__u64 *ops_dev, *sdev;
> +	int err;
> +	char model[32], *dev;
> +
> +	dev = getenv("IOCOST_TEST_DEV");
> +	if (!dev || geteuid() != 0 || sscanf(dev, "%u:%u", &maj, &min) != 2) {
> +		test__skip();
> +		return;
> +	}

A subsystem pattern flags this as potentially concerning: both
serial_test_iocost_model() and serial_test_iocost_model_streams() call
test__skip() without printing a message.  Three different conditions lead
there: IOCOST_TEST_DEV not set, not running as root, or a malformed
major:minor string.  Since IOCOST_TEST_DEV is never set in CI, these tests
are always skipped, and nothing in the output explains that
IOCOST_TEST_DEV=<maj>:<min> is needed to run them.

Many prog_tests print a short message before test__skip() to explain why
(like "printf(\"%s:SKIP: set IOCOST_TEST_DEV=<maj>:<min>\\n\", __func__);").
test_progs still prints a SKIP status line for the test, so the skip is not
entirely silent, but the message would tell a developer how to opt in.

Also, ioc_bpf_attach() returns -EOPNOTSUPP when the device's queue is not
blk-mq or lacks iocost support.  The tests assert ASSERT_OK(err, "attach")
at lines 84 and 169, so a device without the right support is reported as a
failure rather than a skip.  Is this the intended behavior?

> +
> +	skel = iocost_model__open();

[ ... ]

> 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..369818bda1cf
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iocost_model.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Example iocost cost model: the builtin linear HDD formula with all
> + * costs doubled, for one device given by the dev member of the
> + * struct_ops.
> + *
> + * 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.  On a rotational device
> + * still on ctrl=auto, a device with this model attached charges
> + * twice the builtin model under the same workload, so the
> + * doubled cost is a direct check that accounting goes through the
> + * BPF path.  On a non-rotational device, or one with user-pinned
> + * coefficients, the ratio to the builtin model is arbitrary.
> + *
> + * A zero cursor means "no previous IO".  The cursor advances for
> + * every priced bio with a non-zero size (READ/WRITE), merged ones
> + * included, truncating to whole sectors like the builtin, so flushes
> + * and discards leave it alone and merged streams do not drift past
> + * the 16MB seek threshold.
> + *
> + * The model implements the full linear formula itself, including
> + * flushes: there is no fallback to the builtin model, a dataless
> + * A dataless WRITE|REQ_PREFLUSH keeps the write base: the op is still
> + * WRITE, so it carries WSEQIO (or WRANDIO after a seek) plus one page.

Two things here: there's a broken sentence ("... a dataless\n * A dataless
WRITE|REQ_PREFLUSH ..."), and the description contradicts what the code
actually does.

Lines 3-4 say the model is "the builtin linear HDD formula with all costs
doubled", and lines 10-11 say it "charges twice the builtin model under the
same workload".  The commit message also describes it as "the full builtin
linear HDD formula at double cost".

However, the builtin linear formula does not price flushes.
calc_vtime_cost_builtin() starts with `if (!bio->bi_iter.bi_size) goto out;`
and returns cost 0, so ioc_rqos_throttle() does not charge a zero-cost bio.
This model instead charges a dataless WRITE|REQ_PREFLUSH as
`2 * (base + 1 * WPAGE)`.

An empty flush bio also has bi_sector == 0 (blkdev_issue_flush() calls
bio_init() with no sector).  The `cur && priced` branch at lines 114-119
therefore measures the "seek" as the absolute cursor position.  So once the
cgroup has done any IO beyond the first 16MB, every fsync-style flush is
priced as WRANDIO: 2 * (360433855 + 3161261) = 727190232 vtime, about 5.3ms
of device time, where the builtin charges 0.

The same header in iocost_ms.c (lines 16-17) says "The model implements the
full builtin linear formula itself, including flush pricing", but the builtin
has no flush pricing, so that is inaccurate too.  On any workload with
flushes (or discards, see below), the "twice the builtin" claim and the
commit message's 1.99x figure do not hold.

Could the header be clarified to state that flushes are priced even though
the builtin charges them nothing, and that because a flush carries sector 0
it is effectively always priced as random IO?

> + */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>

[ ... ]

> +	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;
> +	}

[ ... ]

> +
> +	cost = 2 * (base + pages * coef_page);
> +	return cost;

Does the comment match the code?  The comment says "unknown ops are priced as
a single page write", but the code prices them per page of bi_size.

pages is computed earlier as `nbytes >> IOC_PAGE_SHIFT` (clamped to at least
1), so the charge is `2 * pages * coef_page`.  For REQ_OP_DISCARD,
REQ_OP_WRITE_ZEROES and REQ_OP_SECURE_ERASE, bi_iter.bi_size is the size of
the range, not a transfer size.  A 1MB write-zeroes bio is 256 pages, which
is about 1.6e9 vtime (~12ms).  Any range of about 85MB or more hits the
VTIME_PER_SEC clamp in calc_vtime_cost(), so each such bio is charged a full
second of device time.  A single page write would be about 6.3e6 vtime
(~46us).

For comparison, calc_vtime_cost_builtin() takes `default: goto out;` for
these ops and returns 0, and ioc_rqos_throttle() then skips the bio with
`if (!abs_cost) return;`.  So these ops are not "twice the builtin" either.

The sibling iocost_ms.c has identical code with the comment "unknown ops are
priced as per-page writes", which matches its code, so the iocost_model.c
comment looks stale.

Since this file is presented as the reference example for the new
iocost_model_ops API, should the comment say per-page pricing, or should
pages be forced to 1 for these ops if a single page was intended?


---
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/35962141400

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

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

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 is running, 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.

Unlike the existing iocost tracepoints, which are state-change
driven and silent in steady state, this allows a bound cost model's
behaviour to be evaluated without drgn.

Depending on the autop profile this is 2-100 events per second per
device; the added cost outside the tracepoint static key is one
increment per active cgroup per period.

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

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 21e4f8cbd9f2..a48751b6dbdd 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2252,6 +2252,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;
@@ -2288,6 +2289,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.
@@ -2449,6 +2452,18 @@ static void ioc_timer_fn(struct timer_list *timer)
 
 	ioc->busy_level = clamp(ioc->busy_level, -1000, 1000);
 
+	/*
+	 * Everything the tick reports is final here: busy_level was just
+	 * computed, running and cur_period haven't changed, nr_active and
+	 * usage_us_sum are complete, and vrate and period_us still hold
+	 * the values this period ran in.  Emit before the refresh below
+	 * so the event reads the completed period directly.
+	 */
+	trace_iocost_ioc_tick(ioc, nr_active, usage_us_sum,
+			      ioc->period_us, ioc->vtime_base_rate,
+			      ioc->busy_level, ioc->running,
+			      now.now - ioc->period_at);
+
 	ioc_adjust_base_vrate(ioc, rq_wait_pct, nr_lagging, nr_shortages,
 			      prev_busy_level, missed_ppm);
 
diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
index e772b1bc60d6..32f19861a78f 100644
--- a/include/trace/events/iocost.h
+++ b/include/trace/events/iocost.h
@@ -178,6 +178,51 @@ 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,
+		 u32 tick_period_us, u64 tick_vrate,
+		 int tick_busy, int tick_running, u64 tick_dur),
+
+	TP_ARGS(ioc, nr_active, usage_us_sum, tick_period_us, tick_vrate,
+		tick_busy, tick_running, tick_dur),
+
+	TP_STRUCT__entry (
+		__string(devname, ioc_name(ioc))
+		__field(u64, cur_period)
+		__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->cur_period = atomic64_read(&ioc->cur_period);
+		__entry->period_us = tick_period_us;
+		__entry->vrate = tick_vrate;
+		__entry->busy_level = tick_busy;
+		__entry->nr_active = nr_active;
+		__entry->usage_pct = tick_dur ?
+			div64_u64(usage_us_sum * 100, tick_dur) : 0;
+		__entry->running = tick_running;
+	),
+
+	TP_printk("[%s] period=%llu:%uus vrate=%llu busy=%d active=%d usage=%u%% running=%d",
+		__get_str(devname), __entry->cur_period, __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 v7 3/4] blk-iocost: add iocost_ioc_tick tracepoint for per-period device summary

[bot+bpf-ci]

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

> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index 21e4f8cbd9f2..a48751b6dbdd 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c

[ ... ]

> diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
> index e772b1bc60d6..32f19861a78f 100644
> --- a/include/trace/events/iocost.h
> +++ b/include/trace/events/iocost.h
> @@ -178,6 +178,51 @@ 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,
> +		 u32 tick_period_us, u64 tick_vrate,
> +		 int tick_busy, int tick_running, u64 tick_dur),
> +
> +	TP_ARGS(ioc, nr_active, usage_us_sum, tick_period_us, tick_vrate,
> +		tick_busy, tick_running, tick_dur),
> +
> +	TP_STRUCT__entry (
> +		__string(devname, ioc_name(ioc))
> +		__field(u64, cur_period)
> +		__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->cur_period = atomic64_read(&ioc->cur_period);
> +		__entry->period_us = tick_period_us;
> +		__entry->vrate = tick_vrate;
> +		__entry->busy_level = tick_busy;

This isn't a bug, but since the ioc pointer is already passed and
cur_period is read from it inside TP_fast_assign, could period_us, vrate
and busy_level also be read from ioc here?

The neighbouring iocost_ioc_vrate_adj event reads ioc->vtime_base_rate
and ioc->busy_level directly from the ioc pointer in its TP_fast_assign.
Reading them directly would make this event's prototype consistent with
that pattern.

> +		__entry->nr_active = nr_active;
> +		__entry->usage_pct = tick_dur ?
> +			div64_u64(usage_us_sum * 100, tick_dur) : 0;
> +		__entry->running = tick_running;
> +	),
> +
> +	TP_printk("[%s] period=%llu:%uus vrate=%llu busy=%d active=%d usage=%u%% running=%d",
> +		__get_str(devname), __entry->cur_period, __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,


---
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/35962141400

[RFC PATCH v7 4/4] docs: cgroup-v2: document the iocost BPF cost model attachment

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Document the BPF cost model attachment in the io.cost.model section
of the cgroup v2 documentation: attaching an iocost_model_ops
struct_ops to a device by its major:minor, the model=bpf readback
while attached (ctrl keeps describing the coefficients), that
detaching restores the builtin model, and that writes never select
a model.

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

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 8d2603751c51..306cc929c88d 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2117,8 +2117,30 @@ IO Interface Files
 	  =====		================================
 	  ctrl		"auto" or "user"
 	  model		The cost model in use - "linear"
+			or "bpf" while a BPF model is
+			attached
 	  =====		================================
 
+	When CONFIG_BLK_CGROUP_IOCOST_BPF is enabled, a BPF cost model
+	can be attached to a device by loading an "iocost_model_ops"
+	struct_ops with the whole disk's major:minor in its "dev" member
+	(a partition's major:minor is rejected);
+	attaching switches the device's pricing to the model, detaching
+	the struct_ops restores the builtin linear model.  While a model
+	is attached, "model" reads back "bpf"; "ctrl" keeps describing
+	the builtin coefficients, which are inert while the model is
+	attached: coefficient writes are stored and take effect again
+	after the struct_ops is detached, and the automatic profile
+	stepping does not switch profiles.  Writing "ctrl=bpf" or
+	"model=bpf" is accepted as a no-op so a saved configuration
+	still parses, but re-attaching the model requires loading the
+	struct_ops again, not writing to this file.  Attaching
+	implicitly enables the controller if needed (disabling wbt
+	like io.cost.qos does); detaching does not disable it again,
+	and writing "enable=0" to io.cost.qos suspends the model's
+	pricing until the controller is re-enabled, while it stays
+	attached.
+
 	When "ctrl" is "auto", the kernel may change all parameters
 	dynamically.  When "ctrl" is set to "user" or any other
 	parameters are written to, "ctrl" become "user" and the
-- 
2.43.0