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

[Tao Cui <cui.tao@linux.dev>]

Hello Tejun,

在 2026/10/17 01:49, Tejun Heo 写道:
> Hello, Tao.
> 
> On Sat, 03 Oct 2026 20:40:43 +0800, Tao Cui wrote:
>> +	dev = getenv("IOCOST_TEST_DEV");
> 
> Nothing in the tree sets IOCOST_TEST_DEV, so both tests always skip in CI.
> The null_blk instance the v11 removal test creates could serve these two
> as well.
> 
Right, that was an oversight. All three tests now create their own
null_blk instance through configfs. The create/remove helpers from the
removal test are shared, the $IOCOST_TEST_DEV handling is gone, and
everything now runs in CI.

>> +		if (ASSERT_OK(err, "readback"))
>> +			ASSERT_EQ(strcmp(model, "bpf"), 0, "model_bpf");
> 
> ASSERT_STREQ() prints both strings on failure.
> 
Switched.

>> +		bpf_link__destroy(skel->links.iocost_2x);
>> +		/* cleared, or the skeleton destroy below detaches again */
>> +		skel->links.iocost_2x = NULL;
> 
> Without clearing, the skeleton destroy would call bpf_link__destroy() on
> the freed link. It wouldn't detach again.
> 
Good catch. The comment now says the link has already been destroyed
and the field is cleared so iocost_model__destroy() doesn't try to
destroy it again.

>> + * The cursor handling follows the builtin: a zero cursor means "no
>> + * previous IO", and the cursor is advanced for bios the builtin
>> + * prices (READ/WRITE with a non-zero size; the builtin skips the
>> + * whole update when the cost is zero), so flushes and discards
>> + * leave it alone and long merged streams do not drift past the
>> + * 16MB seek threshold.
> 
> The model advances the cursor for every merged READ or WRITE bio.
> ioc_rqos_merge() only moves it on a back merge into the request ending at
> the cursor, so a front merge leaves the builtin's cursor alone and moves
> the model's.
> 
Fixed; the example model now moves the cursor the way
ioc_rqos_merge() moves the builtin's, so front merges leave it alone.

>> +	} else {
>> +		base = randio;
>> +		for (i = 1; i < NSLOTS; i++) {
>> +			if (s->stamp[i] < s->stamp[victim])
>> +				victim = i;
>> +		}
> 
> A merged bio which matches no slot, e.g. a front merge or a merge into
> another stream's request, takes the LRU slot and can evict a live stream.
> Maybe skip the slot replacement for IOCOST_COST_F_MERGE?
> 
Agreed; a merged bio which matches no slot is now priced without
touching the table, so a merge can no longer evict a live stream.

Thanks,
Tao

> Thanks.
>