Skip to content

Conversation

@ls-ggg
Copy link
Contributor

@ls-ggg ls-ggg commented Mar 29, 2024

Fix: #4094
when users set or update cpu.rt_period_us and cpu.rt_runtime_us at the same time,
runc should set these two values in the appropriate order to ensure that the intermediate value of cpu.rt_runtime_us/cpu.rt_period_us is less than the limit ratio

@ls-ggg ls-ggg force-pushed the fix-set-cpu.rt branch 8 times, most recently from e5c7a60 to e785c44 Compare March 29, 2024 06:03
@kolyshkin
Copy link
Contributor

This code is quite complicated. Can you please do something similar to commit 1b2adcf (and add a test case, too)?

@ls-ggg ls-ggg force-pushed the fix-set-cpu.rt branch 6 times, most recently from 4a16e8b to 28268ee Compare April 1, 2024 07:06
@ls-ggg
Copy link
Contributor Author

ls-ggg commented Apr 1, 2024

This code is quite complicated. Can you please do something similar to commit 1b2adcf (and add a test case, too)?

This code is quite complicated. Can you please do something similar to commit 1b2adcf (and add a test case, too)?
@kolyshkin Thanks for your suggestion, I have completed the comments and commit information. and added test cases

@ls-ggg ls-ggg force-pushed the fix-set-cpu.rt branch 2 times, most recently from c8a27c4 to 190d5dc Compare April 2, 2024 02:43
@ls-ggg
Copy link
Contributor Author

ls-ggg commented Apr 2, 2024

rebase

@ls-ggg
Copy link
Contributor Author

ls-ggg commented Apr 12, 2024

@kolyshkin Can this PR be approved?

@ls-ggg ls-ggg requested a review from lifubang April 12, 2024 16:21
@ls-ggg ls-ggg force-pushed the fix-set-cpu.rt branch 2 times, most recently from 1c22922 to c4d13e3 Compare April 14, 2024 08:00
As reported in issue opencontainers#4094, sometimes setting rt_period_us and
rt_runtime_us at the same time will fail. The reason is that in
 cgroupv1, these two values cannot be set atomically.

When we set a new rt_period_us, the kernel will determine
whether the current configuration of new_limit1 = old_quota/new_period
exceeds the limit. If it exceeds the limit, an error will be reported.
Maybe it is reasonable to set rt_runtime_us first so that the
new_limit2 = new_quota/old_period.
for example:
The original state of cgv1 is rt_period_us: 10000 rt_runtime_us: 5000
The user wants to change it to rt_period_us: 1000 rt_runtime_us:300.
The new rt_runtime_us should be set first.

In the opposite case, if rt_runtime_us is set first, new_limit2 may still
exceed the limit, but new_limit1 will be valid.
for example:
The original state of cgv1 is rt_period_us: 1000 rt_runtime_us: 500
The user wants to change it to rt_period_us: 10000 rt_runtime_us:3000.
The new rt_period_us should be set first.

Therefore, new_limit1 and new_limit2 should be calculated in advance,
and the smaller corresponding setting order should be selected to set
rt_period_us and rt_runtime_us.

Signed-off-by: ls-ggg <335814617@qq.com>
@ls-ggg ls-ggg requested a review from kolyshkin April 28, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When runc updates the values ​​of cpu-rt-period and cpu-rt-runtime synchronously, an error occurs.

3 participants