From 95770bf95ad766207252ea0c2d2b5ab2415c896f Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Wed, 4 Nov 2009 23:40:39 +0000 Subject: [PATCH] Prevent VLRUQ race in ShakeLooseVCaches When ShakeLooseVCaches is called from afs_Daemon, the xvcache lock is not held. This means that if the GLOCK is dropped for any reason (for example, whilst purging the dentry cache), then ShakeLooseVCaches can be raced, end we can end up attempting to flush the same vcache twice. The symptoms of this in Linux are that we oops in clear_inode. Get the xvcache lock in afs_Daemon(), before calling ShakeLooseVCaches. Also, remove the conditional GLOCK code from that function. If we don't have the GLOCK on entry, then we're really in trouble (and both code paths - afs_Daemon and afs_NewVCache should get the GLOCK for us, anyway) FIXES 125589 Change-Id: I3fe5b41a661cd162ec73c51492925ad87c6d4c13 Reviewed-on: http://gerrit.openafs.org/781 Reviewed-by: Marc Dionne Tested-by: Derrick Brashear Reviewed-by: Derrick Brashear --- src/afs/afs_daemons.c | 6 ++++++ src/afs/afs_vcache.c | 9 --------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/afs/afs_daemons.c b/src/afs/afs_daemons.c index 21cf93109c..db5f2a883c 100644 --- a/src/afs/afs_daemons.c +++ b/src/afs/afs_daemons.c @@ -127,6 +127,10 @@ afs_CheckServerDaemon(void) extern int vfs_context_ref; +/* This function always holds the GLOCK whilst it is running. The caller + * gets the GLOCK before invoking it, and afs_osi_Sleep drops the GLOCK + * whilst we are sleeping, and regains it when we're woken up. + */ void afs_Daemon(void) { @@ -219,7 +223,9 @@ afs_Daemon(void) else anumber = VCACHE_FREE + (afs_maxvcount - afs_cacheStats); + ObtainWriteLock(&afs_xvcache, 734); afs_ShakeLooseVCaches(anumber); + ReleaseWriteLock(&afs_xvcache); last5MinCheck = now; } diff --git a/src/afs/afs_vcache.c b/src/afs/afs_vcache.c index 5d070943a5..6d69b07d6d 100644 --- a/src/afs/afs_vcache.c +++ b/src/afs/afs_vcache.c @@ -634,13 +634,6 @@ afs_ShakeLooseVCaches(afs_int32 anumber) struct afs_q *tq, *uq; int code, fv_slept; afs_int32 target = anumber; - int haveGlock = 1; - - /* Should probably deal better */ - if (!ISAFS_GLOCK()) { - haveGlock = 0; - AFS_GLOCK(); - } if (afsd_dynamic_vcaches || afs_vcount >= afs_maxvcount) { i = 0; @@ -723,8 +716,6 @@ restart: /* printf("recycled %d entries\n", target-anumber); */ - if (!haveGlock) - AFS_GUNLOCK(); #endif return 0; }