Merge pull request #261 from kolyshkin/19.03-aufs-lock

[19.03 backport ENGCORE-831] aufs optimizations #39107

daemon/graphdriver/aufs/aufs.go

@@ -72,7 +72,6 @@ func init() {
 
 // Driver contains information about the filesystem mounted.
 type Driver struct {
-	sync.Mutex
 	root          string
 	uidMaps       []idtools.IDMap
 	gidMaps       []idtools.IDMap
@@ -81,6 +80,7 @@ type Driver struct {
 	pathCache     map[string]string
 	naiveDiff     graphdriver.DiffDriver
 	locker        *locker.Locker
+	mntL          sync.Mutex
 }
 
 // Init returns a new AUFS driver.
@@ -327,11 +327,11 @@ func (a *Driver) Remove(id string) error {
 			break
 		}
 
-		if err != unix.EBUSY {
-			return errors.Wrapf(err, "aufs: unmount error: %s", mountpoint)
+		if errors.Cause(err) != unix.EBUSY {
+			return errors.Wrap(err, "aufs: unmount error")
 		}
 		if retries >= 5 {
-			return errors.Wrapf(err, "aufs: unmount error after retries: %s", mountpoint)
+			return errors.Wrap(err, "aufs: unmount error after retries")
 		}
 		// If unmount returns EBUSY, it could be a transient error. Sleep and retry.
 		retries++
@@ -437,7 +437,7 @@ func (a *Driver) Put(id string) error {
 
 	err := a.unmount(m)
 	if err != nil {
-		logger.Debugf("Failed to unmount %s aufs: %v", id, err)
+		logger.WithError(err).WithField("method", "Put()").Warn()
 	}
 	return err
 }
@@ -547,9 +547,6 @@ func (a *Driver) getParentLayerPaths(id string) ([]string, error) {
 }
 
 func (a *Driver) mount(id string, target string, mountLabel string, layers []string) error {
-	a.Lock()
-	defer a.Unlock()
-
 	// If the id is mounted or we get an error return
 	if mounted, err := a.mounted(target); err != nil || mounted {
 		return err
@@ -564,9 +561,6 @@ func (a *Driver) mount(id string, target string, mountLabel string, layers []str
 }
 
 func (a *Driver) unmount(mountPath string) error {
-	a.Lock()
-	defer a.Unlock()
-
 	if mounted, err := a.mounted(mountPath); err != nil || !mounted {
 		return err
 	}
@@ -579,23 +573,20 @@ func (a *Driver) mounted(mountpoint string) (bool, error) {
 
 // Cleanup aufs and unmount all mountpoints
 func (a *Driver) Cleanup() error {
-	var dirs []string
-	if err := filepath.Walk(a.mntPath(), func(path string, info os.FileInfo, err error) error {
+	dir := a.mntPath()
+	files, err := ioutil.ReadDir(dir)
 	if err != nil {
-			return err
+		return errors.Wrap(err, "aufs readdir error")
 	}
-		if !info.IsDir() {
-			return nil
-		}
-		dirs = append(dirs, path)
-		return nil
-	}); err != nil {
-		return err
+	for _, f := range files {
+		if !f.IsDir() {
+			continue
 		}
 
-	for _, m := range dirs {
+		m := path.Join(dir, f.Name())
+
 		if err := a.unmount(m); err != nil {
-			logger.Debugf("error unmounting %s: %s", m, err)
+			logger.WithError(err).WithField("method", "Cleanup()").Warn()
 		}
 	}
 	return mount.RecursiveUnmount(a.root)
@@ -604,7 +595,7 @@ func (a *Driver) Cleanup() error {
 func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err error) {
 	defer func() {
 		if err != nil {
-			Unmount(target)
+			mount.Unmount(target)
 		}
 	}()
 
@@ -632,14 +623,29 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro
 		opts += ",dirperm1"
 	}
 	data := label.FormatMountLabel(fmt.Sprintf("%s,%s", string(b[:bp]), opts), mountLabel)
-	if err = unix.Mount("none", target, "aufs", 0, data); err != nil {
+	a.mntL.Lock()
+	err = unix.Mount("none", target, "aufs", 0, data)
+	a.mntL.Unlock()
+	if err != nil {
+		err = errors.Wrap(err, "mount target="+target+" data="+data)
 		return
 	}
 
+	for index < len(ro) {
+		bp = 0
 		for ; index < len(ro); index++ {
-		layer := fmt.Sprintf(":%s=ro+wh", ro[index])
-		data := label.FormatMountLabel(fmt.Sprintf("append%s", layer), mountLabel)
-		if err = unix.Mount("none", target, "aufs", unix.MS_REMOUNT, data); err != nil {
+			layer := fmt.Sprintf("append:%s=ro+wh,", ro[index])
+			if bp+len(layer) > len(b) {
+				break
+			}
+			bp += copy(b[bp:], layer)
+		}
+		data := label.FormatMountLabel(string(b[:bp]), mountLabel)
+		a.mntL.Lock()
+		err = unix.Mount("none", target, "aufs", unix.MS_REMOUNT, data)
+		a.mntL.Unlock()
+		if err != nil {
+			err = errors.Wrap(err, "mount target="+target+" flags=MS_REMOUNT data="+data)
 			return
 		}
 	}

daemon/graphdriver/aufs/mount.go

@@ -4,14 +4,38 @@ package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs"
 
 import (
 	"os/exec"
+	"syscall"
 
-	"golang.org/x/sys/unix"
+	"github.com/docker/docker/pkg/mount"
 )
 
 // Unmount the target specified.
 func Unmount(target string) error {
-	if err := exec.Command("auplink", target, "flush").Run(); err != nil {
-		logger.WithError(err).Warnf("Couldn't run auplink before unmount %s", target)
+	const (
+		EINVAL  = 22 // if auplink returns this,
+		retries = 3  // retry a few times
+	)
+
+	for i := 0; ; i++ {
+		out, err := exec.Command("auplink", target, "flush").CombinedOutput()
+		if err == nil {
+			break
 		}
-	return unix.Unmount(target, 0)
+		rc := 0
+		if exiterr, ok := err.(*exec.ExitError); ok {
+			if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
+				rc = status.ExitStatus()
+			}
+		}
+		if i >= retries || rc != EINVAL {
+			logger.WithError(err).WithField("method", "Unmount").Warnf("auplink flush failed: %s", out)
+			break
+		}
+		// auplink failed to find target in /proc/self/mounts because
+		// kernel can't guarantee continuity while reading from it
+		// while mounts table is being changed
+		logger.Debugf("auplink flush error (retrying %d/%d): %s", i+1, retries, out)
+	}
+
+	return mount.Unmount(target)
 }