From: Oliver Kurth Date: Fri, 15 Sep 2017 18:23:47 +0000 (-0700) Subject: lib/file: FileLockSleeper issues X-Git-Tag: stable-10.2.0~136 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8da9461f220ed391f60a78c4f0a9509c474f4d12;p=thirdparty%2Fopen-vm-tools.git lib/file: FileLockSleeper issues 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. --- diff --git a/open-vm-tools/lib/file/file.c b/open-vm-tools/lib/file/file.c index 1ec4f7768..8cfa2902a 100644 --- a/open-vm-tools/lib/file/file.c +++ b/open-vm-tools/lib/file/file.c @@ -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; } diff --git a/open-vm-tools/lib/file/fileInt.h b/open-vm-tools/lib/file/fileInt.h index 65740e69c..fb57a4703 100644 --- a/open-vm-tools/lib/file/fileInt.h +++ b/open-vm-tools/lib/file/fileInt.h @@ -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); /* diff --git a/open-vm-tools/lib/file/fileLockPosix.c b/open-vm-tools/lib/file/fileLockPosix.c index bbccca522..1962c4365 100644 --- a/open-vm-tools/lib/file/fileLockPosix.c +++ b/open-vm-tools/lib/file/fileLockPosix.c @@ -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); diff --git a/open-vm-tools/lib/file/fileLockPrimitive.c b/open-vm-tools/lib/file/fileLockPrimitive.c index 973a1c901..0ebe78286 100644 --- a/open-vm-tools/lib/file/fileLockPrimitive.c +++ b/open-vm-tools/lib/file/fileLockPrimitive.c @@ -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); diff --git a/open-vm-tools/lib/include/fileLock.h b/open-vm-tools/lib/include/fileLock.h index 71a6da458..f955bdd65 100644 --- a/open-vm-tools/lib/include/fileLock.h +++ b/open-vm-tools/lib/include/fileLock.h @@ -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);