Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

specs-go: mark LinuxMemory.Kernel as deprecated #1233

Merged
merged 1 commit into from
Oct 5, 2023
Merged

specs-go: mark LinuxMemory.Kernel as deprecated #1233

merged 1 commit into from
Oct 5, 2023

Conversation

thaJeztah
Copy link
Member

Support for kernel-memory limits was deprecated in the kernel, and documented as "NOT RECOMMENDED" (or "SHOULD NOT" use) in v1.1.0-rc.1 through commit f02cd4a.

This patch marks the field as deprecated in the go implementation of the spec, so that linters and editors produces a warning and consumers get notified of its status.

@thaJeztah
specs-go: mark LinuxMemory.Kernel as deprecated
cabeea7 
Support for kernel-memory limits was deprecated in the kernel, and documented
as "NOT RECOMMENDED" (or "SHOULD NOT" use) in  v1.1.0-rc.1 through commit
f02cd4a.

This patch marks the field as deprecated in the go implementation of the
spec, so that linters and editors produces a warning and consumers get
notified of its status.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

FWIW I did not (yet) update the KernelTCP field, as there was some discussion about KernelTCP on the linked PR; https://github.com/opencontainers/runtime-spec/pull/1093/files#r589703045, as well as opencontainers/runc#3174 (comment)

https://github.com/torvalds/linux/blob/master/Documentation/admin-guide/cgroup-v1/memory.rst
It looks like this definitely turned to "deprecated"...

The memory.kmem.tcp.limit_in_bytes knob is not listed as deprecated in the document you mention, this is the reason why we have this issue opened. I just checked the recent kernel, and unlike memory.kmem.limit_in_bytes (which was deprecated by the kernel commit 58056f77502f3567b760c9a8fc8d2e9081515b2d), the kmem.tcp is still accepted by the kernel.

Given the fact that cgroup v1 is going to be deprecated sooner or later, and we receive no bug reports about inability to set kmem.tcp limit, I guess we can leave this as is for now.

@AkihiroSuda AkihiroSuda added this to the v1.1.1 milestone Sep 29, 2023
giuseppe
giuseppe approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member Author

If people have suggestions for the KernelTCP field, and proper wording for that, also let me know (happy to open a PR for that as well)

@hqhq hqhq merged commit 6331715 into opencontainers:main Oct 5, 2023
3 checks passed
@thaJeztah thaJeztah deleted the go_kmem_deprecated branch October 5, 2023 07:18
@utam0k utam0k mentioned this pull request Jan 5, 2024
Merged
12 tasks
@kolyshkin kolyshkin mentioned this pull request Feb 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giuseppe giuseppe giuseppe approved these changes

@hqhq hqhq hqhq approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@crosbymichael crosbymichael Awaiting requested review from crosbymichael crosbymichael is a code owner

@cyphar cyphar Awaiting requested review from cyphar cyphar is a code owner

@dqminh dqminh Awaiting requested review from dqminh dqminh is a code owner

@kolyshkin kolyshkin Awaiting requested review from kolyshkin kolyshkin is a code owner

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@tianon tianon Awaiting requested review from tianon tianon is a code owner

@vbatts vbatts Awaiting requested review from vbatts

@utam0k utam0k Awaiting requested review from utam0k utam0k is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@thaJeztah @giuseppe @hqhq @AkihiroSuda