From 37edca7a4030d9c2f5153130b62f893f88fa2541 Mon Sep 17 00:00:00 2001 From: Nickolai Zeldovich Date: Tue, 7 Aug 2001 00:39:29 +0000 Subject: [PATCH] solaris-locking-cleanup-20010806 reduce afs vnode lock contention, also implements async page requests "(In afs_GetDCache, the hints in the vnode are only updated if we can grab the write lock without blocking. In afs_GetOnePage, we only grab the read lock, rather than the shared lock -- as far as I can tell, there's nothing that needs the write lock.) FWIW, the particular case where I was being bitten by this lock contention was playing an mp3 from AFS space and at the same time copying it to local disk. The copy kept fetching chunks while holding the read lock, so the mp3 player couldn't grab a write lock in the page fault, even though the data was already in cache. While I'm not fully familiar with the semantics of afs vnode locks [do they even exist? :-)], I believe changing from shared to read locks in afs_GetOnePage should be safe." --- src/afs/SOLARIS/osi_vnodeops.c | 60 ++++++++++++++++++++++------------ src/afs/afs_dcache.c | 20 +++++++----- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/afs/SOLARIS/osi_vnodeops.c b/src/afs/SOLARIS/osi_vnodeops.c index 19cf47f60..19cebd06c 100644 --- a/src/afs/SOLARIS/osi_vnodeops.c +++ b/src/afs/SOLARIS/osi_vnodeops.c @@ -265,10 +265,6 @@ struct AFS_UCRED *acred; afs_int32 toffset; #endif - if (!pl) return 0; /* punt asynch requests */ - - len = PAGESIZE; - pl[0] = NULL; /* Make sure it's empty */ if (!acred) #ifdef AFS_SUN5_ENV osi_Panic("GetOnePage: !acred"); @@ -276,9 +272,36 @@ struct AFS_UCRED *acred; acred = u.u_cred; /* better than nothing */ #endif - /* first, obtain the proper lock for the VM system */ avc = (struct vcache *) vp; /* cast to afs vnode */ +#ifdef AFS_SUN5_ENV + if (avc->credp /*&& AFS_NFSXLATORREQ(acred)*/ && AFS_NFSXLATORREQ(avc->credp)) { + acred = avc->credp; + } +#endif + if (code = afs_InitReq(&treq, acred)) return code; + + if (!pl) { + /* + * This is a read-ahead request, e.g. due to madvise. + */ + tdc = afs_GetDCache(avc, (afs_int32)off, &treq, &offset, &nlen, 1); + if (!tdc) return 0; + + if (!(tdc->flags & DFNextStarted)) { + ObtainReadLock(&avc->lock); + afs_PrefetchChunk(avc, tdc, acred, &treq); + ReleaseReadLock(&avc->lock); + } + afs_PutDCache(tdc); + return 0; + } + + len = PAGESIZE; + pl[0] = NULL; /* Make sure it's empty */ + + /* first, obtain the proper lock for the VM system */ + /* if this is a read request, map the page in read-only. This will * allow us to swap out the dcache entry if there are only read-only * pages created for the chunk, which helps a *lot* when dealing @@ -290,12 +313,6 @@ struct AFS_UCRED *acred; mapForRead = 1; if (protp) *protp = PROT_ALL; -#ifdef AFS_SUN5_ENV - if (avc->credp /*&& AFS_NFSXLATORREQ(acred)*/ && AFS_NFSXLATORREQ(avc->credp)) { - acred = avc->credp; - } -#endif - if (code = afs_InitReq(&treq, acred)) return code; #ifndef AFS_SUN5_ENV if (AFS_NFSXLATORREQ(acred)) { if (rw == S_READ) { @@ -323,7 +340,7 @@ retry: } afs_BozonLock(&avc->pvnLock, avc); - ObtainSharedLock(&avc->lock,566); + ObtainReadLock(&avc->lock); afs_Trace4(afs_iclSetp, CM_TRACE_PAGEIN, ICL_TYPE_POINTER, (afs_int32) vp, ICL_TYPE_LONG, (afs_int32) off, ICL_TYPE_LONG, (afs_int32) len, @@ -337,7 +354,7 @@ retry: * the locks and try again when the VM purge is done. */ ObtainWriteLock(&avc->vlock, 550); if (avc->activeV) { - ReleaseSharedLock(&avc->lock); + ReleaseReadLock(&avc->lock); ReleaseWriteLock(&avc->vlock); afs_BozonUnlock(&avc->pvnLock, avc); afs_PutDCache(tdc); @@ -359,7 +376,7 @@ retry: /* Check to see whether the cache entry is still valid */ if (!(avc->states & CStatd) || !hsame(avc->m.DataVersion, tdc->f.versionNo)) { - ReleaseSharedLock(&avc->lock); + ReleaseReadLock(&avc->lock); afs_BozonUnlock(&avc->pvnLock, avc); afs_PutDCache(tdc); goto retry; @@ -438,19 +455,17 @@ retry: buf->b_blkno = btodb(toffset); bp_mapin(buf); /* map it in to our address space */ #ifndef AFS_SUN5_ENV - ReleaseSharedLock(&avc->lock); + ReleaseReadLock(&avc->lock); #endif #if defined(AFS_SUN5_ENV) AFS_GLOCK(); - UpgradeSToWLock(&avc->lock, 564); code = afs_ustrategy(buf, acred); /* do the I/O */ - ConvertWToSLock(&avc->lock); AFS_GUNLOCK(); #else code = afs_ustrategy(buf); /* do the I/O */ #endif #ifndef AFS_SUN5_ENV - ObtainSharedLock(&avc->lock,245); + ObtainReadLock(&avc->lock); #endif #ifdef AFS_SUN5_ENV /* Before freeing unmap the buffer */ @@ -495,8 +510,13 @@ retry: AFS_GLOCK(); pl[slot] = (struct page *) 0; + /* + * XXX This seems kind-of wrong: we shouldn't be modifying + * avc->states while not holding the write lock (even + * though nothing really uses CHasPages..) + */ avc->states |= CHasPages; - ReleaseSharedLock(&avc->lock); + ReleaseReadLock(&avc->lock); #ifdef AFS_SUN5_ENV ObtainWriteLock(&afs_xdcache,246); if (!mapForRead) { @@ -526,7 +546,7 @@ retry: for(i=0; ilock); + ReleaseReadLock(&avc->lock); afs_BozonUnlock(&avc->pvnLock, avc); #ifdef AFS_SUN5_ENV afs_PutDCache(tdc); diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 318284820..1f1486055 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -1432,14 +1432,18 @@ struct tlocal1 { /* these fields are protected by the lock on the vcache and luck * on the dcache */ -#define updateV2DC(l,v,d,src) { if (l) ObtainWriteLock(&((v)->lock),src);\ - if (hsame((v)->m.DataVersion, (d)->f.versionNo) && (v)->callback) { \ - (v)->quick.dc = (d); \ - (v)->quick.stamp = (d)->stamp = MakeStamp(); \ - (v)->quick.minLoc = AFS_CHUNKTOBASE((d)->f.chunk); \ - /* Don't think I need these next two lines forever */ \ - (v)->quick.len = (d)->f.chunkBytes; \ - (v)->h1.dchint = (d); } if(l) ReleaseWriteLock(&((v)->lock)); } +#define updateV2DC(l,v,d,src) { \ + if (!l || 0 == NBObtainWriteLock(&((v)->lock),src)) { \ + if (hsame((v)->m.DataVersion, (d)->f.versionNo) && (v)->callback) { \ + (v)->quick.dc = (d); \ + (v)->quick.stamp = (d)->stamp = MakeStamp(); \ + (v)->quick.minLoc = AFS_CHUNKTOBASE((d)->f.chunk); \ + /* Don't think I need these next two lines forever */ \ + (v)->quick.len = (d)->f.chunkBytes; \ + (v)->h1.dchint = (d); \ + } \ + if(l) ReleaseWriteLock(&((v)->lock)); \ + } } struct dcache *afs_GetDCache(avc, abyte, areq, aoffset, alen, aflags) register struct vcache *avc; /*Held*/ -- 2.39.5