Skip to content

Conversation

@tiborvass
Copy link
Contributor

@tiborvass tiborvass commented Jun 23, 2017

This fixes a runc bug that is very annoying on kernels such as RHEL 7.2
that have the following bug: when setting memory.kmem.limit_in_bytes to
a bigger value than the default max value (bigger than 2^63-1), the
buggy kernel sets it to 0, instead of setting it to the max value. This
results in the process being unable to run with a 0 byte limit on kmem.

That kernel bug exposes a runc bug introduced in 8430cc4, that casted an
int64(-1) to uint64 in EnableKernelMemoryAccounting, which is called
every time the user specifies any memory-related cgroup setting as
determined by the memoryAssigned function.

This patch is the smallest fix for this clearly unwanted behavior.
Reverting 8430cc4, although desirable because int64 values are casted to
uint64 in many other places introducing similar bugs, it is out of scope
for this patch since it would require a spec change.

Signed-off-by: Tibor Vass [email protected]

Related #1492 #1375
cc @crosbymichael @justincormack @tonistiigi @hqhq

This fixes a runc bug that is very annoying on kernels such as RHEL 7.2
that have the following bug: when setting memory.kmem.limit_in_bytes to
a bigger value than the default max value (bigger than 2^63-1), the
buggy kernel sets it to 0, instead of setting it to the max value. This
results in the process being unable to run with a 0 byte limit on kmem.

That kernel bug exposes a runc bug introduced in 8430cc4, that casted an
int64(-1) to uint64 in EnableKernelMemoryAccounting, which is called
every time the user specifies any memory-related cgroup setting as
determined by the memoryAssigned function.

This patch is the smallest fix for this clearly unwanted behavior.
Reverting 8430cc4, although desirable because int64 values are casted to
uint64 in many other places introducing similar bugs, it is out of scope
for this patch since it would require a spec change.

Signed-off-by: Tibor Vass <[email protected]>
@crosbymichael
Copy link
Member

Lets revert the entire commit for now in your other PR.

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.

2 participants