From 7c2285d7a22f3cde119f59fa8135b232be3a7648 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Thu, 1 Jun 2006 16:42:02 +0000 Subject: [PATCH] DEVEL15-windows-smb-fid-faster-20060531 Speed up the performance of the cache manager by not holding the smb_fid_t mutex across calls to cm_SyncOp and AFS RPCs. Ensure that all smb_fid_t flag references are protected. (cherry picked from commit 87313c96b1271b4730a27dcee1b6c062b0a37425) --- src/WINNT/afsd/cm_scache.c | 3 +- src/WINNT/afsd/smb.c | 86 ++++++++++++++++++++++++-------------- src/WINNT/afsd/smb3.c | 42 ++++++++++++++----- 3 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c index 18b599a26c..54142327b1 100644 --- a/src/WINNT/afsd/cm_scache.c +++ b/src/WINNT/afsd/cm_scache.c @@ -853,7 +853,8 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req if (!cm_HaveCallback(scp)) { osi_Log1(afsd_logp, "CM SyncOp getting callback on scp 0x%p", scp); - if (bufLocked) lock_ReleaseMutex(&bufp->mx); + if (bufLocked) + lock_ReleaseMutex(&bufp->mx); code = cm_GetCallback(scp, userp, reqp, 0); if (bufLocked) { lock_ReleaseMutex(&scp->mx); diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index bea1b0f8da..d23df4b73a 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -5692,11 +5692,16 @@ long smb_ReceiveCoreFlush(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) userp = smb_GetUserFromVCP(vcp, inp); lock_ObtainMutex(&fidp->mx); - if (fidp->flags & SMB_FID_OPENWRITE) - code = cm_FSync(fidp->scp, userp, &req); - else + if (fidp->flags & SMB_FID_OPENWRITE) { + cm_scache_t * scp = fidp->scp; + cm_HoldSCache(scp); + lock_ReleaseMutex(&fidp->mx); + code = cm_FSync(scp, userp, &req); + cm_ReleaseSCache(scp); + } else { code = 0; - lock_ReleaseMutex(&fidp->mx); + lock_ReleaseMutex(&fidp->mx); + } smb_ReleaseFID(fidp); @@ -5758,6 +5763,7 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, cm_req_t req; cm_scache_t *dscp = fidp->NTopen_dscp; char *pathp = fidp->NTopen_pathp; + cm_scache_t * scp; osi_Log3(smb_logp, "smb_CloseFID Closing fidp 0x%x (fid=%d vcp=0x%x)", fidp, fidp->fid, vcp); @@ -5794,33 +5800,38 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, lock_ObtainMutex(&fidp->mx); } + scp = fidp->scp; + if (scp) + cm_HoldSCache(scp); + /* watch for ioctl closes, and read-only opens */ - if (fidp->scp != NULL && + if (scp != NULL && (fidp->flags & (SMB_FID_OPENWRITE | SMB_FID_DELONCLOSE)) == SMB_FID_OPENWRITE) { if (dosTime != 0 && dosTime != -1) { - fidp->scp->mask |= CM_SCACHEMASK_CLIENTMODTIME; + scp->mask |= CM_SCACHEMASK_CLIENTMODTIME; /* This fixes defect 10958 */ CompensateForSmbClientLastWriteTimeBugs(&dosTime); smb_UnixTimeFromDosUTime(&fidp->scp->clientModTime, dosTime); } - code = cm_FSync(fidp->scp, userp, &req); + lock_ReleaseMutex(&fidp->mx); + code = cm_FSync(scp, userp, &req); + lock_ObtainMutex(&fidp->mx); } else code = 0; /* unlock any pending locks */ - if (!(fidp->flags & SMB_FID_IOCTL) && fidp->scp && - fidp->scp->fileType == CM_SCACHETYPE_FILE) { + if (!(fidp->flags & SMB_FID_IOCTL) && scp && + scp->fileType == CM_SCACHETYPE_FILE) { cm_key_t key; - cm_scache_t * scp; long tcode; - /* CM_UNLOCK_BY_FID doesn't look at the process ID. We pass + lock_ReleaseMutex(&fidp->mx); + + /* CM_UNLOCK_BY_FID doesn't look at the process ID. We pass in zero. */ key = cm_GenerateKey(vcp->vcID, 0, fidp->fid); - scp = fidp->scp; - cm_HoldSCache(scp); lock_ObtainMutex(&scp->mx); tcode = cm_SyncOp(scp, NULL, userp, &req, 0, @@ -5841,14 +5852,15 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, post_syncopdone: lock_ReleaseMutex(&scp->mx); - cm_ReleaseSCache(scp); + lock_ObtainMutex(&fidp->mx); } if (fidp->flags & SMB_FID_DELONCLOSE) { char *fullPathp; - smb_FullName(dscp, fidp->scp, pathp, &fullPathp, userp, &req); - if (fidp->scp->fileType == CM_SCACHETYPE_DIRECTORY) { + lock_ReleaseMutex(&fidp->mx); + smb_FullName(dscp, scp, pathp, &fullPathp, userp, &req); + if (scp->fileType == CM_SCACHETYPE_DIRECTORY) { code = cm_RemoveDir(dscp, fullPathp, userp, &req); if (code == 0 && (dscp->flags & CM_SCACHEFLAG_ANYWATCH)) smb_NotifyChange(FILE_ACTION_REMOVED, @@ -5862,16 +5874,17 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, dscp, fullPathp, NULL, TRUE); } free(fullPathp); + lock_ObtainMutex(&fidp->mx); fidp->flags &= ~SMB_FID_DELONCLOSE; } /* if this was a newly created file, then clear the creator * in the stat cache entry. */ if (fidp->flags & SMB_FID_CREATED) { - lock_ObtainMutex(&fidp->scp->mx); - if (fidp->scp->creator == userp) - fidp->scp->creator = NULL; - lock_ReleaseMutex(&fidp->scp->mx); + lock_ObtainMutex(&scp->mx); + if (scp->creator == userp) + scp->creator = NULL; + lock_ReleaseMutex(&scp->mx); fidp->flags &= ~SMB_FID_CREATED; } @@ -5889,6 +5902,9 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp, if (dscp) cm_ReleaseSCache(dscp); + if (scp) + cm_ReleaseSCache(scp); + if (pathp) free(pathp); @@ -5965,6 +5981,7 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, if (fidp->curr_chunk == fidp->prev_chunk + 1) sequential = 1; } + lock_ReleaseMutex(&fidp->mx); /* start by looking up the file's end */ code = cm_SyncOp(scp, NULL, userp, &req, 0, @@ -6067,7 +6084,6 @@ long smb_ReadData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, done: lock_ReleaseMutex(&scp->mx); - lock_ReleaseMutex(&fidp->mx); if (bufferp) buf_Release(bufferp); @@ -6117,9 +6133,20 @@ long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, offset = *offsetp; lock_ObtainMutex(&fidp->mx); + /* make sure we have a writable FD */ + if (!(fidp->flags & SMB_FID_OPENWRITE)) { + osi_Log2(smb_logp, "smb_WriteData fid %d not OPENWRITE flags 0x%x", + fidp->fid, fidp->flags); + lock_ReleaseMutex(&fidp->mx); + code = CM_ERROR_BADFDOP; + goto done; + } + scp = fidp->scp; - lock_ObtainMutex(&scp->mx); + cm_HoldSCache(scp); + lock_ReleaseMutex(&fidp->mx); + lock_ObtainMutex(&scp->mx); /* start by looking up the file's end */ code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_NEEDCALLBACK @@ -6128,14 +6155,6 @@ long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, if (code) goto done; - /* make sure we have a writable FD */ - if (!(fidp->flags & SMB_FID_OPENWRITE)) { - osi_Log2(smb_logp, "smb_WriteData fid %d not OPENWRITE flags 0x%x", - fidp->fid, fidp->flags); - code = CM_ERROR_BADFDOP; - goto done; - } - /* now we have the entry locked, look up the length */ fileLength = scp->length; minLength = fileLength; @@ -6302,6 +6321,7 @@ long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, buf_Release(bufferp); } + lock_ObtainMutex(&fidp->mx); if (code == 0 && filter != 0 && (fidp->flags & SMB_FID_NTOPEN) && (fidp->NTopen_dscp->flags & CM_SCACHEFLAG_ANYWATCH)) { smb_NotifyChange(FILE_ACTION_MODIFIED, filter, @@ -6323,6 +6343,8 @@ long smb_WriteData(smb_fid_t *fidp, osi_hyper_t *offsetp, long count, char *op, writeBackOffset.HighPart, cm_chunkSize, 0, userp); } + cm_ReleaseSCache(scp); + osi_Log3(smb_logp, "smb_WriteData fid %d returns 0x%x written %d bytes", fidp->fid, code, *writtenp); return code; @@ -7113,6 +7135,8 @@ long smb_ReceiveCoreSeek(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) lock_ObtainMutex(&fidp->mx); scp = fidp->scp; + cm_HoldSCache(scp); + lock_ReleaseMutex(&fidp->mx); lock_ObtainMutex(&scp->mx); code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); @@ -7131,8 +7155,8 @@ long smb_ReceiveCoreSeek(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) smb_SetSMBDataLength(outp, 0); } lock_ReleaseMutex(&scp->mx); - lock_ReleaseMutex(&fidp->mx); smb_ReleaseFID(fidp); + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); return code; } diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index 0675d6896f..5a23c5d00d 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -2869,6 +2869,7 @@ long smb_ReceiveTran2QFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t unsigned short infoLevel; int nbytesRequired; unsigned short fid; + int delonclose = 0; cm_user_t *userp; smb_fid_t *fidp; cm_scache_t *scp; @@ -2920,7 +2921,10 @@ long smb_ReceiveTran2QFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t } lock_ObtainMutex(&fidp->mx); + delonclose = fidp->flags & SMB_FID_DELONCLOSE; scp = fidp->scp; + cm_HoldSCache(scp); + lock_ReleaseMutex(&fidp->mx); lock_ObtainMutex(&scp->mx); code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); @@ -2945,7 +2949,7 @@ long smb_ReceiveTran2QFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t *((LARGE_INTEGER *)op) = scp->length; op += 8; /* alloc size */ *((LARGE_INTEGER *)op) = scp->length; op += 8; /* EOF */ *((u_long *)op) = scp->linkCount; op += 4; - *op++ = ((fidp->flags & SMB_FID_DELONCLOSE) ? 1 : 0); + *op++ = (delonclose ? 1 : 0); *op++ = (scp->fileType == CM_SCACHETYPE_DIRECTORY ? 1 : 0); *op++ = 0; *op++ = 0; @@ -2957,10 +2961,14 @@ long smb_ReceiveTran2QFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t unsigned long len; char *name; + lock_ReleaseMutex(&scp->mx); + lock_ObtainMutex(&fidp->mx); + lock_ObtainMutex(&scp->mx); if (fidp->NTopen_wholepathp) name = fidp->NTopen_wholepathp; else name = "\\"; /* probably can't happen */ + lock_ReleaseMutex(&fidp->mx); len = (unsigned long)strlen(name); outp->totalData = (len*2) + 4; /* this is actually what we want to return */ *((u_long *)op) = len * 2; op += 4; @@ -2970,7 +2978,7 @@ long smb_ReceiveTran2QFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t /* send and free the packets */ done: lock_ReleaseMutex(&scp->mx); - lock_ReleaseMutex(&fidp->mx); + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); smb_ReleaseFID(fidp); if (code == 0) @@ -3027,6 +3035,9 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet smb_SendTran2Error(vcp, p, op, CM_ERROR_NOACCESS); return 0; } + + scp = fidp->scp; + cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); osi_Log1(smb_logp, "T2 SFileInfo type 0x%x", infoLevel); @@ -3043,8 +3054,6 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet goto done; } - scp = fidp->scp; - if (infoLevel == SMB_QUERY_FILE_BASIC_INFO) { FILETIME lastMod; unsigned int attribute; @@ -3053,16 +3062,18 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet /* lock the vnode with a callback; we need the current status * to determine what the new status is, in some cases. */ - lock_ObtainMutex(&fidp->mx); lock_ObtainMutex(&scp->mx); code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_GETSTATUS | CM_SCACHESYNC_NEEDCALLBACK); + lock_ReleaseMutex(&scp->mx); if (code) { - lock_ReleaseMutex(&scp->mx); goto done; } + lock_ObtainMutex(&fidp->mx); + lock_ObtainMutex(&scp->mx); + /* prepare for setattr call */ attr.mask = 0; @@ -3130,6 +3141,7 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet } done: + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); smb_ReleaseFID(fidp); if (code == 0) @@ -4886,6 +4898,8 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } + scp = fidp->scp; + cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); /* set inp->fid so that later read calls in same msg can find fid */ @@ -4893,7 +4907,6 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) userp = smb_GetUserFromVCP(vcp, inp); - scp = fidp->scp; lock_ObtainMutex(&scp->mx); code = cm_SyncOp(scp, NULL, userp, &req, 0, @@ -5092,6 +5105,7 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) doneSync: lock_ReleaseMutex(&scp->mx); + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); smb_ReleaseFID(fidp); @@ -5123,17 +5137,19 @@ long smb_ReceiveV3GetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } + scp = fidp->scp; + cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); userp = smb_GetUserFromVCP(vcp, inp); - scp = fidp->scp; /* otherwise, stat the file */ lock_ObtainMutex(&scp->mx); code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); - if (code) goto done; + if (code) + goto done; /* decode times. We need a search time, but the response to this * call provides the date first, not the time, as returned in the @@ -5162,6 +5178,7 @@ long smb_ReceiveV3GetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * done: lock_ReleaseMutex(&scp->mx); + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); smb_ReleaseFID(fidp); return code; @@ -5194,11 +5211,12 @@ long smb_ReceiveV3SetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } + scp = fidp->scp; + cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); userp = smb_GetUserFromVCP(vcp, inp); - scp = fidp->scp; /* now prepare to call cm_setattr. This message only sets various times, * and AFS only implements mtime, and we'll set the mtime if that's @@ -5219,8 +5237,10 @@ long smb_ReceiveV3SetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * osi_Log1(smb_logp, "**smb_UnixTimeFromSearchTime failed searchTime=%ld", searchTime); } } - else code = 0; + else + code = 0; + cm_ReleaseSCache(scp); cm_ReleaseUser(userp); smb_ReleaseFID(fidp); return code;