]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
lib/file: FileLockSleeper issues
authorOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:23:47 +0000 (11:23 -0700)
committerOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:23:47 +0000 (11:23 -0700)
While waiting for a contended file lock, FileLockSleeper was using
fixed values for how long to sleep between attempts. This can lead to
a "thundering herd" of waiters - several waiters all on the same
cadence. Randomize waiting to avoid any cadence.

Waiting for a lock should wait no longer than the specified time. Fix
This.

open-vm-tools/lib/file/file.c
open-vm-tools/lib/file/fileInt.h
open-vm-tools/lib/file/fileLockPosix.c
open-vm-tools/lib/file/fileLockPrimitive.c
open-vm-tools/lib/include/fileLock.h

index 1ec4f7768a3da501b414278e0720b496db79e0e2..8cfa2902a0023f3c9e7818d3271666b39ce288c6 100644 (file)
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 1998-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 1998-2017 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -2124,27 +2124,44 @@ FileSimpleRandom(void)
  */
 
 uint32
-FileSleeper(uint32 msecMinSleepTime,  // IN:
-            uint32 msecMaxSleepTime)  // IN:
+FileSleeper(uint32 minSleepTimeMsec,  // IN:
+            uint32 maxSleepTimeMsec)  // IN:
 {
    uint32 variance;
-   uint32 msecActualSleepTime;
+   uint32 actualSleepTimeMsec;
+#if defined(_WIN32)
+   uint32 totalSleepTimeMsec;
+#endif
 
-   ASSERT(msecMinSleepTime <= msecMaxSleepTime);
+   ASSERT(minSleepTimeMsec <= maxSleepTimeMsec);
 
-   variance = msecMaxSleepTime - msecMinSleepTime;
+   variance = maxSleepTimeMsec - minSleepTimeMsec;
 
    if (variance == 0) {
-      msecActualSleepTime = msecMinSleepTime;
+      actualSleepTimeMsec = minSleepTimeMsec;
    } else {
       float fpRand = ((float) FileSimpleRandom()) / ((float) ~((uint32) 0));
 
-      msecActualSleepTime = msecMinSleepTime + (uint32) (fpRand * variance);
+      actualSleepTimeMsec = minSleepTimeMsec + (uint32) (fpRand * variance);
    }
 
-   Util_Usleep(1000 * msecActualSleepTime);
+#if defined(_WIN32)
+   /* Clamp individual sleeps to avoid Windows issues */
+   totalSleepTimeMsec = actualSleepTimeMsec;
+
+   while (totalSleepTimeMsec > 0) {
+      uint32 sleepTimeMsec = (totalSleepTimeMsec > 900) ? 900 :
+                                                          totalSleepTimeMsec;
+
+      Util_Usleep(1000 * sleepTimeMsec);
+
+      totalSleepTimeMsec -= sleepTimeMsec;
+   }
+#else
+   Util_Usleep(1000 * actualSleepTimeMsec);
+#endif
 
-   return msecActualSleepTime;
+   return actualSleepTimeMsec;
 }
 
 
@@ -2506,10 +2523,10 @@ File_ContainSymLink(const char *pathName)  // IN:
 
    File_GetPathName(pathName, &path, &base);
 
-   if (   (path != NULL)
-       && (base != NULL)
-       && (strcmp(path, "") != 0)
-       && (strcmp(base, "") != 0)) {
+   if ((path != NULL) &&
+       (base != NULL) &&
+       (strcmp(path, "") != 0) &&
+       (strcmp(base, "") != 0)) {
       if (File_ContainSymLink(path)) {
          retValue = TRUE;
       }
index 65740e69cd099f651aa1704ca5fca0b551991527..fb57a47034cc06da6b7dbe4981ea0a23badcf54e 100644 (file)
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2007-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 2007-2017 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -120,22 +120,22 @@ Bool FileRetryThisError(DWORD error,
                         DWORD *codes);
 
 int FileAttributesRetry(const char *pathName,
-                        uint32 msecMaxWaitTime,
+                        uint32 maxWaitTimeMsec,
                         FileData *fileData);
 
 int FileDeletionRetry(const char *pathName,
                       Bool handleLink,
-                      uint32 msecMaxWaitTime);
+                      uint32 maxWaitTimeMsec);
 
 int FileCreateDirectoryRetry(const char *pathName,
                              int mask,
-                             uint32 msecMaxWaitTime);
+                             uint32 maxWaitTimeMsec);
 
 int FileRemoveDirectoryRetry(const char *pathName,
-                             uint32 msecMaxWaitTime);
+                             uint32 maxWaitTimeMsec);
 
 int FileListDirectoryRetry(const char *pathName,
-                           uint32 msecMaxWaitTime,
+                           uint32 maxWaitTimeMsec,
                            char ***ids);
 
 #define FileAttributes(a, b)       FileAttributesRetry((a), 0, (b))
