From 1e53ca008459dbe06a4c74540e421b113a5138a8 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 1 Feb 2006 18:03:18 +0000 Subject: [PATCH] STABLE14-windows-more-cleanups-20060201 * remove all references to 'dead_vcp'; cleanup smb_vc_t's as soon as we know they are dead * add mx holds across the cm_cell_t updates * add cm_FindSCacheParent() and remove duplicate code elsewhere * add mx holds across scp->flags updates * add cm_CleanFile() * clear CM_SCACHEFLAG_CALLBACK when discarding callbacks * fix smb fid wrapping. wrap at 0xFFFF instead of 0 because 0xFFFF is -1 which is INVALID_HANDLE * add missing mx holds around vcp->flags updates (cherry picked from commit 8b39114d5b36f60904e5a615a16b43b7e65c3017) --- src/WINNT/afsd/cm_cell.c | 2 + src/WINNT/afsd/cm_conn.c | 15 +++++-- src/WINNT/afsd/cm_dcache.c | 1 + src/WINNT/afsd/cm_freelance.c | 2 + src/WINNT/afsd/cm_ioctl.c | 55 +++++++++++------------- src/WINNT/afsd/cm_ioctl.h | 2 + src/WINNT/afsd/cm_scache.c | 40 +++++++++++++++++- src/WINNT/afsd/cm_scache.h | 2 + src/WINNT/afsd/cm_vnodeops.c | 2 +- src/WINNT/afsd/smb.c | 79 +++++++++++------------------------ 10 files changed, 111 insertions(+), 89 deletions(-) diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c index 2b039dcb8..65bfa58fa 100644 --- a/src/WINNT/afsd/cm_cell.c +++ b/src/WINNT/afsd/cm_cell.c @@ -66,6 +66,7 @@ cm_cell_t *cm_UpdateCell(cm_cell_t * cp) if (cp == NULL) return NULL; + lock_ObtainMutex(&cp->mx); if ((cp->vlServersp == NULL #ifdef AFS_FREELANCE_CLIENT && !(cp->flags & CM_CELLFLAG_FREELANCE) @@ -112,6 +113,7 @@ cm_cell_t *cm_UpdateCell(cm_cell_t * cp) cp->timeout = time(0) + 7200; } } + lock_ReleaseMutex(&cp->mx); return cp; } diff --git a/src/WINNT/afsd/cm_conn.c b/src/WINNT/afsd/cm_conn.c index 4bb8247f1..ae3fbd670 100644 --- a/src/WINNT/afsd/cm_conn.c +++ b/src/WINNT/afsd/cm_conn.c @@ -406,10 +406,19 @@ cm_Analyze(cm_conn_t *connp, cm_user_t *userp, cm_req_t *reqp, cm_scache_t * scp; osi_Log4(afsd_logp, "cm_Analyze passed VNOVNODE cell %u vol %u vn %u uniq %u.", fidp->cell, fidp->volume, fidp->vnode, fidp->unique); - if (!cm_GetSCache(fidp, &scp, userp, reqp)) { - cm_FlushParent(scp, userp, reqp); - cm_FlushFile(scp, userp, reqp); + scp = cm_FindSCache(fidp); + if (scp) { + cm_scache_t *pscp = cm_FindSCacheParent(scp); + cm_CleanFile(scp, userp, reqp); cm_ReleaseSCache(scp); + if (pscp) { + if (pscp->cbExpires > 0 && pscp->cbServerp != NULL) { + lock_ObtainMutex(&pscp->mx); + cm_DiscardSCache(pscp); + lock_ReleaseMutex(&pscp->mx); + } + cm_ReleaseSCache(pscp); + } } } else { osi_Log0(afsd_logp, "cm_Analyze passed VNOVNODE unknown fid."); diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c index 7e92a10ea..edccbb2ea 100644 --- a/src/WINNT/afsd/cm_dcache.c +++ b/src/WINNT/afsd/cm_dcache.c @@ -501,6 +501,7 @@ void cm_BkgStore(cm_scache_t *scp, long p1, long p2, long p3, long p4, lock_ReleaseMutex(&scp->mx); } +/* Called with scp locked */ void cm_ClearPrefetchFlag(long code, cm_scache_t *scp, osi_hyper_t *base) { osi_hyper_t thyper; diff --git a/src/WINNT/afsd/cm_freelance.c b/src/WINNT/afsd/cm_freelance.c index 4ab1d5ad5..e539a3517 100644 --- a/src/WINNT/afsd/cm_freelance.c +++ b/src/WINNT/afsd/cm_freelance.c @@ -426,7 +426,9 @@ int cm_reInitLocalMountPoints() { lscpp = &tscp->nextp, tscp = tscp->nextp) { if (tscp == scp) { *lscpp = scp->nextp; + lock_ObtainMutex(&scp->mx); scp->flags &= ~CM_SCACHEFLAG_INHASH; + lock_ReleaseMutex(&scp->mx); break; } } diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c index 71c463e82..9f7db6547 100644 --- a/src/WINNT/afsd/cm_ioctl.c +++ b/src/WINNT/afsd/cm_ioctl.c @@ -62,6 +62,23 @@ void cm_InitIoctl(void) lock_InitializeMutex(&cm_Afsdsbmt_Lock, "AFSDSBMT.INI Access Lock"); } + +long cm_CleanFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) +{ + long code; + + lock_ObtainWrite(&scp->bufCreateLock); + code = buf_CleanVnode(scp, userp, reqp); + + lock_ObtainMutex(&scp->mx); + cm_DiscardSCache(scp); + lock_ReleaseMutex(&scp->mx); + + lock_ReleaseWrite(&scp->bufCreateLock); + osi_Log2(afsd_logp,"cm_CleanFile scp 0x%x returns error: [%x]",scp, code); + return code; +} + long cm_FlushFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) { long code; @@ -70,14 +87,7 @@ long cm_FlushFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) code = buf_FlushCleanPages(scp, userp, reqp); lock_ObtainMutex(&scp->mx); - scp->cbServerp = NULL; - scp->cbExpires = 0; - cm_dnlcPurgedp(scp); - cm_dnlcPurgevp(scp); - cm_FreeAllACLEnts(scp); - - /* Force mount points and symlinks to be re-evaluated */ - scp->mountPointStringp[0] = '\0'; + cm_DiscardSCache(scp); lock_ReleaseMutex(&scp->mx); @@ -89,30 +99,13 @@ long cm_FlushFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) long cm_FlushParent(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) { long code = 0; - int i; - cm_fid_t parent_fid; - - lock_ObtainWrite(&cm_scacheLock); - cm_HoldSCacheNoLock(scp); - parent_fid = scp->fid; - parent_fid.vnode = scp->parentVnode; - parent_fid.unique = scp->parentUnique; - cm_ReleaseSCacheNoLock(scp); + cm_scache_t * pscp; - for (i=0; inextp) { - if (!cm_FidCmp(&scp->fid, &parent_fid)) { - cm_HoldSCacheNoLock(scp); - lock_ReleaseWrite(&cm_scacheLock); + pscp = cm_FindSCacheParent(scp); - /* now flush the file */ - code = cm_FlushFile(scp, userp, reqp); - lock_ObtainWrite(&cm_scacheLock); - cm_ReleaseSCacheNoLock(scp); - } - } - } - lock_ReleaseWrite(&cm_scacheLock); + /* now flush the file */ + code = cm_FlushFile(pscp, userp, reqp); + cm_ReleaseSCache(scp); return code; } @@ -1266,6 +1259,7 @@ long cm_IoctlNewCell(struct smb_ioctl *ioctlp, struct cm_user *userp) for (cp = cm_data.allCellsp; cp; cp=cp->nextp) { long code; + lock_ObtainMutex(&cp->mx); /* delete all previous server lists - cm_FreeServerList will ask for write on cm_ServerLock*/ cm_FreeServerList(&cp->vlServersp); cp->vlServersp = NULL; @@ -1293,6 +1287,7 @@ long cm_IoctlNewCell(struct smb_ioctl *ioctlp, struct cm_user *userp) cp->flags &= ~CM_CELLFLAG_VLSERVER_INVALID; cm_RandomizeServer(&cp->vlServersp); } + lock_ReleaseMutex(&cp->mx); } lock_ReleaseWrite(&cm_cellLock); diff --git a/src/WINNT/afsd/cm_ioctl.h b/src/WINNT/afsd/cm_ioctl.h index a0d3c0173..246a78834 100644 --- a/src/WINNT/afsd/cm_ioctl.h +++ b/src/WINNT/afsd/cm_ioctl.h @@ -116,6 +116,8 @@ extern long cm_IoctlStoreBehind(smb_ioctl_t *ioctlp, cm_user_t *userp); extern long cm_IoctlCreateMountPoint(smb_ioctl_t *ioctlp, cm_user_t *userp); +extern long cm_CleanFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp); + extern long cm_FlushFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp); extern long cm_FlushVolume(cm_user_t *, cm_req_t *reqp, afs_uint32 cell, afs_uint32 volume); diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c index 31ceeba02..9eca4aaa8 100644 --- a/src/WINNT/afsd/cm_scache.c +++ b/src/WINNT/afsd/cm_scache.c @@ -473,7 +473,8 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp, mp = ""; } scp = cm_GetNewSCache(); - + + lock_ObtainMutex(&scp->mx); scp->fid = *fidp; scp->volp = cm_data.rootSCachep->volp; scp->dotdotFid.cell=AFS_FAKE_ROOT_CELL_ID; @@ -504,6 +505,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp, scp->parentVnode=0x1; scp->group=0; scp->dataVersion=cm_data.fakeDirVersion; + lock_ReleaseMutex(&scp->mx); *outScpp = scp; lock_ReleaseWrite(&cm_scacheLock); /*afsi_log(" getscache done");*/ @@ -544,6 +546,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp, /* now, if we don't have the fid, recycle something */ scp = cm_GetNewSCache(); osi_assert(!(scp->flags & CM_SCACHEFLAG_INHASH)); + lock_ObtainMutex(&scp->mx); scp->fid = *fidp; scp->volp = volp; /* a held reference */ @@ -565,6 +568,7 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp, cm_data.hashTablep[hash] = scp; scp->flags |= CM_SCACHEFLAG_INHASH; scp->refCount = 1; + lock_ReleaseMutex(&scp->mx); /* XXX - The following fields in the cm_scache are * uninitialized: @@ -579,6 +583,36 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp, return 0; } + +/* Returns a held reference to the scache's parent + * if it exists */ +cm_scache_t * cm_FindSCacheParent(cm_scache_t * scp) +{ + long code = 0; + int i; + cm_fid_t parent_fid; + cm_scache_t * pscp = NULL; + + lock_ObtainWrite(&cm_scacheLock); + parent_fid = scp->fid; + parent_fid.vnode = scp->parentVnode; + parent_fid.unique = scp->parentUnique; + + if (cm_FidCmp(&scp->fid, &parent_fid)) { + for (i=0; inextp) { + if (!cm_FidCmp(&pscp->fid, &parent_fid)) { + cm_HoldSCacheNoLock(pscp); + break; + } + } + } + } + lock_ReleaseWrite(&cm_scacheLock); + + return pscp; +} + /* synchronize a fetch, store, read, write, fetch status or store status. * Called with scache mutex held, and returns with it held, but temporarily * drops it during the fetch. @@ -1180,9 +1214,13 @@ void cm_DiscardSCache(cm_scache_t *scp) scp->cbServerp = NULL; } scp->cbExpires = 0; + scp->flags &= ~CM_SCACHEFLAG_CALLBACK; cm_dnlcPurgedp(scp); cm_dnlcPurgevp(scp); cm_FreeAllACLEnts(scp); + + /* Force mount points and symlinks to be re-evaluated */ + scp->mountPointStringp[0] = '\0'; } void cm_AFSFidFromFid(AFSFid *afsFidp, cm_fid_t *fidp) diff --git a/src/WINNT/afsd/cm_scache.h b/src/WINNT/afsd/cm_scache.h index 48a8778ec..4a97a88c0 100644 --- a/src/WINNT/afsd/cm_scache.h +++ b/src/WINNT/afsd/cm_scache.h @@ -315,6 +315,8 @@ extern void cm_ReleaseSCache(cm_scache_t *); extern cm_scache_t *cm_FindSCache(cm_fid_t *fidp); +extern cm_scache_t *cm_FindSCacheParent(cm_scache_t *); + extern osi_rwlock_t cm_scacheLock; extern osi_queue_t *cm_allFileLocks; diff --git a/src/WINNT/afsd/cm_vnodeops.c b/src/WINNT/afsd/cm_vnodeops.c index 0be176fb6..d9541c9a2 100644 --- a/src/WINNT/afsd/cm_vnodeops.c +++ b/src/WINNT/afsd/cm_vnodeops.c @@ -1180,7 +1180,7 @@ long cm_LookupInternal(cm_scache_t *dscp, char *namep, long flags, cm_user_t *us if ( !dnlcHit && !(flags & CM_FLAG_NOMOUNTCHASE) && rock.ExactFound ) { /* lock the directory entry to prevent racing callback revokes */ lock_ObtainMutex(&dscp->mx); - if ( dscp->cbServerp && dscp->cbExpires ) + if ( dscp->cbServerp != NULL && dscp->cbExpires > 0 ) cm_dnlcEnter(dscp, namep, tscp); lock_ReleaseMutex(&dscp->mx); } diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index 268ed5cf0..88af17cd6 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -38,7 +38,6 @@ static char *illegalChars = "\\/:*?\"<>|"; BOOL isWindows2000 = FALSE; -smb_vc_t *dead_vcp = NULL; smb_vc_t *active_vcp = NULL; int smbShutdownFlag = 0; @@ -1329,7 +1328,7 @@ smb_fid_t *smb_FindFID(smb_vc_t *vcp, unsigned short fid, int flags) if (fid == fidp->fid) { if (newFid) { fid++; - if (fid == 0) { + if (fid == 0xFFFF) { osi_Log1(smb_logp, "New FID number wraps on vcp 0x%x", vcp); fid = 1; @@ -1350,7 +1349,7 @@ smb_fid_t *smb_FindFID(smb_vc_t *vcp, unsigned short fid, int flags) osi_Log1(smb_logp, "Event Object Already Exists: %s", osi_LogSaveString(smb_logp, eventName)); thrd_CloseHandle(event); fid++; - if (fid == 0) { + if (fid == 0xFFFF) { osi_Log1(smb_logp, "New FID wraps around for vcp 0x%x", vcp); fid = 1; } @@ -1913,7 +1912,9 @@ void smb_GCDirSearches(int isV3) */ if (tp->refCount == 0 && (isV3 || tp->cookie <= 255)) { /* hold and delete */ + lock_ObtainMutex(&tp->mx); tp->flags |= SMB_DIRSEARCH_DELETE; + lock_ReleaseMutex(&tp->mx); victimsp[victimCount++] = tp; tp->refCount++; } @@ -2481,27 +2482,16 @@ void smb_SendPacket(smb_vc_t *vcp, smb_packet_t *inp) s = "unknown error"; } osi_Log2(smb_logp, "SendPacket failure code %d \"%s\"", code, s); - osi_Log2(smb_logp, "setting dead_vcp 0x%x, user struct 0x%x", + osi_Log2(smb_logp, "marking dead vcp 0x%x, user struct 0x%x", vcp, vcp->usersp); - smb_HoldVC(vcp); + + lock_ObtainWrite(&smb_globalLock); lock_ObtainMutex(&vcp->mx); vcp->flags |= SMB_VCFLAG_ALREADYDEAD; + dead_sessions[vcp->session] = TRUE; lock_ReleaseMutex(&vcp->mx); - - lock_ObtainWrite(&smb_globalLock); - if (dead_vcp) { - smb_vc_t * dvcp = dead_vcp; - dead_vcp = vcp; - dead_sessions[vcp->session] = TRUE; - lock_ReleaseWrite(&smb_globalLock); - osi_Log1(smb_logp,"Previous dead_vcp %x", dvcp); - smb_CleanupDeadVC(dvcp); - smb_ReleaseVC(dvcp); - } else { - dead_vcp = vcp; - dead_sessions[vcp->session] = TRUE; - lock_ReleaseWrite(&smb_globalLock); - } + lock_ReleaseWrite(&smb_globalLock); + smb_CleanupDeadVC(vcp); } if (localNCB) @@ -3092,6 +3082,7 @@ long smb_ReceiveNegotiate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) tcounter++; /* which proto entry we're looking at */ } + lock_ObtainMutex(&vcp->mx); if (NTProtoIndex != -1) { protoIndex = NTProtoIndex; vcp->flags |= (SMB_VCFLAG_USENT | SMB_VCFLAG_USEV3); @@ -3105,6 +3096,7 @@ long smb_ReceiveNegotiate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) vcp->flags |= SMB_VCFLAG_USECORE; } else protoIndex = -1; + lock_ReleaseMutex(&vcp->mx); if (protoIndex == -1) return CM_ERROR_INVAL; @@ -6281,10 +6273,10 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) truncAttr.length.HighPart = 0; lock_ObtainMutex(&fidp->mx); code = cm_SetAttr(fidp->scp, &truncAttr, userp, &req); + fidp->flags |= SMB_FID_LENGTHSETDONE; lock_ReleaseMutex(&fidp->mx); smb_SetSMBParm(outp, 0, /* count */ 0); smb_SetSMBDataLength(outp, 0); - fidp->flags |= SMB_FID_LENGTHSETDONE; goto done; } @@ -7740,28 +7732,16 @@ void smb_Server(VOID *parmp) vcp = smb_FindVC(ncbp->ncb_lsn, 0, lanas[idx_session]); if (vcp) { lock_ObtainWrite(&smb_globalLock); - if (dead_vcp == vcp) { - osi_Log1(smb_logp, "dead_vcp already set, 0x%x", dead_vcp); - lock_ReleaseWrite(&smb_globalLock); - } else if (!(vcp->flags & SMB_VCFLAG_ALREADYDEAD)) { - osi_Log2(smb_logp, "setting dead_vcp 0x%x, user struct 0x%x", + if (!(vcp->flags & SMB_VCFLAG_ALREADYDEAD)) { + osi_Log2(smb_logp, "marking dead vcp 0x%x, user struct 0x%x", vcp, vcp->usersp); - if (dead_vcp) { - smb_vc_t * dvcp = dead_vcp; - dead_vcp = vcp; /* transfer the reference */ - dead_sessions[vcp->session] = TRUE; - lock_ReleaseWrite(&smb_globalLock); - osi_Log1(smb_logp,"Previous dead_vcp %x", dvcp); - smb_CleanupDeadVC(dvcp); - smb_ReleaseVC(dvcp); - } else { - dead_vcp = vcp; /* transfer the reference */ - dead_sessions[vcp->session] = TRUE; - lock_ReleaseWrite(&smb_globalLock); - } lock_ObtainMutex(&vcp->mx); vcp->flags |= SMB_VCFLAG_ALREADYDEAD; + dead_sessions[vcp->session] = TRUE; lock_ReleaseMutex(&vcp->mx); + lock_ReleaseWrite(&smb_globalLock); + smb_CleanupDeadVC(vcp); + smb_ReleaseVC(vcp); vcp = NULL; } } @@ -7808,25 +7788,16 @@ void smb_Server(VOID *parmp) if (vcp && vcp->errorCount++ > 3) { osi_Log2(smb_logp, "session [ %d ] closed, vcp->errorCount = %d", idx_session, vcp->errorCount); lock_ObtainWrite(&smb_globalLock); - if (dead_vcp == vcp) { - osi_Log1(smb_logp, "dead_vcp already set, 0x%x", dead_vcp); - lock_ReleaseWrite(&smb_globalLock); - } else if (!(vcp->flags & SMB_VCFLAG_ALREADYDEAD)) { - osi_Log2(smb_logp, "setting dead_vcp 0x%x, user struct 0x%x", + if (!(vcp->flags & SMB_VCFLAG_ALREADYDEAD)) { + osi_Log2(smb_logp, "marking dead vcp 0x%x, user struct 0x%x", vcp, vcp->usersp); - if (dead_vcp) { - smb_vc_t * dvcp = dead_vcp; - dead_vcp = vcp; /* transfer reference */ - dead_sessions[vcp->session] = TRUE; - lock_ReleaseWrite(&smb_globalLock); - - osi_Log1(smb_logp,"Previous dead_vcp %x", dvcp); - smb_CleanupDeadVC(dvcp); - smb_ReleaseVC(dvcp); - } lock_ObtainMutex(&vcp->mx); vcp->flags |= SMB_VCFLAG_ALREADYDEAD; + dead_sessions[vcp->session] = TRUE; lock_ReleaseMutex(&vcp->mx); + lock_ReleaseWrite(&smb_globalLock); + smb_CleanupDeadVC(vcp); + smb_ReleaseVC(vcp); vcp = NULL; } goto doneWithNCB; -- 2.39.5