Re: [PATCH 3/5] selftests/bpf: add iocost cost model test

[bot+bpf-ci]

> selftests/bpf: add iocost cost model test

TL;DR: Nit only: in the iocost_model selftest, the comment before the final
"pair_balanced_again" assertion describes the re-attach init check, not the
detach-time init/free pairing it actually tests.

> diff --git a/tools/testing/selftests/bpf/prog_tests/iocost_model.c b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> new file mode 100644
> index 000000000000..5373d52d93a6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> @@ -0,0 +1,806 @@

[ ... ]

> +void serial_test_iocost_model_removal(void)
> +{

[ ... ]

> +	err = iocost_model__attach(skel2);
> +	if (!ASSERT_OK(err, "attach_again"))
> +		goto out2;
> +
> +	err = readback_model(dev, model, sizeof(model));
> +	if (ASSERT_OK(err, "readback_again"))
> +		ASSERT_STREQ(model, "bpf", "model_bpf_again");
> +	ASSERT_GT(skel2->bss->iocg_inits, 0, "pair_init_again");
> +
> +	bpf_link__destroy(skel2->links.iocost_2x);
> +	skel2->links.iocost_2x = NULL;
> +
> +	err = readback_model(dev, model, sizeof(model));
> +	if (ASSERT_OK(err, "readback_after_detach_again"))
> +		ASSERT_STREQ(model, "linear", "model_linear_again");
> +
> +	/* the re-attach walk delivered init for the recreated device */
> +	ASSERT_EQ(skel2->bss->iocg_frees, skel2->bss->iocg_inits,
> +		  "pair_balanced_again");

This isn't a bug, but the comment above the final assertion doesn't seem to
describe the check below it.

The comment says "the re-attach walk delivered init for the recreated
device", while the assertion that follows compares iocg_frees against
iocg_inits after bpf_link__destroy(), which is the detach walk delivering a
matching iocg_free() for every iocg_init().  The init side is the
ASSERT_GT(skel2->bss->iocg_inits, 0, "pair_init_again") check a few lines
earlier, right after the attach.

Should this comment sit next to the "pair_init_again" check, or say that the
detach walk paired every init with a free?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40710751825