Hello Shashank. On Tue, Jul 01, 2025 at 11:13:54PM +0900, Shashank Balaji <[email protected]> wrote: > cpu.max selftests (both the normal one and the nested one) test the > working of throttling by setting up cpu.max, running a cpu hog process > for a specified duration, and comparing usage_usec as reported by > cpu.stat with the duration of the cpu hog: they should be far enough. > > Currently, this is done by using values_close, which has two problems: > > 1. Semantic: values_close is used with an error percentage of 95%, which > one will not expect on seeing "values close". The intent it's > actually going for is "values far". > > 2. Accuracy: the tests can pass even if usage_usec is upto around double > the expected amount. That's too high of a margin for usage_usec. > > Overall, this patchset improves the readability and accuracy of the > cpu.max tests. > > Signed-off-by: Shashank Balaji <[email protected]>
I think you're getting at an actual bug in the test definition.
I think that the test_cpucg_max should either run hog_cpus_timed with
CPU_HOG_CLOCK_PROCESS instead of CPU_HOG_CLOCK_WALL to make sense or the
expected_usage_usec should be defined with the configured quota in mind
(i.e. 1/100). (The latter seems to make the test more natural.)
With such defined metrics, the asserted expression could be
values_close(usage_usec, expected_usage_usec, 10)
based on your numbers, error is around 20% so our helper's argument is
roughly half of that. (I'd be fine even with err=20 to prevent some
false positives.)
I think those changes could even be in one patch but I leave that up to
you. My comment to your 2nd patch is that I'd like to stick to relative
errors and keep positive values_close() predicate that's used in other
selftests too. (But those 95% in the current code are clumsy given two
different qualities are compared.)
Thanks,
Michal
signature.asc
Description: PGP signature
