From: Jeffrey Altman Date: Thu, 28 Feb 2008 17:16:28 +0000 (+0000) Subject: windows-smb-lock-timeouts-20080228 X-Git-Tag: BP-openafs-windows-kdfs-ifs~68 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=e9f7894e3576ef9e01234966f7869440f1823718;p=packages%2Fo%2Fopenafs.git windows-smb-lock-timeouts-20080228 LICENSE MIT Attempts to open files which are already write-locked by another client took forever to return a lock not granted error. This was because cm_Analyze() would retry the lock request for up to the RDRtimeout in response to the EAGAIN error. The problem was that cm_IntSetLock() was not setting the CM_REQ_NORETRY flag. While examining this issue, discovered two other things: (1) the infinite wait logic on lock request processing was broken (2) the cancel outstanding lock request logic wasn't implemented (3) cm_Analyze() would put the thread to sleep even when retries were not permitted. Also removed a number of compile time warnings. --- diff --git a/src/WINNT/afsd/cm.h b/src/WINNT/afsd/cm.h index 427339235..3b3161a25 100644 --- a/src/WINNT/afsd/cm.h +++ b/src/WINNT/afsd/cm.h @@ -90,6 +90,8 @@ #define CM_ERROR_BPLUS_NOMATCH (CM_ERROR_BASE+55) #define CM_ERROR_EAS_NOT_SUPPORTED (CM_ERROR_BASE+56) #define CM_ERROR_RANGE_NOT_LOCKED (CM_ERROR_BASE+57) +#define CM_ERROR_NOSUCHDEVICE (CM_ERROR_BASE+58) +#define CM_ERROR_LOCK_NOT_GRANTED (CM_ERROR_BASE+59) /* Used by cm_FollowMountPoint and cm_GetVolumeByName */ #define RWVOL 0 diff --git a/src/WINNT/afsd/cm_conn.c b/src/WINNT/afsd/cm_conn.c index 7ff818deb..585c05240 100644 --- a/src/WINNT/afsd/cm_conn.c +++ b/src/WINNT/afsd/cm_conn.c @@ -202,7 +202,10 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp, timeUsed = (GetTickCount() - reqp->startTime) / 1000; /* leave 5 seconds margin for sleep */ - timeLeft = HardDeadtimeout - timeUsed; + if (reqp->flags & CM_REQ_NORETRY) + timeLeft = 0; + else + timeLeft = HardDeadtimeout - timeUsed; /* get a pointer to the cell */ if (errorCode) { @@ -221,7 +224,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp, } if (errorCode == CM_ERROR_TIMEDOUT) { - if (timeLeft > 5 ) { + if ( timeLeft > 5 ) { thrd_Sleep(3000); cm_CheckServers(CM_FLAG_CHECKDOWNSERVERS, cellp); retry = 1; @@ -231,7 +234,7 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp, else if (errorCode == UAEWOULDBLOCK || errorCode == EWOULDBLOCK || errorCode == UAEAGAIN || errorCode == EAGAIN) { osi_Log0(afsd_logp, "cm_Analyze passed EWOULDBLOCK or EAGAIN."); - if (timeLeft > 5 ) { + if ( timeLeft > 5 ) { thrd_Sleep(1000); retry = 1; } diff --git a/src/WINNT/afsd/cm_dcache.h b/src/WINNT/afsd/cm_dcache.h index 8cde66da6..63f4264c9 100644 --- a/src/WINNT/afsd/cm_dcache.h +++ b/src/WINNT/afsd/cm_dcache.h @@ -57,4 +57,7 @@ extern long cm_ValidateDCache(void); extern long cm_ShutdownDCache(void); +extern long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags, + cm_user_t *userp, cm_req_t *reqp); + #endif /* __CM_DCACHE_ENV__ */ diff --git a/src/WINNT/afsd/cm_vnodeops.c b/src/WINNT/afsd/cm_vnodeops.c index 8c13ee197..6d77ede7d 100644 --- a/src/WINNT/afsd/cm_vnodeops.c +++ b/src/WINNT/afsd/cm_vnodeops.c @@ -3963,6 +3963,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType, cm_conn_t * connp; struct rx_connection * callp; AFSVolSync volSync; + afs_uint32 reqflags = reqp->flags; tfid.Volume = scp->fid.volume; tfid.Vnode = scp->fid.vnode; @@ -3971,6 +3972,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType, osi_Log2(afsd_logp, "CALL SetLock scp 0x%p for lock %d", scp, lockType); + reqp->flags |= CM_REQ_NORETRY; lock_ReleaseMutex(&scp->mx); do { @@ -3994,7 +3996,7 @@ long cm_IntSetLock(cm_scache_t * scp, cm_user_t * userp, int lockType, } lock_ObtainMutex(&scp->mx); - + reqp->flags = reqflags; return code; } diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index a44ee1490..f5bd1605c 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -78,9 +78,9 @@ smb_dispatch_t smb_dispatchTable[SMB_NOPCODES]; smb_packet_t *smb_packetFreeListp; smb_ncb_t *smb_ncbFreeListp; -int smb_NumServerThreads; +afs_uint32 smb_NumServerThreads; -int numNCBs, numSessions, numVCs; +afs_uint32 numNCBs, numSessions, numVCs; int smb_maxVCPerServer; int smb_maxMpxRequests; @@ -214,7 +214,7 @@ void smb_UpdateServerPriority() time_t now = osi_Time(); /* Give one priority boost for each 15 seconds */ - SetThreadPriority(GetCurrentThread(), (now - *tp) / 15); + SetThreadPriority(GetCurrentThread(), (int)((now - *tp) / 15)); } } @@ -1664,7 +1664,7 @@ int smb_FindShare(smb_vc_t *vcp, smb_user_t *uidp, char *shareName, snprintf(pathstr, sizeof(pathstr)/sizeof(char), "/" CM_PREFIX_VOL "%s", shareName); pathstr[sizeof(pathstr)/sizeof(char) - 1] = '\0'; - len = strlen(pathstr) + 1; + len = (DWORD)(strlen(pathstr) + 1); *pathNamep = malloc(len); if (*pathNamep) { @@ -2641,7 +2641,12 @@ void smb_MapNTError(long code, unsigned long *NTStatusp) else if (code == CM_ERROR_RANGE_NOT_LOCKED) { NTStatus = 0xC000007EL; /* Range Not Locked */ } - else { + else if (code == CM_ERROR_NOSUCHDEVICE) { + NTStatus = 0xC000000EL; /* No Such Device */ + } + else if (code == CM_ERROR_LOCK_NOT_GRANTED) { + NTStatus = 0xC0000055L; /* Lock Not Granted */ + } else { NTStatus = 0xC0982001L; /* SMB non-specific error */ } @@ -3396,6 +3401,11 @@ void smb_WaitingLocksDaemon() if (wl->state == SMB_WAITINGLOCKSTATE_DONE) continue; + if (wl->state == SMB_WAITINGLOCKSTATE_CANCELLED) { + code = CM_ERROR_LOCK_NOT_GRANTED; + break; + } + osi_assertx(wl->state != SMB_WAITINGLOCKSTATE_ERROR, "!SMB_WAITINGLOCKSTATE_ERROR"); /* wl->state is either _DONE or _WAITING. _ERROR @@ -3414,8 +3424,8 @@ void smb_WaitingLocksDaemon() if (code == CM_ERROR_WOULDBLOCK) { /* no progress */ - if (wlRequest->timeRemaining != 0xffffffff - && (wlRequest->timeRemaining -= 1000) < 0) + if (wlRequest->msTimeout != 0xffffffff + && ((osi_Time() - wlRequest->start_t) * 1000 > wlRequest->msTimeout)) goto endWait; continue; @@ -3439,9 +3449,10 @@ void smb_WaitingLocksDaemon() for (wl = wlRequest->locks; wl; wl = wlNext) { wlNext = (smb_waitingLock_t *) osi_QNext(&wl->q); - - cm_Unlock(scp, wlRequest->lockType, wl->LOffset, - wl->LLength, wl->key, NULL, &req); + + if (wl->state == SMB_WAITINGLOCKSTATE_DONE) + cm_Unlock(scp, wlRequest->lockType, wl->LOffset, + wl->LLength, wl->key, NULL, &req); osi_QRemove((osi_queue_t **) &wlRequest->locks, &wl->q); @@ -6019,7 +6030,7 @@ long smb_ReceiveCoreClose(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) /* * smb_ReadData -- common code for Read, Read And X, and Raw Read */ -long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, +long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op, cm_user_t *userp, long *readp) { osi_hyper_t offset; @@ -6030,7 +6041,8 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, osi_hyper_t thyper; osi_hyper_t lastByte; osi_hyper_t bufferOffset; - long bufIndex, nbytes; + long bufIndex; + afs_uint32 nbytes; int chunk; int sequential = (fidp->flags & SMB_FID_SEQUENTIAL); cm_req_t req; @@ -6170,7 +6182,7 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, /* * smb_WriteData -- common code for Write and Raw Write */ -long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, +long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op, cm_user_t *userp, long *writtenp) { osi_hyper_t offset = *offsetp; @@ -8018,7 +8030,7 @@ void InitNCBslot(int idx) { struct smb_packet *bufp; EVENT_HANDLE retHandle; - int i; + afs_uint32 i; char eventName[MAX_PATH]; osi_assertx( idx < (sizeof(NCBs) / sizeof(NCBs[0])), "invalid index" ); @@ -8050,7 +8062,7 @@ void smb_Listener(void *parmp) long code = 0; long len; long i; - int session, thread; + afs_uint32 session, thread; smb_vc_t *vcp = NULL; int flags = 0; char rname[NCBNAMSZ+1]; @@ -9120,7 +9132,7 @@ void smb_Shutdown(void) { NCB *ncbp; long code = 0; - int i; + afs_uint32 i; smb_vc_t *vcp; /*fprintf(stderr, "Entering smb_Shutdown\n");*/ diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h index 6a80a9353..86ec4a90e 100644 --- a/src/WINNT/afsd/smb.h +++ b/src/WINNT/afsd/smb.h @@ -482,9 +482,10 @@ typedef struct smb_waitingLock { int state; } smb_waitingLock_t; -#define SMB_WAITINGLOCKSTATE_WAITING 0 -#define SMB_WAITINGLOCKSTATE_DONE 1 -#define SMB_WAITINGLOCKSTATE_ERROR 2 +#define SMB_WAITINGLOCKSTATE_WAITING 0 +#define SMB_WAITINGLOCKSTATE_DONE 1 +#define SMB_WAITINGLOCKSTATE_ERROR 2 +#define SMB_WAITINGLOCKSTATE_CANCELLED 3 /* waiting lock request */ typedef struct smb_waitingLockRequest { @@ -494,7 +495,8 @@ typedef struct smb_waitingLockRequest { smb_packet_t *inp; smb_packet_t *outp; int lockType; - time_t timeRemaining; + time_t start_t; /* osi_Time */ + afs_uint32 msTimeout; /* msecs, 0xFFFFFFFF = wait forever */ smb_waitingLock_t * locks; } smb_waitingLockRequest_t; @@ -713,10 +715,10 @@ extern unsigned char *smb_ParseVblBlock(unsigned char *inp, char **chainpp, int extern int smb_SUser(cm_user_t *userp); -long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, +long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op, cm_user_t *userp, long *writtenp); -extern long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, +extern long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, afs_uint32 count, char *op, cm_user_t *userp, long *readp); extern long smb_Rename(smb_vc_t *vcp, smb_packet_t *inp, char *oldPathp, char *newPathp, int attrs); diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index 4dfa3c387..6aa394be9 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -73,10 +73,10 @@ afs_uint32 smb_IsExecutableFileName(const char *name) if ( smb_ExecutableExtensions == NULL || name == NULL) return 0; - len = strlen(name); + len = (int)strlen(name); for ( i=0; smb_ExecutableExtensions[i]; i++) { - j = len - strlen(smb_ExecutableExtensions[i]); + j = len - (int)strlen(smb_ExecutableExtensions[i]); if (_stricmp(smb_ExecutableExtensions[i], &name[j]) == 0) return 1; } @@ -2883,7 +2883,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t goto done; } else if (infoLevel == SMB_QUERY_FILE_NAME_INFO) { - len = strlen(lastComp); + len = (unsigned int)strlen(lastComp); qpi.u.QPfileNameInfo.fileNameLength = (len + 1) * 2; mbstowcs((unsigned short *)qpi.u.QPfileNameInfo.fileName, lastComp, len + 1); @@ -2961,7 +2961,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t qpi.u.QPfileAllInfo.currentByteOffset.LowPart = 0; qpi.u.QPfileAllInfo.mode = 0; qpi.u.QPfileAllInfo.alignmentRequirement = 0; - len = strlen(lastComp); + len = (unsigned int)strlen(lastComp); qpi.u.QPfileAllInfo.fileNameLength = (len + 1) * 2; mbstowcs((unsigned short *)qpi.u.QPfileAllInfo.fileName, lastComp, len + 1); } @@ -3627,8 +3627,8 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t osi_Log2(smb_logp,"ReceiveTran2GetDfsReferral [%d][%s]", maxReferralLevel, osi_LogSaveString(smb_logp, requestFileName)); - nbnLen = strlen(cm_NetbiosName); - reqLen = strlen(requestFileName); + nbnLen = (int)strlen(cm_NetbiosName); + reqLen = (int)strlen(requestFileName); if (reqLen > nbnLen + 2 && requestFileName[0] == '\\' && !_strnicmp(cm_NetbiosName,&requestFileName[1],nbnLen) && @@ -3705,10 +3705,10 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t /* scp should now be the DfsLink we are looking for */ if (scp) { /* figure out how much of the input path was used */ - reqLen = nbnLen+2 + strlen(pathName) + 1 + strlen(lastComponent); + reqLen = (int)(nbnLen+2 + strlen(pathName) + 1 + strlen(lastComponent)); strcpy(referralPath, &scp->mountPointStringp[strlen("msdfs:")]); - refLen = strlen(referralPath); + refLen = (int)strlen(referralPath); found = 1; } } else { @@ -3792,7 +3792,7 @@ smb_ReceiveTran2GetDFSReferral(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t return 0; #else /* DFS_SUPPORT */ osi_Log0(smb_logp,"ReceiveTran2GetDfsReferral - NOT_SUPPORTED"); - return CM_ERROR_BADOP; + return CM_ERROR_NOSUCHDEVICE; #endif /* DFS_SUPPORT */ } @@ -4530,7 +4530,7 @@ long smb_T2SearchDirSingle(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t *op ohbytes += 4; /* EASIZE */ /* add header to name & term. null */ - onbytes = strlen(maskp); + onbytes = (int)strlen(maskp); orbytes = ohbytes + onbytes + 1; /* now, we round up the record to a 4 byte alignment, and we make @@ -5783,7 +5783,7 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) cm_scache_t *scp; unsigned char LockType; unsigned short NumberOfUnlocks, NumberOfLocks; - long Timeout; + afs_uint32 Timeout; char *op; char *op_locks; LARGE_INTEGER LOffset, LLength; @@ -5850,18 +5850,9 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) LockType |= LOCKING_ANDX_SHARED_LOCK; } - if ((LockType & LOCKING_ANDX_CANCEL_LOCK) || - (LockType & LOCKING_ANDX_CHANGE_LOCKTYPE)) { - - /* We don't support these requests. Apparently, we can safely - not deal with them too. */ - osi_Log1(smb_logp, "smb_ReceiveV3Locking received unsupported request [%s]", - ((LockType & LOCKING_ANDX_CANCEL_LOCK)? - "LOCKING_ANDX_CANCEL_LOCK": - "LOCKING_ANDX_CHANGE_LOCKTYPE")); - /* No need to call osi_LogSaveString since these are string - constants.*/ - + if (LockType & LOCKING_ANDX_CHANGE_LOCKTYPE) { + /* AFS does not support atomic changes of lock types from read or write and vice-versa */ + osi_Log0(smb_logp, "smb_ReceiveV3Locking received unsupported request [LOCKING_ANDX_CHANGE_LOCKTYPE]"); code = CM_ERROR_BADOP; goto done; @@ -5869,6 +5860,35 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) op = smb_GetSMBData(inp, NULL); + if (LockType & LOCKING_ANDX_CANCEL_LOCK) { + /* Cancel outstanding lock requests */ + smb_waitingLock_t * wl; + + for (i=0; ivcID, pid, fidp->fid); + + lock_ObtainWrite(&smb_globalLock); + for (wlRequest = smb_allWaitingLocks; wlRequest; wlRequest = (smb_waitingLockRequest_t *) osi_QNext(&wlRequest->q)) + { + for (wl = wlRequest->locks; wl; wl = (smb_waitingLock_t *) osi_QNext(&wl->q)) { + if (wl->key == key && LargeIntegerEqualTo(wl->LOffset, LOffset) && + LargeIntegerEqualTo(wl->LLength, LLength)) { + wl->state = SMB_WAITINGLOCKSTATE_CANCELLED; + goto found_lock_request; + } + } + } + found_lock_request: + lock_ReleaseWrite(&smb_globalLock); + } + code = 0; + smb_SetSMBDataLength(outp, 0); + goto done; + } + + for (i=0; iinp = smb_CopyPacket(inp); wlRequest->outp = smb_CopyPacket(outp); wlRequest->lockType = LockType; - wlRequest->timeRemaining = Timeout; + wlRequest->msTimeout = Timeout; + wlRequest->start_t = osi_Time(); wlRequest->locks = NULL; /* The waiting lock request needs to have enough @@ -6315,8 +6336,6 @@ long smb_ReceiveV3WriteX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) written = 0; } - done_writing: - /* slots 0 and 1 are reserved for request chaining and will be filled in when we return. */ smb_SetSMBParm(outp, 2, total_written); @@ -7199,7 +7218,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) LLength.LowPart = SMB_FID_QLOCK_LENGTH; /* If we are not opening the file for writing, then we don't - try to get an exclusive lock. Noone else should be able to + try to get an exclusive lock. No one else should be able to get an exclusive lock on the file anyway, although someone else can get a shared lock. */ if ((fidflags & SMB_FID_SHARE_READ) || @@ -8254,6 +8273,12 @@ long smb_ReceiveNTTransact(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) break; case 6: return smb_ReceiveNTTranQuerySecurityDesc(vcp, inp, outp); + case 7: + osi_Log0(smb_logp, "SMB NT Transact Query Quota - not implemented"); + break; + case 8: + osi_Log0(smb_logp, "SMB NT Transact Set Quota - not implemented"); + break; } return CM_ERROR_INVAL; }