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.

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

cgroup: Add support for memory.kmem.tcp.limit_in_bytes #235

Merged
merged 1 commit into from
Dec 15, 2015
Merged

cgroup: Add support for memory.kmem.tcp.limit_in_bytes #235

merged 1 commit into from
Dec 15, 2015

Conversation

yangdongsheng
Copy link

Signed-off-by: Dongsheng Yang yangds.fnst@cn.fujitsu.com

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
@vbatts
Copy link
Member

vbatts commented Oct 28, 2015

while i'm not opposed to this particularly, It does circle back to the conversation about requiring a field for every kernel parameter that exists.

@vishh
Copy link
Contributor

vishh commented Oct 28, 2015

Yes. Frankly I don't see much value in this unless we provide a higher level abstraction like that of lmctfy.

@vishh
Copy link
Contributor

vishh commented Oct 28, 2015

There is an on-going discussion about cgroups support in the mailing list. Lets hold on to this PR until that thread comes to a conclusion.
Sorry for the trouble @yangdongsheng!

@yangdongsheng
Copy link
Author

@vishh That's fine. I will read the discussion then. Thanx for your review. :)

@vbatts
Copy link
Member

vbatts commented Dec 14, 2015

@vishh thoughts on revisiting this? or proposal on better abstraction?

@vishh
Copy link
Contributor

vishh commented Dec 14, 2015

@vbatts: Cgroup parameters have to be pointers. That is the current agreement. This PR requires that change, otherwise LGTM!

@vishh vishh assigned vbatts and unassigned vishh Dec 14, 2015
@hqhq
Copy link
Contributor

hqhq commented Dec 15, 2015

LGTM, @vishh Do you mind merge this and rebase in #233, this can accelerate the process :)

@yangdongsheng
Copy link
Author

@hqhq Thanx, that's good point. I agree with that.

@vishh
Copy link
Contributor

vishh commented Dec 15, 2015

SGTM.

vishh added a commit that referenced this pull request Dec 15, 2015
cgroup: Add support for memory.kmem.tcp.limit_in_bytes
@vishh vishh merged commit 56b8875 into opencontainers:master Dec 15, 2015
@@ -201,6 +201,8 @@ The following parameters can be specified to setup the controller:

* **`kernel`** *(uint64, optional)* - sets hard limit for kernel memory

* **`kernelTCP`** *(uint64, optional)* - sets hard limit for kernel memory in tcp using
Copy link
Contributor

Choose a reason for hiding this comment

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

using… what? We still have this unfinished sentence in the current tip.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops.. @wking Can you post a PR to fix this ?

Copy link
Contributor

Choose a reason for hiding this comment

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

On Mon, Apr 11, 2016 at 03:54:09PM -0700, Vish Kannan wrote:

+* kernelTCP (uint64, optional) - sets hard limit for kernel memory in tcp using

Oops.. @wking Can you post a PR to fix this ?

@vbatts just pushed c1612afd4e7326 in #370 which should cover this.

Copy link
Contributor

@wking wking Apr 11, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

i didn't address this comment

On Mon, Apr 11, 2016 at 6:59 PM, W. Trevor King notifications@github.com
wrote:

In runtime-config-linux.md
#235 (comment)
:

@@ -201,6 +201,8 @@ The following parameters can be specified to setup the controller:

  • kernel (uint64, optional) - sets hard limit for kernel memory

+* kernelTCP (uint64, optional) - sets hard limit for kernel memory in tcp using

On Mon, Apr 11, 2016 at 03:54:09PM -0700, Vish Kannan wrote: > +*
kernelTCP (uint64, optional) - sets hard limit for kernel memory in
tcp using Oops.. @wking https://github.com/wking Can you post a PR to
fix this ?
@vbatts https://github.com/vbatts just pushed c1612af
c1612af
d4e7326
d4e7326
in #370 #370 which
should cover this.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/runtime-spec/pull/235/files/e9a6d94848a3be8a65bc68c3b2c52639041504ed#r59296300

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Apr 11, 2016
Avoid the dangling 'using' from e9a6d94 (cgroup: Add support for
memory.kmem.tcp.limit_in_bytes, 2015-10-26, opencontainers#235).  This looks like
it's aimed at memory.kmem.tcp.limit_in_bytes [1], so I've tried to
echo the kernel docs by mentioning buffer memory.  I'd personally
prefer linking to the kernel docs and mentioning
memory.kmem.tcp.limit_in_bytes, but that seemed like too big of a
break from the existing style for this commit.

[1]: https://kernel.org/doc/Documentation/cgroup-v1/memory.txt

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Apr 11, 2016
Avoid the dangling 'using' from e9a6d94 (cgroup: Add support for
memory.kmem.tcp.limit_in_bytes, 2015-10-26, opencontainers#235).  I've tried to echo
the kernel docs by mentioning buffer memory [1].  I'd personally
prefer linking to the kernel docs and mentioning
memory.kmem.tcp.limit_in_bytes, but that seemed like too big of a
break from the existing style for this commit.

[1]: https://kernel.org/doc/Documentation/cgroup-v1/memory.txt

Signed-off-by: W. Trevor King <wking@tremily.us>
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
Avoid the dangling 'using' from e9a6d94 (cgroup: Add support for
memory.kmem.tcp.limit_in_bytes, 2015-10-26, opencontainers#235).  I've tried to echo
the kernel docs by mentioning buffer memory [1].  I'd personally
prefer linking to the kernel docs and mentioning
memory.kmem.tcp.limit_in_bytes, but that seemed like too big of a
break from the existing style for this commit.

[1]: https://kernel.org/doc/Documentation/cgroup-v1/memory.txt

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

5 participants