Skip to content

Conversation

@uran0sH
Copy link

@uran0sH uran0sH commented Jul 7, 2024

In cgroup v2, there is no /sys/fs/cgroup/cpu directory. All groups are mounted on /sys/fs/cgroup. And if we would like to open child's group's cpu subsystems, we need to "echo "+cpu" > cgroup.subtree_control" in the parent group.

@uran0sH
Copy link
Author

uran0sH commented Jul 7, 2024

@vingu-linaro if you have time, plz review this pr. thanks

@uran0sH uran0sH force-pushed the dev branch 2 times, most recently from 74e7474 to 909b59e Compare July 7, 2024 09:46
@uran0sH uran0sH force-pushed the dev branch 2 times, most recently from beab893 to 0f0a372 Compare July 9, 2024 11:54
@uran0sH uran0sH requested a review from Werkov July 9, 2024 12:19
@uran0sH
Copy link
Author

uran0sH commented Jul 10, 2024

In summary, I should only enable the threaded mode on the target cgroup type(like Werkov said that I also need to check the domain in other controllers) and write the tid to the cgroup.procs? Thanks @vingu-linaro @Werkov

@uran0sH uran0sH force-pushed the dev branch 4 times, most recently from a14cfc8 to 8edf7b4 Compare July 11, 2024 16:57
@uran0sH uran0sH requested review from Werkov and vingu-linaro July 11, 2024 16:58
Copy link

@Werkov Werkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code seem to work in some situations (single threaded processes) but could be broken in some other (noted in a comment). There may be no tests in rt-app that would capture this so it may only become visible when used by more users :)

@uran0sH uran0sH changed the title Add cgroup v2 support [WIP] Add cgroup v2 support Aug 25, 2024
@uran0sH uran0sH changed the title [WIP] Add cgroup v2 support Add cgroup v2 support Sep 1, 2024
Signed-off-by: Wenyu Huang <huangwenyuu@outlook.com>
Copy link

@Werkov Werkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could work.
Added some stylistic/resource tracking remarks.


file = strcmp(name, "/") ? "/tasks" : "tasks";
if (ctrl.version == 1) {
file = strcmp(name, "/") ? "/tasks" : "tasks";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder it this special casing for root cgrp is worth it (here and other places).

-sprintf(path, "%s%s%s", ctrl.mount_point, name, file);
+sprintf(path, "%s%s/%s", ctrl.mount_point, name, file);

with possibly double // being resolved by open.

if (fclose(tasks)) {
perror("fclose");
goto error;
if (ctrl.version == 1) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v2 file deserves to be closed here too, no?

goto error;
}
if (fprintf(cgroup_file, "threaded") < 0) {
perror("fprintf");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cgroup_file not closed here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also some better message may ease future debugging, like cgroup.type <- threaded failed.

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.

3 participants