Retry file writes on EINTR errors

adunham-stripe · adunham-stripe · commit f2c27b8f97e7 · 2020-07-06T19:39:26.000-07:00
Go 1.14 introduces asynchronous preemption, which results in applications that invoke slow syscalls (e.g. writing to a cgroup file) getting EINTR errors. The Go runtime will fix this in version 1.15 onwards: golang/go#38033 But this fix won't be backported: golang/go#39026 (comment) As such, introduce a 'writeFile' helper function that retries when the error is an EINTR error. Signed-off-by: Andrew Dunham <adunham@stripe.com>
diff --git a/blkio.go b/blkio.go
@@ -20,7 +20,6 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -72,7 +71,7 @@ func (b *blkioController) Create(path string, resources *specs.LinuxResources) e
 	}
 	for _, t := range createBlkioSettings(resources.BlockIO) {
 		if t.value != nil {
-			if err := ioutil.WriteFile(
+			if err := retryingWriteFile(
 				filepath.Join(b.Path(path), fmt.Sprintf("blkio.%s", t.name)),
 				t.format(t.value),
 				defaultFilePerm,
diff --git a/cgroup.go b/cgroup.go
@@ -18,7 +18,6 @@ package cgroups
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -169,7 +168,7 @@ func (c *cgroup) add(process Process) error {
 		if err != nil {
 			return err
 		}
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(s.Path(p), cgroupProcs),
 			[]byte(strconv.Itoa(process.Pid)),
 			defaultFilePerm,
@@ -199,7 +198,7 @@ func (c *cgroup) addTask(process Process) error {
 		if err != nil {
 			return err
 		}
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(s.Path(p), cgroupTasks),
 			[]byte(strconv.Itoa(process.Pid)),
 			defaultFilePerm,
diff --git a/cpu.go b/cpu.go
@@ -19,7 +19,6 @@ package cgroups
 import (
 	"bufio"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -84,7 +83,7 @@ func (c *cpuController) Create(path string, resources *specs.LinuxResources) err
 				value = []byte(strconv.FormatInt(*t.ivalue, 10))
 			}
 			if value != nil {
-				if err := ioutil.WriteFile(
+				if err := retryingWriteFile(
 					filepath.Join(c.Path(path), fmt.Sprintf("cpu.%s", t.name)),
 					value,
 					defaultFilePerm,
diff --git a/cpuset.go b/cpuset.go
@@ -69,7 +69,7 @@ func (c *cpusetController) Create(path string, resources *specs.LinuxResources)
 			},
 		} {
 			if t.value != "" {
-				if err := ioutil.WriteFile(
+				if err := retryingWriteFile(
 					filepath.Join(c.Path(path), fmt.Sprintf("cpuset.%s", t.name)),
 					[]byte(t.value),
 					defaultFilePerm,
@@ -134,7 +134,7 @@ func (c *cpusetController) copyIfNeeded(current, parent string) error {
 		return err
 	}
 	if isEmpty(currentCpus) {
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(current, "cpuset.cpus"),
 			parentCpus,
 			defaultFilePerm,
@@ -143,7 +143,7 @@ func (c *cpusetController) copyIfNeeded(current, parent string) error {
 		}
 	}
 	if isEmpty(currentMems) {
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(current, "cpuset.mems"),
 			parentMems,
 			defaultFilePerm,
diff --git a/devices.go b/devices.go
@@ -18,7 +18,6 @@ package cgroups
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -61,7 +60,7 @@ func (d *devicesController) Create(path string, resources *specs.LinuxResources)
 		if device.Type == "" {
 			device.Type = "a"
 		}
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(d.Path(path), file),
 			[]byte(deviceString(device)),
 			defaultFilePerm,
diff --git a/freezer.go b/freezer.go
@@ -50,7 +50,7 @@ func (f *freezerController) Thaw(path string) error {
 }
 
 func (f *freezerController) changeState(path string, state State) error {
-	return ioutil.WriteFile(
+	return retryingWriteFile(
 		filepath.Join(f.root, path, "freezer.state"),
 		[]byte(strings.ToUpper(string(state))),
 		defaultFilePerm,
diff --git a/hugetlb.go b/hugetlb.go
@@ -17,7 +17,6 @@
 package cgroups
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -57,7 +56,7 @@ func (h *hugetlbController) Create(path string, resources *specs.LinuxResources)
 		return err
 	}
 	for _, limit := range resources.HugepageLimits {
-		if err := ioutil.WriteFile(
+		if err := retryingWriteFile(
 			filepath.Join(h.Path(path), strings.Join([]string{"hugetlb", limit.Pagesize, "limit_in_bytes"}, ".")),
 			[]byte(strconv.FormatUint(limit.Limit, 10)),
 			defaultFilePerm,
diff --git a/memory.go b/memory.go
@@ -20,7 +20,6 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -214,7 +213,7 @@ func (m *memoryController) Create(path string, resources *specs.LinuxResources)
 		// until a limit is set on the cgroup and limit cannot be set once the
 		// cgroup has children, or if there are already tasks in the cgroup.
 		for _, i := range []int64{1, -1} {
-			if err := ioutil.WriteFile(
+			if err := retryingWriteFile(
 				filepath.Join(m.Path(path), "memory.kmem.limit_in_bytes"),
 				[]byte(strconv.FormatInt(i, 10)),
 				defaultFilePerm,
@@ -378,7 +377,7 @@ func (m *memoryController) parseStats(r io.Reader, stat *v1.MemoryStat) error {
 func (m *memoryController) set(path string, settings []memorySettings) error {
 	for _, t := range settings {
 		if t.value != nil {
-			if err := ioutil.WriteFile(
+			if err := retryingWriteFile(
 				filepath.Join(m.Path(path), fmt.Sprintf("memory.%s", t.name)),
 				[]byte(strconv.FormatInt(*t.value, 10)),
 				defaultFilePerm,
@@ -468,7 +467,7 @@ func (m *memoryController) memoryEvent(path string, event MemoryEvent) (uintptr,
 	defer evtFile.Close()
 	data := fmt.Sprintf("%d %d %s", efd, evtFile.Fd(), event.Arg())
 	evctlPath := filepath.Join(root, "cgroup.event_control")
-	if err := ioutil.WriteFile(evctlPath, []byte(data), 0700); err != nil {
+	if err := retryingWriteFile(evctlPath, []byte(data), 0700); err != nil {
 		unix.Close(efd)
 		return 0, err
 	}
diff --git a/net_cls.go b/net_cls.go
@@ -17,7 +17,6 @@
 package cgroups
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -48,7 +47,7 @@ func (n *netclsController) Create(path string, resources *specs.LinuxResources)
 		return err
 	}
 	if resources.Network != nil && resources.Network.ClassID != nil && *resources.Network.ClassID > 0 {
-		return ioutil.WriteFile(
+		return retryingWriteFile(
 			filepath.Join(n.Path(path), "net_cls.classid"),
 			[]byte(strconv.FormatUint(uint64(*resources.Network.ClassID), 10)),
 			defaultFilePerm,
diff --git a/net_prio.go b/net_prio.go
@@ -18,7 +18,6 @@ package cgroups
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -49,7 +48,7 @@ func (n *netprioController) Create(path string, resources *specs.LinuxResources)
 	}
 	if resources.Network != nil {
 		for _, prio := range resources.Network.Priorities {
-			if err := ioutil.WriteFile(
+			if err := retryingWriteFile(
 				filepath.Join(n.Path(path), "net_prio.ifpriomap"),
 				formatPrio(prio.Name, prio.Priority),
 				defaultFilePerm,
diff --git a/pids.go b/pids.go
@@ -50,7 +50,7 @@ func (p *pidsController) Create(path string, resources *specs.LinuxResources) er
 		return err
 	}
 	if resources.Pids != nil && resources.Pids.Limit > 0 {
-		return ioutil.WriteFile(
+		return retryingWriteFile(
 			filepath.Join(p.Path(path), "pids.max"),
 			[]byte(strconv.FormatInt(resources.Pids.Limit, 10)),
 			defaultFilePerm,
diff --git a/rdma.go b/rdma.go
@@ -67,7 +67,7 @@ func (p *rdmaController) Create(path string, resources *specs.LinuxResources) er
 
 	for device, limit := range resources.Rdma {
 		if device != "" && (limit.HcaHandles != nil || limit.HcaObjects != nil) {
-			return ioutil.WriteFile(
+			return retryingWriteFile(
 				filepath.Join(p.Path(path), "rdma.max"),
 				[]byte(createCmdString(device, &limit)),
 				defaultFilePerm,
diff --git a/utils.go b/utils.go
@@ -18,6 +18,7 @@ package cgroups
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -26,6 +27,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 	"time"
 
 	units "github.com/docker/go-units"
@@ -382,3 +384,16 @@ func cleanPath(path string) string {
 	}
 	return filepath.Clean(path)
 }
+
+func retryingWriteFile(path string, data []byte, mode os.FileMode) error {
+	// Retry writes on EINTR; see:
+	//    https://github.com/golang/go/issues/38033
+	for {
+		err := ioutil.WriteFile(path, []byte(data), mode)
+		if err == nil {
+			return nil
+		} else if !errors.Is(err, syscall.EINTR) {
+			return err
+		}
+	}
+}
diff --git a/v2/manager.go b/v2/manager.go
@@ -18,6 +18,7 @@ package v2
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"math"
@@ -149,11 +150,21 @@ func (c *Value) write(path string, perm os.FileMode) error {
 	default:
 		return ErrInvalidFormat
 	}
-	return ioutil.WriteFile(
-		filepath.Join(path, c.filename),
-		data,
-		perm,
-	)
+
+	// Retry writes on EINTR; see:
+	//    https://github.com/golang/go/issues/38033
+	for {
+		err := ioutil.WriteFile(
+			filepath.Join(path, c.filename),
+			data,
+			perm,
+		)
+		if err == nil {
+			return nil
+		} else if !errors.Is(err, syscall.EINTR) {
+			return err
+		}
+	}
 }
 
 func writeValues(path string, values []Value) error {

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ func (f *freezerController) Thaw(path string) error {`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`func (f *freezerController) changeState(path string, state State) error {`
`53`		`- return ioutil.WriteFile(`
	`53`	`+ return retryingWriteFile(`
`54`	`54`	`filepath.Join(f.root, path, "freezer.state"),`
`55`	`55`	`[]byte(strings.ToUpper(string(state))),`
`56`	`56`	`defaultFilePerm,`