-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
libct/cg: support SCHED_IDLE for runc cgroupfs #3377
Conversation
d06fe5a
to
f9ca48b
Compare
482bccd
to
fa7fb14
Compare
@kolyshkin PTAL |
c918a6c
to
b108c38
Compare
libcontainer/cgroups/fs2/cpu.go
Outdated
if r.CpuIdle != nil { | ||
if err := cgroups.WriteFile(dirPath, "cpu.idle", strconv.FormatInt(*r.CpuIdle, 10)); err != nil { | ||
return err | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move this below to after if !isCpuSet(r)
.
libcontainer/configs/cgroup_linux.go
Outdated
@@ -84,6 +84,10 @@ type Resources struct { | |||
// MEM to use | |||
CpusetMems string `json:"cpuset_mems"` | |||
|
|||
//nolint:revive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of ignoring revive warning, call it CPUIdle
(hopefully we'll rename the rest of Cpu*
fields at some point).
libcontainer/specconv/spec_linux.go
Outdated
@@ -733,6 +733,7 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi | |||
if r.CPU.Quota != nil { | |||
c.Resources.CpuQuota = *r.CPU.Quota | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits, plus can you please make the runtime-spec bump a separate commit (for clarity)?
Rebased to fix CI |
@kolyshkin Can I ask you to review it again? |
Looks good but needs rebase |
@wineway Could you rebase? |
Signed-off-by: wineway <[email protected]>
sorry, I missed last comment; rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
runtime-spec: opencontainers/runtime-spec#1136