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

[Tejun Heo <tj@kernel.org>]

Hello, Tao.

On Thu, 24 Sep 2026 13:45:46 +0800, Tao Cui wrote:

> -	/* 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
> +	   )

Can you add a helper which returns the model in use with a stub returning
NULL for !CONFIG_BLK_CGROUP_IOCOST_BPF? That'd remove most of the #ifdefs
including the one in this condition and the duplicated seq_printf() in
ioc_cost_model_prfill().

> +		/* 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);

This can just be pages * coeff like the builtin. An overflow only skews
the met/missed accounting, same as a user-set linear coefficient, and it
also gets rid of the 64-bit division.

> +	bdevf = bdev_file_open_by_dev(new_decode_dev(ops->dev),
> +				      BLK_OPEN_READ, NULL, NULL);

When the disk goes away, the model should be ejected completely.
ioc_rqos_exit() unbinds it but the open bdev file keeps the dead disk and
the driver module pinned until the link is destroyed. Can you drop all
device references on removal like hid_bpf_destroy_device() does and look
up the device like blkg_conf_open_bdev() does, with blkdev_get_no_open()
and disk_live() checked under rq_qos_mutex? The two attach issues bpf-ci
reported, the missing re-attach check in .reg and the missing disk_live()
check, are real.

Thanks.

--
tejun