libct/cg/fs2/freezer: optimize and fix #2955
Conversation
Could you mention inotify in the code comments as TODO (or just open an issue for inotify), then LGTM |
|
LGTM aside from Akihiro's nit. |
In case configs.Undefined or any wrong value is passed, there is no need to check whether the freezer is supported. Move arguments check to the beginning to avoid an unnecessary call to supportFreezer(). While at it, simplify the "whether to return an error if freezer is not supported" check. Signed-off-by: Kir Kolyshkin <[email protected]>
Before this patch, setFreezer does - open/read/close (to check if the freezer is supported) - open/write/close (to set the value) - open/read/close (to check the value) Three opens is a bit excessive. Refactor to only open the file once: - open (to check if the freezer is supported) - write (to set the value) - seek/read (to check the value) - close Signed-off-by: Kir Kolyshkin <[email protected]>
According to cgroup v2 documentation [1]: > Freezing of the cgroup may take some time; when this action is > completed, the “frozen” value in the cgroup.events control file will > be updated to “1” and the corresponding notification will be issued. Implement polling of cgroup.events, waiting for "frozen 1" to appear. In case something goes wrong, limit the maximum number of retries and return "undefined" after some time (currently 10s). [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html Signed-off-by: Kir Kolyshkin <[email protected]>
Added a (I actually did a poll(2)-based implementation but it looked so horrible I ditched it. Haven't tried inotify yet (although there's already an implementation in |
|
Fedora CI failed during |
fs2.setFreezerimprovements:config.Undefinedis passed).Please see individual commits for details.
Note: I chose to use simple wait/retry for re-reading
cgroup.events. An implementation based onpoll(2)orinotify(7)is possible, but it makes things more complicated. If needed, a poll- or inotify-based waiting can be add later.