Re: [RFC v3 2/3] tools/ufq_iosched: add BPF example scheduler and build scaffolding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ufq_iosched: bpf_rbtree_left() returns structural child missing logical predecessor
--

commit 64af8341d2a8ec597c3320deffd60d7e970c5a0d
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
Subject: tools/ufq_iosched: add BPF example scheduler and build scaffolding

Add ufq_iosched as a simple example for the UFQ block I/O scheduler. In the
ufq_simple example, we implement the eBPF struct_ops hooks the kernel exposes so
we can exercise and validate the behavior and stability of the kernel UFQ
scheduling framework.

diff --git a/tools/ufq_iosched/ufq_simple.bpf.c b/tools/ufq_iosched/ufq_simple.bpf.c
new file mode 100644
index 0000000000000..f49ef0d192588
--- /dev/null
+++ b/tools/ufq_iosched/ufq_simple.bpf.c
[ ... ]
> +static struct request *merge_bio_left_unlock(struct ufq_simple_data *ufq_sd,
> +					     struct bpf_rb_root *tree,
> +					     struct sort_tree_node *snode,
> +					     struct request *cand)
> +{
> +	struct sort_tree_node *left_node = NULL, *owned;
> +	sector_t cand_start, left_start, left_end;
> +	struct bpf_rb_node *tmp, *removed = NULL;
> +	struct request *left_rq = NULL;
> +	bool merged = false;
> +
> +	cand_start = cand->__sector;
> +	tmp = bpf_rbtree_left(tree, &snode->rb_node);
> +	if (!tmp)
> +		goto end;
> +
> +	left_node = container_of(tmp, struct sort_tree_node, rb_node);

[Severity: Medium]
Will bpf_rbtree_left() return the structural left child of the RB-tree node
instead of the logically adjacent predecessor in the sorted order?

If the logical predecessor is an ancestor or the rightmost node of the left
subtree, will this structural check test the wrong node and miss valid double
merge opportunities for I/O requests?

[ ... ]
> +static struct request *merge_bio_right_unlock(struct ufq_simple_data *ufq_sd,
> +					      struct bpf_rb_root *tree,
> +					      struct sort_tree_node *snode,
> +					      struct request *cand)
> +{
> +	struct bpf_rb_node *right_rb, *removed = NULL;
> +	struct sort_tree_node *right_node = NULL;
> +	struct request *right_rq = NULL;
> +	sector_t cand_end, right_start;
> +	bool merged = false;
> +
> +	cand_end = cand->__sector + (cand->__data_len >> SECTOR_SHIFT);
> +
> +	right_rb = bpf_rbtree_right(tree, &snode->rb_node);
> +	if (!right_rb)
> +		goto end;
> +
> +	right_node = container_of(right_rb, struct sort_tree_node, rb_node);

[Severity: Medium]
Does bpf_rbtree_right() return the structural right child instead of the
logically adjacent successor?

Similar to the left check above, will this structural check bypass valid
double merge opportunities when the logical successor is not the direct
right child?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003042748.33795-1-kaitao.cheng@linux.dev?part=2