@@ -201,8 +201,8 @@ typedef struct lock_values
    char         *memberName;
    unsigned int  lamportNumber;
    Bool          exclusivity;
-   uint32        waitTime;
-   uint32        msecMaxWaitTime;
+   VmTimeType    startTimeMsec;
+   uint32        maxWaitTimeMsec;
    ActiveLock   *lockList;
 } LockValues;
 
@@ -212,8 +212,8 @@ typedef struct lock_values
 
 #define FILELOCK_DATA_SIZE 512
 
-uint32 FileSleeper(uint32 msecMinSleepTime,
-                   uint32 msecMaxSleepTime);
+uint32 FileSleeper(uint32 minSleepTimeMsec,
+                   uint32 maxSleepTimeMsec);
 
 uint32 FileSimpleRandom(void);
 
@@ -232,7 +232,7 @@ int FileLockMemberValues(const char *lockDir,
 
 FileLockToken *FileLockIntrinsic(const char *filePathName,
                                  Bool exclusivity,
-                                 uint32 msecMaxWaitTime,
+                                 uint32 maxWaitTimeMsec,
                                  int *err);
 
 int FileUnlockIntrinsic(FileLockToken *tokenPtr);
@@ -258,7 +258,7 @@ FileIOCreateRetry(FileIODescriptor *fd,
                   int access,
                   FileIOOpenAction action,
                   int mode,
-                  uint32 msecMaxWaitTime);
+                  uint32 maxWaitTimeMsec);
 
 
 /*
index bbccca522a474cb7e479bde50657ff66116e2e2a..1962c436529877ff907e0eba221ab003d0ad9905 100644 (file)
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2006-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 2006-2017 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -1093,9 +1093,9 @@ FileLockNormalizePath(const char *filePath)  // IN:
  * FileLock_Lock --
  *
  *      Obtain a lock on a file; shared or exclusive access. Also specify
- *      how long to wait on lock acquisition - msecMaxWaitTime
+ *      how long to wait on lock acquisition - maxWaitTimeMsec
  *
- *      msecMaxWaitTime specifies the maximum amount of time, in
+ *      maxWaitTimeMsec specifies the maximum amount of time, in
  *      milliseconds, to wait for the lock before returning the "not
  *      acquired" status. A value of FILELOCK_TRYLOCK_WAIT is the
  *      equivalent of a "try lock" - the lock will be acquired only if
@@ -1116,7 +1116,7 @@ FileLockNormalizePath(const char *filePath)  // IN:
 FileLockToken *
 FileLock_Lock(const char *filePath,          // IN:
               const Bool readOnly,           // IN:
-              const uint32 msecMaxWaitTime,  // IN:
+              const uint32 maxWaitTimeMsec,  // IN:
               int *err,                      // OUT/OPT: returns errno
               MsgList **msgs)                // IN/OUT/OPT: add error message
 {
@@ -1134,7 +1134,7 @@ FileLock_Lock(const char *filePath,          // IN:
 
       tokenPtr = NULL;
    } else {
-      tokenPtr = FileLockIntrinsic(normalizedPath, !readOnly, msecMaxWaitTime,
+      tokenPtr = FileLockIntrinsic(normalizedPath, !readOnly, maxWaitTimeMsec,
                                    &res);
 
       free(normalizedPath);
index 973a1c901c98e0ce1fd1dafb9d6a868e5195352f..0ebe7828618e85b8ffbf65903d944cd84ade3802 100644 (file)
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2007-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 2007-2017 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -21,8 +21,8 @@
  *
  *      Portable file locking via Lamport's Bakery algorithm.
  *
- * This implementation does rely upon a remove directory operation to fail
- * if the directory contains any files.
+ *      This implementation relies upon a remove directory operation failing
+ *      if the directory contains any files.
  */
 
 #define _GNU_SOURCE /* For O_NOFOLLOW */
@@ -62,8 +62,8 @@
 
 #define LOCK_SHARED     "S"
 #define LOCK_EXCLUSIVE  "X"
-#define FILELOCK_PROGRESS_DEARTH 8000 // Dearth of progress time in msec
-#define FILELOCK_PROGRESS_SAMPLE 200  // Progress sampling time in msec
+#define FILELOCK_PROGRESS_DEARTH 8000 // Dearth of progress time in milliseconds
+#define FILELOCK_PROGRESS_SAMPLE 200  // Progress sampling time in milliseconds
 
 static char implicitReadToken;
 
@@ -72,9 +72,9 @@ static char implicitReadToken;
 
 typedef struct parse_table
 {
-   int type;
-   char *name;
-   void *valuePtr;
+   int    type;
+   char  *name;
+   void  *valuePtr;
 } ParseTable;
 
 /*
@@ -120,39 +120,39 @@ struct FileLockToken
  */
 
 static int
