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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] block: direct 64-bit division in calc_size_vtime_cost()
--

commit 848a45831f82ba85b8460e41c4e1203c6310b105
Author: Tao Cui <cuitao@kylinos.cn>
blk-iocost: add BPF struct_ops cost model support

This commit adds the iocost_model_ops BPF struct_ops type, allowing BPF
programs to replace the built-in linear cost model for blk-iocost. It attaches
per-device following the hid_bpf_ops model and takes over pricing for every IO
on the device.

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

[ ... ]

> @@ -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)

[Severity: Low]
Does this direct 64-bit division cause build failures on 32-bit architectures?

Using the division operator for 64-bit integers like VTIME_PER_SEC / coeff 
forces the compiler to emit a call to __udivdi3, which is omitted in the 
Linux kernel and results in an undefined reference link-time error when 
CONFIG_BLK_CGROUP_IOCOST_BPF is enabled. 

Could this be changed to use div64_u64() instead?

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

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