From d48e3d1e7a5a5d065a0046b9115043bed47510a7 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 20 Feb 2008 17:35:45 +0000 Subject: [PATCH] windows-smb-locking-20080220 LICENSE MIT minor improvements to smb_rctLock usage. --- src/WINNT/afsd/cm_user.c | 4 +- src/WINNT/afsd/smb.c | 96 ++++++++++++++++++---------------------- src/WINNT/afsd/smb.h | 2 +- src/WINNT/afsd/smb3.c | 5 ++- 4 files changed, 49 insertions(+), 58 deletions(-) diff --git a/src/WINNT/afsd/cm_user.c b/src/WINNT/afsd/cm_user.c index 9000a9514..d90169273 100644 --- a/src/WINNT/afsd/cm_user.c +++ b/src/WINNT/afsd/cm_user.c @@ -160,7 +160,7 @@ void cm_CheckTokenCache(time_t now) /* * For every vcp, get the user and check his tokens */ - lock_ObtainWrite(&smb_rctLock); + lock_ObtainRead(&smb_rctLock); for (vcp=smb_allVCsp; vcp; vcp=vcp->nextp) { for (usersp=vcp->usersp; usersp; usersp=usersp->nextp) { if (usersp->unp) { @@ -192,7 +192,7 @@ void cm_CheckTokenCache(time_t now) } } } - lock_ReleaseWrite(&smb_rctLock); + lock_ReleaseRead(&smb_rctLock); } #ifdef USE_ROOT_TOKENS diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index c4a253417..1796ad0db 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -807,7 +807,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana) { smb_vc_t *vcp; - lock_ObtainWrite(&smb_globalLock); /* for numVCs */ + lock_ObtainWrite(&smb_globalLock); /* for numVCs */ lock_ObtainWrite(&smb_rctLock); for (vcp = smb_allVCsp; vcp; vcp=vcp->nextp) { if (vcp->magic != SMB_VC_MAGIC) @@ -883,7 +883,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana) } } lock_ReleaseWrite(&smb_rctLock); - lock_ReleaseWrite(&smb_globalLock); + lock_ReleaseWrite(&smb_globalLock); return vcp; } @@ -908,35 +908,35 @@ void smb_ReleaseVCInternal(smb_vc_t *vcp) vcp->refCount--; if (vcp->refCount == 0) { - if (vcp->flags & SMB_VCFLAG_ALREADYDEAD) { - /* remove VCP from smb_deadVCsp */ - for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) { - if (*vcpp == vcp) { - *vcpp = vcp->nextp; - break; - } - } - lock_FinalizeMutex(&vcp->mx); - memset(vcp,0,sizeof(smb_vc_t)); - free(vcp); - } else { - for (avcp = smb_allVCsp; avcp; avcp = avcp->nextp) { - if (avcp == vcp) - break; - } - osi_Log3(smb_logp,"VCP not dead and %sin smb_allVCsp vcp %x ref %d", - avcp?"not ":"",vcp, vcp->refCount); + if (vcp->flags & SMB_VCFLAG_ALREADYDEAD) { + /* remove VCP from smb_deadVCsp */ + for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) { + if (*vcpp == vcp) { + *vcpp = vcp->nextp; + break; + } + } + lock_FinalizeMutex(&vcp->mx); + memset(vcp,0,sizeof(smb_vc_t)); + free(vcp); + } else { + for (avcp = smb_allVCsp; avcp; avcp = avcp->nextp) { + if (avcp == vcp) + break; + } + osi_Log3(smb_logp,"VCP not dead and %sin smb_allVCsp vcp %x ref %d", + avcp?"not ":"",vcp, vcp->refCount); #ifdef DEBUG - GenerateMiniDump(NULL); + GenerateMiniDump(NULL); #endif - /* This is a wrong. However, I suspect that there is an undercount - * and I don't want to release 1.4.1 in a state that will allow - * smb_vc_t objects to be deallocated while still in the - * smb_allVCsp list. The list is supposed to keep a reference - * to the smb_vc_t. Put it back. - */ - vcp->refCount++; - } + /* This is a wrong. However, I suspect that there is an undercount + * and I don't want to release 1.4.1 in a state that will allow + * smb_vc_t objects to be deallocated while still in the + * smb_allVCsp list. The list is supposed to keep a reference + * to the smb_vc_t. Put it back. + */ + vcp->refCount++; + } } } @@ -1035,11 +1035,7 @@ void smb_CleanupDeadVC(smb_vc_t *vcp) osi_Log2(smb_logp, " Cleanup TID %d (tidp=0x%x)", tid, tidpIter); smb_HoldTIDNoLock(tidpIter); - lock_ReleaseWrite(&smb_rctLock); - - smb_ReleaseTID(tidpIter); - - lock_ObtainWrite(&smb_rctLock); + smb_ReleaseTID(tidpIter, TRUE); tidpNext = vcp->tidsp; } @@ -1076,9 +1072,7 @@ smb_tid_t *smb_FindTID(smb_vc_t *vcp, unsigned short tid, int flags) for (tidp = vcp->tidsp; tidp; tidp = tidp->nextp) { if (tidp->refCount == 0 && tidp->delete) { tidp->refCount++; - lock_ReleaseWrite(&smb_rctLock); - smb_ReleaseTID(tidp); - lock_ObtainWrite(&smb_rctLock); + smb_ReleaseTID(tidp, TRUE); goto retry; } @@ -1107,14 +1101,15 @@ void smb_HoldTIDNoLock(smb_tid_t *tidp) tidp->refCount++; } -void smb_ReleaseTID(smb_tid_t *tidp) +void smb_ReleaseTID(smb_tid_t *tidp, afs_uint32 locked) { smb_tid_t *tp; smb_tid_t **ltpp; cm_user_t *userp; userp = NULL; - lock_ObtainWrite(&smb_rctLock); + if (!locked) + lock_ObtainWrite(&smb_rctLock); osi_assertx(tidp->refCount-- > 0, "smb_tid_t refCount 0"); if (tidp->refCount == 0 && (tidp->delete)) { ltpp = &tidp->vcp->tidsp; @@ -1130,7 +1125,8 @@ void smb_ReleaseTID(smb_tid_t *tidp) smb_ReleaseVCNoLock(tidp->vcp); tidp->vcp = NULL; } - lock_ReleaseWrite(&smb_rctLock); + if (!locked) + lock_ReleaseWrite(&smb_rctLock); if (userp) cm_ReleaseUser(userp); } @@ -1242,10 +1238,8 @@ void smb_ReleaseUsername(smb_username_t *unp) free(unp); } lock_ReleaseWrite(&smb_rctLock); - - if (userp) { + if (userp) cm_ReleaseUser(userp); - } } void smb_HoldUIDNoLock(smb_user_t *uidp) @@ -1341,7 +1335,7 @@ long smb_LookupTIDPath(smb_vc_t *vcp, unsigned short tid, char ** treepath) /* tidp->pathname would be NULL, but that's fine */ } *treepath = tidp->pathname; - smb_ReleaseTID(tidp); + smb_ReleaseTID(tidp, FALSE); } return code; } @@ -2951,6 +2945,7 @@ long smb_ReceiveCoreReadRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp lock_ReleaseMutex(&smb_RawBufLock); } + lock_ReleaseMutex(&fidp->mx); smb_ReleaseFID(fidp); return rc; } @@ -3347,11 +3342,8 @@ void smb_Daemon(void *parmp) free(unp->name); free(unp->machine); free(unp); - if (userp) { - lock_ReleaseWrite(&smb_rctLock); + if (userp) cm_ReleaseUser(userp); - lock_ObtainWrite(&smb_rctLock); - } } else { unpp = &(*unpp)->nextp; } @@ -3552,14 +3544,14 @@ long smb_ReceiveCoreTreeConnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * if (uidp) smb_ReleaseUID(uidp); if (!shareFound) { - smb_ReleaseTID(tidp); + smb_ReleaseTID(tidp, FALSE); return CM_ERROR_BADSHARENAME; } lock_ObtainMutex(&tidp->mx); tidp->userp = userp; tidp->pathname = sharePath; lock_ReleaseMutex(&tidp->mx); - smb_ReleaseTID(tidp); + smb_ReleaseTID(tidp, FALSE); smb_SetSMBParm(rsp, 0, SMB_PACKETSIZE); smb_SetSMBParm(rsp, 1, newTid); @@ -4782,8 +4774,8 @@ long smb_ReceiveCoreTreeDisconnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_ if (tidp) { lock_ObtainWrite(&smb_rctLock); tidp->delete = 1; + smb_ReleaseTID(tidp, TRUE); lock_ReleaseWrite(&smb_rctLock); - smb_ReleaseTID(tidp); } return 0; @@ -9226,9 +9218,7 @@ void smb_Shutdown(void) if (tidp->userp) { cm_user_t *userp = tidp->userp; tidp->userp = NULL; - lock_ReleaseWrite(&smb_rctLock); cm_ReleaseUser(userp); - lock_ObtainWrite(&smb_rctLock); } } } diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h index 7abdf5709..aa74dca46 100644 --- a/src/WINNT/afsd/smb.h +++ b/src/WINNT/afsd/smb.h @@ -548,7 +548,7 @@ extern smb_tid_t *smb_FindTID(smb_vc_t *vcp, unsigned short tid, int flags); extern void smb_HoldTIDNoLock(smb_tid_t *tidp); -extern void smb_ReleaseTID(smb_tid_t *tidp); +extern void smb_ReleaseTID(smb_tid_t *tidp, afs_uint32 locked); extern smb_user_t *smb_FindUID(smb_vc_t *vcp, unsigned short uid, int flags); diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index a30877d16..41ed6fd90 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -1061,7 +1061,7 @@ long smb_ReceiveV3TreeConnectX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *o if (!shareFound) { if (uidp) smb_ReleaseUID(uidp); - smb_ReleaseTID(tidp); + smb_ReleaseTID(tidp, FALSE); return CM_ERROR_BADSHARENAME; } @@ -1098,7 +1098,7 @@ long smb_ReceiveV3TreeConnectX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *o if (ipc) tidp->flags |= SMB_TIDFLAG_IPC; lock_ReleaseMutex(&tidp->mx); - smb_ReleaseTID(tidp); + smb_ReleaseTID(tidp, FALSE); ((smb_t *)outp)->tid = newTid; ((smb_t *)inp)->tid = newTid; @@ -4657,6 +4657,7 @@ long smb_T2SearchDirSingle(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t *op smb_FreeTran2Packet(outp); if (dep) free(dep); + if (scp) cm_ReleaseSCache(scp); cm_ReleaseSCache(targetscp); cm_ReleaseUser(userp); -- 2.39.5