From f31014197cbe9438cc956ed12c47093a0324c82d Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 12 Feb 2016 11:01:45 -0500 Subject: [PATCH] Add finer-grained locking for aufs ``` benchmark old ns/op new ns/op delta BenchmarkConcurrentAccess-8 10269529748 26834747 -99.74% benchmark old allocs new allocs delta BenchmarkConcurrentAccess-8 309948 7232 -97.67% benchmark old bytes new bytes delta BenchmarkConcurrentAccess-8 23943576 1578441 -93.41% ``` Signed-off-by: Brian Goff --- daemon/graphdriver/aufs/aufs.go | 94 ++++++++++++++++++++++---------- daemon/graphdriver/aufs/mount.go | 2 +- 2 files changed, 67 insertions(+), 29 deletions(-) diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index e03576aa8a..529d44c265 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -66,6 +66,7 @@ func init() { type data struct { referenceCount int path string + sync.Mutex } // Driver contains information about the filesystem mounted. @@ -76,7 +77,7 @@ type Driver struct { root string uidMaps []idtools.IDMap gidMaps []idtools.IDMap - sync.Mutex // Protects concurrent modification to active + globalLock sync.Mutex // Protects concurrent modification to active active map[string]*data } @@ -202,7 +203,20 @@ func (a *Driver) Exists(id string) bool { // Create three folders for each id // mnt, layers, and diff func (a *Driver) Create(id, parent, mountLabel string) error { - if err := a.createDirsFor(id); err != nil { + m := a.getActive(id) + m.Lock() + + var err error + defer func() { + a.globalLock.Lock() + if err != nil { + delete(a.active, id) + } + a.globalLock.Unlock() + m.Unlock() + }() + + if err = a.createDirsFor(id); err != nil { return err } // Write the layers metadata @@ -213,23 +227,22 @@ func (a *Driver) Create(id, parent, mountLabel string) error { defer f.Close() if parent != "" { - ids, err := getParentIds(a.rootPath(), parent) + var ids []string + ids, err = getParentIds(a.rootPath(), parent) if err != nil { return err } - if _, err := fmt.Fprintln(f, parent); err != nil { + if _, err = fmt.Fprintln(f, parent); err != nil { return err } for _, i := range ids { - if _, err := fmt.Fprintln(f, i); err != nil { + if _, err = fmt.Fprintln(f, i); err != nil { return err } } } - a.Lock() - a.active[id] = &data{} - a.Unlock() + return nil } @@ -253,11 +266,10 @@ func (a *Driver) createDirsFor(id string) error { // Remove will unmount and remove the given id. func (a *Driver) Remove(id string) error { - // Protect the a.active from concurrent access - a.Lock() - defer a.Unlock() + m := a.getActive(id) + m.Lock() + defer m.Unlock() - m := a.active[id] if m != nil { if m.referenceCount > 0 { return nil @@ -288,9 +300,9 @@ func (a *Driver) Remove(id string) error { return err } if m != nil { - a.Lock() + a.globalLock.Lock() delete(a.active, id) - a.Unlock() + a.globalLock.Unlock() } return nil } @@ -298,21 +310,36 @@ func (a *Driver) Remove(id string) error { // Get returns the rootfs path for the id. // This will mount the dir at it's given path func (a *Driver) Get(id, mountLabel string) (string, error) { - // Protect the a.active from concurrent access - a.Lock() - defer a.Unlock() - - m := a.active[id] - if m == nil { - m = &data{} - a.active[id] = m - } + m := a.getActive(id) + m.Lock() + defer m.Unlock() parents, err := a.getParentLayerPaths(id) if err != nil && !os.IsNotExist(err) { return "", err } + var parentLocks []*data + a.globalLock.Lock() + for _, p := range parents { + parentM, exists := a.active[p] + if !exists { + parentM = &data{} + a.active[p] = parentM + } + parentLocks = append(parentLocks, parentM) + } + a.globalLock.Unlock() + + for _, l := range parentLocks { + l.Lock() + } + defer func() { + for _, l := range parentLocks { + l.Unlock() + } + }() + // If a dir does not have a parent ( no layers )do not try to mount // just return the diff path to the data m.path = path.Join(a.rootPath(), "diff", id) @@ -328,13 +355,24 @@ func (a *Driver) Get(id, mountLabel string) (string, error) { return m.path, nil } +func (a *Driver) getActive(id string) *data { + // Protect the a.active from concurrent access + a.globalLock.Lock() + m, exists := a.active[id] + if !exists { + m = &data{} + a.active[id] = m + } + a.globalLock.Unlock() + return m +} + // Put unmounts and updates list of active mounts. func (a *Driver) Put(id string) error { - // Protect the a.active from concurrent access - a.Lock() - defer a.Unlock() + m := a.getActive(id) + m.Lock() + defer m.Unlock() - m := a.active[id] if m == nil { // but it might be still here if a.Exists(id) { @@ -346,6 +384,7 @@ func (a *Driver) Put(id string) error { } return nil } + if count := m.referenceCount; count > 1 { m.referenceCount = count - 1 } else { @@ -354,7 +393,6 @@ func (a *Driver) Put(id string) error { if ids != nil && len(ids) > 0 { a.unmount(m) } - delete(a.active, id) } return nil } diff --git a/daemon/graphdriver/aufs/mount.go b/daemon/graphdriver/aufs/mount.go index d7e9bf9fd7..36fa62e41b 100644 --- a/daemon/graphdriver/aufs/mount.go +++ b/daemon/graphdriver/aufs/mount.go @@ -12,7 +12,7 @@ import ( // Unmount the target specified. func Unmount(target string) error { if err := exec.Command("auplink", target, "flush").Run(); err != nil { - logrus.Errorf("Couldn't run auplink before unmount: %s", err) + logrus.Errorf("Couldn't run auplink before unmount %s: %s", target, err) } if err := syscall.Unmount(target, 0); err != nil { return err