-FileLockSleeper(LockValues *myValues,  // IN/OUT:
-                uint32 *loopCount)     // IN/OUT:
+FileLockSleeper(LockValues *myValues)  // IN/OUT:
 {
-   uint32 msecSleepTime;
+   VmTimeType ageMsec;
+   uint32 maxSleepTimeMsec;
 
-   if ((myValues->msecMaxWaitTime == FILELOCK_TRYLOCK_WAIT) ||
-       ((myValues->msecMaxWaitTime != FILELOCK_INFINITE_WAIT) &&
-        (myValues->waitTime > myValues->msecMaxWaitTime))) {
+   if (myValues->maxWaitTimeMsec == FILELOCK_TRYLOCK_WAIT) {
       return EAGAIN;
    }
 
-   if (*loopCount <= 20) {
-      /* most locks are "short" */
-      msecSleepTime = 100;
-      *loopCount += 1;
-   } else if (*loopCount < 40) {
-      /* lock has been around a while, linear back-off */
-      msecSleepTime = 100 * (*loopCount - 19);
-      *loopCount += 1;
-   } else {
-      /* WOW! long time... Set a maximum */
-      msecSleepTime = 2000;
+   ageMsec = Hostinfo_SystemTimerMS() - myValues->startTimeMsec;
+
+   if ((myValues->maxWaitTimeMsec != FILELOCK_INFINITE_WAIT) &&
+       (ageMsec >= myValues->maxWaitTimeMsec)) {
+      return EAGAIN;
    }
 
-   myValues->waitTime += msecSleepTime;
+   if (ageMsec <= 2000) {
+      /* Most locks are "short" */
+      maxSleepTimeMsec = 100;
+   } else {
+      /*
+       * The lock has been around a while, linear back off with an upper bound.
+       */
 
-   /* Clamp individual sleeps to avoid Windows issues */
-   while (msecSleepTime) {
-      uint32 sleepTime = (msecSleepTime > 900) ? 900 : msecSleepTime;
+      maxSleepTimeMsec = ageMsec/10;
 
-      msecSleepTime -= FileSleeper(sleepTime, sleepTime);
+      if (maxSleepTimeMsec > 2000) {
+         maxSleepTimeMsec = 2000;
+      }
    }
 
+   (void) FileSleeper(maxSleepTimeMsec / 10, maxSleepTimeMsec);
+
    return 0;
 }
 
@@ -582,7 +582,6 @@ FileLockActivateList(const char *dirName,   // IN:
    ActiveLock   *ptr;
 
    ASSERT(dirName != NULL);
-
    ASSERT(*dirName == 'D');
 
    /* Search the list for a matching entry */
@@ -1099,17 +1098,14 @@ FileLockWaitForPossession(const char *lockDir,       // IN:
         ((strcmp(memberValues->lockType, LOCK_EXCLUSIVE) == 0) ||
          (strcmp(myValues->lockType, LOCK_EXCLUSIVE) == 0))) {
       char *path;
-      uint32 loopCount;
       Bool   thisMachine;
 
       thisMachine = FileLockMachineIDMatch(myValues->machineID,
                                            memberValues->machineID);
 
-      loopCount = 0;
-
       path = Unicode_Join(lockDir, DIRSEPS, fileName, NULL);
 
-      while ((err = FileLockSleeper(myValues, &loopCount)) == 0) {
+      while ((err = FileLockSleeper(myValues)) == 0) {
          /* still there? */
          err = FileAttributesRobust(path, NULL);
          if (err != 0) {
@@ -1138,7 +1134,7 @@ FileLockWaitForPossession(const char *lockDir,       // IN:
        * attempts. This can assist in debugging locking problems.
        */
 
-      if ((myValues->msecMaxWaitTime != FILELOCK_TRYLOCK_WAIT) &&
+      if ((myValues->maxWaitTimeMsec != FILELOCK_TRYLOCK_WAIT) &&
           (err == EAGAIN)) {
          if (thisMachine) {
             Log(LGPFX" %s timeout on '%s' due to a local process '%s'\n",
@@ -1538,7 +1534,7 @@ FileLockCreateMemberFile(FileIODescriptor *desc,       // IN:
  *      which requires kernel support for mandatory locking. Such locks
  *      are automatically broken if the host holding the lock fails.
  *
- *      msecMaxWaitTime specifies the maximum amount of time, in
+ *      maxWaitTimeMsec specifies the maximum amount of time, in
  *      milliseconds, to wait for the lock before returning the "not
  *      acquired" status. A value of FILELOCK_TRYLOCK_WAIT is the
  *      equivalent of a "try lock" - the lock will be acquired only if
@@ -1564,7 +1560,6 @@ FileLockIntrinsicMandatory(const char *pathName,   // IN:
                            int *err)               // OUT:
 {
    int access;
-   int loopCount = 0;
    int errnum;
    FileIOResult result;
    FileLockToken *tokenPtr = Util_SafeMalloc(sizeof *tokenPtr);
@@ -1574,8 +1569,8 @@ FileLockIntrinsicMandatory(const char *pathName,   // IN:
    tokenPtr->pathName = Unicode_Duplicate(pathName);
    FileIO_Invalidate(&tokenPtr->u.mandatory.lockFd);
 
-   access = myValues->exclusivity ? FILEIO_OPEN_ACCESS_WRITE
-                                  : FILEIO_OPEN_ACCESS_READ;
+   access = myValues->exclusivity ? FILEIO_OPEN_ACCESS_WRITE :
+                                    FILEIO_OPEN_ACCESS_READ;
    access |= FILEIO_OPEN_EXCLUSIVE_LOCK;
 
    do {
@@ -1587,7 +1582,7 @@ FileLockIntrinsicMandatory(const char *pathName,   // IN:
       if (result != FILEIO_LOCK_FAILED) {
          break;
       }
-   } while (FileLockSleeper(myValues, &loopCount) == 0);
+   } while (FileLockSleeper(myValues) == 0);
 
    if (FileIO_IsSuccess(result)) {
       ASSERT(FileIO_IsValid(&tokenPtr->u.mandatory.lockFd));
@@ -1812,7 +1807,7 @@ bail:
  *      implemented via mandatory locks and a more portable scheme depending
  *      on host OS support.
  *
- *      msecMaxWaitTime specifies the maximum amount of time, in
+ *      maxWaitTimeMsec specifies the maximum amount of time, in
  *      milliseconds, to wait for the lock before returning the "not
  *      acquired" status. A value of FILELOCK_TRYLOCK_WAIT is the
  *      equivalent of a "try lock" - the lock will be acquired only if
@@ -1834,7 +1829,7 @@ bail:
 FileLockToken *
 FileLockIntrinsic(const char *pathName,    // IN:
                   Bool exclusivity,        // IN:
-                  uint32 msecMaxWaitTime,  // IN:
+                  uint32 maxWaitTimeMsec,  // IN:
                   int *err)                // OUT:
 {
    char *lockBase;
@@ -1846,12 +1841,12 @@ FileLockIntrinsic(const char *pathName,    // IN:
 
    myValues.lockType = exclusivity ? LOCK_EXCLUSIVE : LOCK_SHARED;
    myValues.exclusivity = exclusivity;
-   myValues.waitTime = 0;
-   myValues.msecMaxWaitTime = msecMaxWaitTime;
+   myValues.startTimeMsec = Hostinfo_SystemTimerMS();
+   myValues.maxWaitTimeMsec = maxWaitTimeMsec;
 
    if (File_SupportsMandatoryLock(pathName)) {
       LOG(1, ("Requesting %s lock on %s (mandatory, %u).\n",
-          myValues.lockType, pathName, myValues.msecMaxWaitTime));
+          myValues.lockType, pathName, myValues.maxWaitTimeMsec));
 
       tokenPtr = FileLockIntrinsicMandatory(pathName, lockBase, &myValues, err);
    } else {
@@ -1863,7 +1858,7 @@ FileLockIntrinsic(const char *pathName,    // IN:
 
       LOG(1, ("Requesting %s lock on %s (%s, %s, %u).\n",
           myValues.lockType, pathName, myValues.machineID,
-          myValues.executionID, myValues.msecMaxWaitTime));
+          myValues.executionID, myValues.maxWaitTimeMsec));
 
       tokenPtr = FileLockIntrinsicPortable(pathName, lockBase, &myValues, err);
 
index 71a6da458281c17175e75db225e419671678d66a..f955bdd658c4a31d0dfbe0e440a546f70fe1cafb 100644 (file)
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 1998-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 1998-2017 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -36,7 +36,7 @@ extern "C" {
 #endif
 
 // The default time, in msec, to wait for a lock before giving up
-#define        FILELOCK_DEFAULT_WAIT 2500
+#define        FILELOCK_DEFAULT_WAIT (vmx86_server ? 7000 : 3500)
 
 // The wait time that provides "try lock" functionality
 #define        FILELOCK_TRYLOCK_WAIT 0
@@ -58,7 +58,7 @@ char *FileLock_TokenPathName(const FileLockToken *fileLockToken);
 
 FileLockToken *FileLock_Lock(const char *filePath,
                              const Bool readOnly,
-                             const uint32 msecMaxWaitTime,
+                             const uint32 maxWaitTimeMsec,
                              int *err,
                              MsgList **msgs);