Skip to content

cgroup: use cgroup.kill if available#724

Merged
giuseppe merged 2 commits intocontainers:mainfrom
giuseppe:support-cgroup.kill
Aug 31, 2021
Merged

cgroup: use cgroup.kill if available#724
giuseppe merged 2 commits intocontainers:mainfrom
giuseppe:support-cgroup.kill

Conversation

@giuseppe
Copy link
Copy Markdown
Member

Linux 5.14 supports the file "cgroup.kill" to kill all the processes
in a cgroup. It avoids the cost of opening "cgroup.procs" and send
the kill signal to each process listed.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@giuseppe giuseppe force-pushed the support-cgroup.kill branch from aa66c62 to dbd5d2c Compare August 27, 2021 08:58
Copy link
Copy Markdown
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

I am not sure but should we silently fallback to killing process iteratively when write( fails for cgroup.kill instead of returning crun_error

@giuseppe
Copy link
Copy Markdown
Member Author

I am not sure but should we silently fallback to killing process iteratively when write( fails for cgroup.kill instead of returning crun_error

yes, I think we need to fallback in any case. Either if it is not supported, or if for any reason it fail, so we don't leave processes around.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Aug 27, 2021

I agree we should fall back and try our best to kill all of the processes in the cgroup.

@giuseppe giuseppe force-pushed the support-cgroup.kill branch from dbd5d2c to 8eaabf6 Compare August 30, 2021 08:20
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Linux 5.14 supports the file "cgroup.kill" to kill all the processes
in a cgroup.  It avoids the cost of opening "cgroup.procs" and send
the kill signal to each process listed.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@flouthoc
Copy link
Copy Markdown
Collaborator

LGTM

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