Re: [RFC PATCH 2/8] blk-iocost: define iocost_model_ops cost model interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `iocost_model_ops` interface uses recycled cgroup IDs (`iocg_id`) without a lifecycle callback (e.g., `release`), making safe per-cgroup state management impossible.
- [Low] Incorrect header reference in BPF struct_ops documentation.
--

commit 64a8ffdba67eaf906639412faa34e6125d2960c1
Author: Tao Cui <cuitao@kylinos.cn>

blk-iocost: define iocost_model_ops cost model interface

This commit defines the interface for pluggable cost models via a struct_ops
with a single calc_cost callback. It passes iocg_id to allow models to keep
per-cgroup state. The model is called from the IO submission path under RCU.

> diff --git a/include/linux/blk-iocost.h b/include/linux/blk-iocost.h
> new file mode 100644
> index 0000000000000..7111b5c03dc36
> --- /dev/null
> +++ b/include/linux/blk-iocost.h
> @@ -0,0 +1,56 @@

[ ... ]

> +struct iocost_model_ops {
> +	/*
> +	 * @op:		REQ_OP_* value (uapi blk_opf.h)

[Severity: Low]
Is this the correct header reference?

The file blk_opf.h does not exist in uapi. Should this point to
include/linux/blk_types.h instead?

> +	 * @nbytes:	IO size in bytes
> +	 * @sector:	starting sector
> +	 * @cursor:	iocg cursor sector, 0 if none (sequentiality hint)
> +	 * @iocg_id:	css id of the issuing cgroup
> +	 * @flags:	IOCOST_COST_F_*
> +	 */
> +	u64 (*calc_cost)(u64 op, u64 nbytes, u64 sector, u64 cursor,
> +			 u64 iocg_id, u64 flags);
> +};

[Severity: High]
Does this struct_ops interface need a release or offline callback for cgroup
state cleanup?

When a cgroup is deleted and its CSS ID is recycled, the new cgroup inherits
the historical IO state of the deleted cgroup if the BPF model tracks it. 
There is no reliable way for the BPF program to garbage collect entries,
which can eventually exhaust map space.

Could unprivileged users with cgroup delegation create and destroy cgroups,
triggering map exhaustion and causing a denial of service for the cost model
(which falls back to failure or builtin behavior once max_entries is reached)?

The commit message notes that the need for a release(iocg_id) callback is
an open question. Since BPF struct_ops signatures are frozen once merged, 
is it safe to omit it from the initial interface? This appears to be
confirmed in a follow-up patch where map entries have no deletion logic
and will leak in a production environment.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908100143.47598-1-cui.tao@linux.dev?part=2