Skip to content

Commit 4fe70f3

Browse files
thaJeztahthaJeztah
committed
v1: reduce duplicated code
Code for handling Tasks and Processes, except for "cgroup.procs" vs "tasks", and Tasks being a different type. This patch: - Makes the Task type an alias for Process (as they're identical otherwise) - Adds a `processType` type for the `cgroupProcs` and `cgroupTasks` consts. This type is an alias for "string", and mostly for clarity (indicating it's an 'enum'). - Merges the `cgroup.add()` and `cgroup.addTask()` functions, adding a `pType` argument. - Merges the `cgroup.processes()` and `cgroup.tasks()` functions, adding a `pType` argument. - Merges the `readPids()` and `readTasksPids()` utilities, adding a `pType` argument. - Move locking and validation into `cgroup.add()`. All places using `cgroup.add()` were taking a lock and doing this validation, so looks we can move this code into `cgroup.add()` itself. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent e6ce30d commit 4fe70f3

3 files changed

Lines changed: 31 additions & 127 deletions

File tree

‎cgroup.go‎

Lines changed: 19 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -151,46 +151,20 @@ func (c *cgroup) Subsystems() []Subsystem {
151151

152152
// Add moves the provided process into the new cgroup
153153
func (c *cgroup) Add(process Process) error {
154-
if process.Pid <= 0 {
155-
return ErrInvalidPid
156-
}
157-
c.mu.Lock()
158-
defer c.mu.Unlock()
159-
if c.err != nil {
160-
return c.err
161-
}
162-
return c.add(process)
154+
return c.add(process, cgroupProcs)
163155
}
164156

165157
// AddProc moves the provided process id into the new cgroup
166158
func (c *cgroup) AddProc(pid uint64) error {
167-
c.mu.Lock()
168-
defer c.mu.Unlock()
169-
if c.err != nil {
170-
return c.err
171-
}
172-
return c.Add(Process{Pid: int(pid)})
173-
}
174-
175-
func (c *cgroup) add(process Process) error {
176-
for _, s := range pathers(c.subsystems) {
177-
p, err := c.path(s.Name())
178-
if err != nil {
179-
return err
180-
}
181-
if err := retryingWriteFile(
182-
filepath.Join(s.Path(p), cgroupProcs),
183-
[]byte(strconv.Itoa(process.Pid)),
184-
defaultFilePerm,
185-
); err != nil {
186-
return err
187-
}
188-
}
189-
return nil
159+
return c.add(Process{Pid: int(pid)}, cgroupProcs)
190160
}
191161

192162
// AddTask moves the provided tasks (threads) into the new cgroup
193163
func (c *cgroup) AddTask(process Process) error {
164+
return c.add(process, cgroupTasks)
165+
}
166+
167+
func (c *cgroup) add(process Process, pType procType) error {
194168
if process.Pid <= 0 {
195169
return ErrInvalidPid
196170
}
@@ -199,20 +173,17 @@ func (c *cgroup) AddTask(process Process) error {
199173
if c.err != nil {
200174
return c.err
201175
}
202-
return c.addTask(process)
203-
}
204-
205-
func (c *cgroup) addTask(process Process) error {
206176
for _, s := range pathers(c.subsystems) {
207177
p, err := c.path(s.Name())
208178
if err != nil {
209179
return err
210180
}
211-
if err := retryingWriteFile(
212-
filepath.Join(s.Path(p), cgroupTasks),
181+
err = retryingWriteFile(
182+
filepath.Join(s.Path(p), pType),
213183
[]byte(strconv.Itoa(process.Pid)),
214184
defaultFilePerm,
215-
); err != nil {
185+
)
186+
if err != nil {
216187
return err
217188
}
218189
}
@@ -336,39 +307,7 @@ func (c *cgroup) Processes(subsystem Name, recursive bool) ([]Process, error) {
336307
if c.err != nil {
337308
return nil, c.err
338309
}
339-
return c.processes(subsystem, recursive)
340-
}
341-
342-
func (c *cgroup) processes(subsystem Name, recursive bool) ([]Process, error) {
343-
s := c.getSubsystem(subsystem)
344-
sp, err := c.path(subsystem)
345-
if err != nil {
346-
return nil, err
347-
}
348-
path := s.(pather).Path(sp)
349-
var processes []Process
350-
err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error {
351-
if err != nil {
352-
return err
353-
}
354-
if !recursive && info.IsDir() {
355-
if p == path {
356-
return nil
357-
}
358-
return filepath.SkipDir
359-
}
360-
dir, name := filepath.Split(p)
361-
if name != cgroupProcs {
362-
return nil
363-
}
364-
procs, err := readPids(dir, subsystem)
365-
if err != nil {
366-
return err
367-
}
368-
processes = append(processes, procs...)
369-
return nil
370-
})
371-
return processes, err
310+
return c.processes(subsystem, recursive, cgroupProcs)
372311
}
373312

374313
// Tasks returns the tasks running inside the cgroup along
@@ -379,17 +318,17 @@ func (c *cgroup) Tasks(subsystem Name, recursive bool) ([]Task, error) {
379318
if c.err != nil {
380319
return nil, c.err
381320
}
382-
return c.tasks(subsystem, recursive)
321+
return c.processes(subsystem, recursive, cgroupTasks)
383322
}
384323

385-
func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) {
324+
func (c *cgroup) processes(subsystem Name, recursive bool, pType procType) ([]Process, error) {
386325
s := c.getSubsystem(subsystem)
387326
sp, err := c.path(subsystem)
388327
if err != nil {
389328
return nil, err
390329
}
391330
path := s.(pather).Path(sp)
392-
var tasks []Task
331+
var processes []Process
393332
err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error {
394333
if err != nil {
395334
return err
@@ -401,17 +340,17 @@ func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) {
401340
return filepath.SkipDir
402341
}
403342
dir, name := filepath.Split(p)
404-
if name != cgroupTasks {
343+
if name != pType {
405344
return nil
406345
}
407-
procs, err := readTasksPids(dir, subsystem)
346+
procs, err := readPids(dir, subsystem, pType)
408347
if err != nil {
409348
return err
410349
}
411-
tasks = append(tasks, procs...)
350+
processes = append(processes, procs...)
412351
return nil
413352
})
414-
return tasks, err
353+
return processes, err
415354
}
416355

417356
// Freeze freezes the entire cgroup and all the processes inside it
@@ -521,7 +460,7 @@ func (c *cgroup) MoveTo(destination Cgroup) error {
521460
return c.err
522461
}
523462
for _, s := range c.subsystems {
524-
processes, err := c.processes(s.Name(), true)
463+
processes, err := c.processes(s.Name(), true, cgroupProcs)
525464
if err != nil {
526465
return err
527466
}

‎control.go‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ import (
2323
specs "github.com/opencontainers/runtime-spec/specs-go"
2424
)
2525

26+
type procType = string
27+
2628
const (
27-
cgroupProcs = "cgroup.procs"
28-
cgroupTasks = "tasks"
29-
defaultDirPerm = 0755
29+
cgroupProcs procType = "cgroup.procs"
30+
cgroupTasks procType = "tasks"
31+
defaultDirPerm = 0755
3032
)
3133

3234
// defaultFilePerm is a var so that the test framework can change the filemode
@@ -37,22 +39,15 @@ const (
3739
var defaultFilePerm = os.FileMode(0)
3840

3941
type Process struct {
40-
// Subsystem is the name of the subsystem that the process is in
42+
// Subsystem is the name of the subsystem that the process / task is in.
4143
Subsystem Name
42-
// Pid is the process id of the process
44+
// Pid is the process id of the process / task.
4345
Pid int
44-
// Path is the full path of the subsystem and location that the process is in
46+
// Path is the full path of the subsystem and location that the process / task is in.
4547
Path string
4648
}
4749

48-
type Task struct {
49-
// Subsystem is the name of the subsystem that the task is in
50-
Subsystem Name
51-
// Pid is the process id of the task
52-
Pid int
53-
// Path is the full path of the subsystem and location that the task is in
54-
Path string
55-
}
50+
type Task = Process
5651

5752
// Cgroup handles interactions with the individual groups to perform
5853
// actions on them as them main interface to this cgroup package

‎utils.go‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ func remove(path string) error {
164164
return fmt.Errorf("cgroups: unable to remove path %q", path)
165165
}
166166

167-
// readPids will read all the pids of processes in a cgroup by the provided path
168-
func readPids(path string, subsystem Name) ([]Process, error) {
169-
f, err := os.Open(filepath.Join(path, cgroupProcs))
167+
// readPids will read all the pids of processes or tasks in a cgroup by the provided path
168+
func readPids(path string, subsystem Name, pType procType) ([]Process, error) {
169+
f, err := os.Open(filepath.Join(path, pType))
170170
if err != nil {
171171
return nil, err
172172
}
@@ -195,36 +195,6 @@ func readPids(path string, subsystem Name) ([]Process, error) {
195195
return out, nil
196196
}
197197

198-
// readTasksPids will read all the pids of tasks in a cgroup by the provided path
199-
func readTasksPids(path string, subsystem Name) ([]Task, error) {
200-
f, err := os.Open(filepath.Join(path, cgroupTasks))
201-
if err != nil {
202-
return nil, err
203-
}
204-
defer f.Close()
205-
var (
206-
out []Task
207-
s = bufio.NewScanner(f)
208-
)
209-
for s.Scan() {
210-
if t := s.Text(); t != "" {
211-
pid, err := strconv.Atoi(t)
212-
if err != nil {
213-
return nil, err
214-
}
215-
out = append(out, Task{
216-
Pid: pid,
217-
Subsystem: subsystem,
218-
Path: path,
219-
})
220-
}
221-
}
222-
if err := s.Err(); err != nil {
223-
return nil, err
224-
}
225-
return out, nil
226-
}
227-
228198
func hugePageSizes() ([]string, error) {
229199
var (
230200
pageSizes []string

0 commit comments

Comments
 (0)