search

google / lmctfy

lmctfy is the open source version of Google’s container stack, which provides Linux application containers.
Other
3.41k stars 237 forks source link

Replace kint64max with -1 for limit_in_bytes. #9

Closed abhishekrai closed 11 years ago

abhishekrai commented 11 years ago

The kernel rejects kint64max values with EINVAL on 3.8.0-19 when writing to "memory.limit_in_bytes". It accepts values up to (kint64max & ~(PAGE_SIZE-1)). My suspicion is that when lmctfy passes kint64max, somewhere its being parsed as an int64 and also being aligned to the next page boundary. This is not surprising given that I can reproduce this without lmctfy also --

echo 9223372036854775807 >/dev/cgroup/memory/test/memory.limit_in_bytes bash: echo: write error: Invalid argument

It seems that -1 is the proper way of telling the kernel to set max limit (from looking at mem_cgroup_write() in memcontrol.cc and Documentation/cgroups/memory.txt).

echo -1 >/dev/cgroup/memory/test/memory.limit_in_bytes cat /dev/cgroup/memory/test/memory.limit_in_bytes 9223372036854775807

vmarmol commented 11 years ago

Hi Abhishek, thanks for the patch! Only two comments:

abhishekrai commented 11 years ago

Thanks for the quick review Victor!

On Mon, Oct 28, 2013 at 7:36 AM, Victor Marmol notifications@github.comwrote:

Hi Abhishek, thanks for the patch! Only two comments:

  • Please make sure to also change the unit test in memory_resource_handler_test.cc (and ensure everything passes).

Done

BTW, I was wondering how do you validate or test against actual kernels? For example, is there a test suite that one can run against their kernel to check if lmctfy is working correctly.

Done

Thanks, Abhishek

-

— Reply to this email directly or view it on GitHubhttps://github.com/google/lmctfy/pull/9#issuecomment-27216504 .

rjnagal commented 11 years ago

On Mon, Oct 28, 2013 at 6:50 PM, Abhishek Rai notifications@github.comwrote:

Thanks for the quick review Victor!

On Mon, Oct 28, 2013 at 7:36 AM, Victor Marmol notifications@github.comwrote:

Hi Abhishek, thanks for the patch! Only two comments:

  • Please make sure to also change the unit test in memory_resource_handler_test.cc (and ensure everything passes).

Done

BTW, I was wondering how do you validate or test against actual kernels? For example, is there a test suite that one can run against their kernel to check if lmctfy is working correctly.

We don't have one for any standard ubuntu kernels. We are planning to add that in two steps:

  1. Add a lmctfy check command that will return all available features from the kernel and gcontain build under use. This will validate availability of most interfaces.
  2. The actual integration test that validates available features.

Hoping to get 1. in soon.

-r

Done

Thanks, Abhishek

— Reply to this email directly or view it on GitHub< https://github.com/google/lmctfy/pull/9#issuecomment-27216504> .

— Reply to this email directly or view it on GitHubhttps://github.com/google/lmctfy/pull/9#issuecomment-27272813 .

abhishekrai commented 11 years ago

On Mon, Oct 28, 2013 at 7:21 PM, Rohit Jnagal notifications@github.comwrote:

On Mon, Oct 28, 2013 at 6:50 PM, Abhishek Rai notifications@github.comwrote:

Thanks for the quick review Victor!

On Mon, Oct 28, 2013 at 7:36 AM, Victor Marmol notifications@github.comwrote:

Hi Abhishek, thanks for the patch! Only two comments:

  • Please make sure to also change the unit test in memory_resource_handler_test.cc (and ensure everything passes).

Done

BTW, I was wondering how do you validate or test against actual kernels? For example, is there a test suite that one can run against their kernel to check if lmctfy is working correctly.

We don't have one for any standard ubuntu kernels. We are planning to add that in two steps:

  1. Add a lmctfy check command that will return all available features from the kernel and gcontain build under use. This will validate availability of most interfaces.
  2. The actual integration test that validates available features.

Hoping to get 1. in soon.

Thanks Rohit, that sounds great.

-r

Done

Thanks, Abhishek

— Reply to this email directly or view it on GitHub< https://github.com/google/lmctfy/pull/9#issuecomment-27216504> .

— Reply to this email directly or view it on GitHub< https://github.com/google/lmctfy/pull/9#issuecomment-27272813> .

— Reply to this email directly or view it on GitHubhttps://github.com/google/lmctfy/pull/9#issuecomment-27273897 .

vmarmol commented 11 years ago

Thanks for the change!