From 0c89335b5aa7ae3582862596878936dfcbe99bf1 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Tue, 30 Apr 2013 17:32:26 -0500 Subject: [PATCH 1/1] afs: Refactor GetDSlot parameters The 'indexvalid' and 'datavalid' parameters were really representing 3 different scenarios, not 2 different values with 2 possibilities each. Change these to a single parameter, 'type', with 3 different values: DSLOT_NEW, DSLOT_UNUSED, and DSLOT_VALID. Hopefully this will make the relevant code paths easier to understand. This should incur no functional change; it is just code reorganization. Change-Id: Iac921e74532de89121b461fa0f53ddb778735e0c Reviewed-on: http://gerrit.openafs.org/9834 Tested-by: BuildBot Reviewed-by: Chas Williams - CONTRACTOR Reviewed-by: Daria Brashear --- src/afs/afs_chunkops.h | 20 +++++++++++++++----- src/afs/afs_dcache.c | 44 ++++++++++++++++++++++---------------------- src/afs/afs_prototypes.h | 4 ++-- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/afs/afs_chunkops.h b/src/afs/afs_chunkops.h index 1cedc7d..a68d7d8 100644 --- a/src/afs/afs_chunkops.h +++ b/src/afs/afs_chunkops.h @@ -47,6 +47,15 @@ #define AFS_SETCHUNKSIZE(chunk) { afs_LogChunk = chunk; \ afs_FirstCSize = afs_OtherCSize = (1 << chunk); } +/** + * The states a dcache slot can be in. + */ +typedef enum dslot_state { + DSLOT_NEW = 0, /**< Use to create a slot if needed. */ + DSLOT_UNUSED = 1, /**< Contains a free or discarded dcache. */ + DSLOT_VALID = 2, /**< The dcache is known to exist and be valid. */ +} dslot_state; + /* * Functions exported by a cache type */ @@ -60,7 +69,7 @@ struct afs_cacheOps { int (*close) (struct osi_file * fp); int (*vreadUIO) (afs_dcache_id_t *, struct uio *); int (*vwriteUIO) (struct vcache *, afs_dcache_id_t *, struct uio *); - struct dcache *(*GetDSlot) (afs_int32 aslot, int indexvalid, int datavalid); + struct dcache *(*GetDSlot) (afs_int32 aslot, dslot_state type); struct volume *(*GetVolSlot) (afs_int32 volid, struct cell *cell); int (*HandleLink) (struct vcache * avc, struct vrequest * areq); }; @@ -76,14 +85,15 @@ struct afs_cacheOps { /* Use afs_GetValidDSlot to get a dcache from a dcache slot number when we * know the dcache contains data we want (e.g. it's on the hash table) */ -#define afs_GetValidDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, 1, 1) +#define afs_GetValidDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, DSLOT_VALID) /* Use afs_GetUnusedDSlot when loading a dcache entry that is on the free or * discard lists (the dcache does not contain valid data, but we know the * dcache entry itself exists). */ -#define afs_GetUnusedDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, 1, 0) +#define afs_GetUnusedDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, DSLOT_UNUSED) /* Use afs_GetNewDSlot only when initializing dcache slots (the given slot - * number may not exist at all). */ -#define afs_GetNewDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, 0, 0) + * number may not exist at all). This will always succeed; if we can't find the + * given dslot, we will create a new, empty slot. */ +#define afs_GetNewDSlot(slot) (*(afs_cacheType->GetDSlot))(slot, DSLOT_NEW) /* These memcpys should get optimised to simple assignments when afs_dcache_id_t * is simple */ diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 338e8db..46b3bad 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -2703,14 +2703,14 @@ afs_WriteThroughDSlots(void) * * Parameters: * aslot : Dcache slot to look at. - * needvalid : Whether the specified slot should already exist + * type : What 'type' of dslot to get; see the dslot_state enum * * Environment: * Must be called with afs_xdcache write-locked. */ struct dcache * -afs_MemGetDSlot(afs_int32 aslot, int indexvalid, int datavalid) +afs_MemGetDSlot(afs_int32 aslot, dslot_state type) { struct dcache *tdc; int existing = 0; @@ -2731,10 +2731,14 @@ afs_MemGetDSlot(afs_int32 aslot, int indexvalid, int datavalid) return tdc; } - /* if 'indexvalid' is true, the slot must already exist and be populated - * somewhere. for memcache, the only place that dcache entries exist is - * in memory, so if we did not find it above, something is very wrong. */ - osi_Assert(!indexvalid); + /* if we got here, the given slot is not in memory in our list of known + * slots. for memcache, the only place a dslot can exist is in memory, so + * if the caller is expecting to get back a known dslot, and we've reached + * here, something is very wrong. DSLOT_NEW is the only type of dslot that + * may not exist; for all others, the caller assumes the given dslot + * already exists. so, 'type' had better be DSLOT_NEW here, or something is + * very wrong. */ + osi_Assert(type == DSLOT_NEW); if (!afs_freeDSList) afs_GetDownDSlot(4); @@ -2795,20 +2799,13 @@ unsigned int last_error = 0, lasterrtime = 0; * * Parameters: * aslot : Dcache slot to look at. - * indexvalid : 1 if we know the slot we're giving is valid, and thus - * reading the dcache from the disk index should succeed. 0 - * if we are initializing a new dcache, and so reading from - * the disk index may fail. - * datavalid : 0 if we are loading a dcache entry from the free or - * discard list, so we know the data in the given dcache is - * not valid. 1 if we are loading a known used dcache, so the - * data in the dcache must be valid. + * type : What 'type' of dslot to get; see the dslot_state enum * * Environment: * afs_xdcache lock write-locked. */ struct dcache * -afs_UFSGetDSlot(afs_int32 aslot, int indexvalid, int datavalid) +afs_UFSGetDSlot(afs_int32 aslot, dslot_state type) { afs_int32 code; struct dcache *tdc; @@ -2871,7 +2868,10 @@ afs_UFSGetDSlot(afs_int32 aslot, int indexvalid, int datavalid) last_error = code; #endif lasterrtime = osi_Time(); - if (indexvalid) { + if (type != DSLOT_NEW) { + /* If we are requesting a non-DSLOT_NEW slot, this is an error. + * non-DSLOT_NEW slots are supposed to already exist, so if we + * failed to read in the slot, something is wrong. */ struct osi_stat tstat; if (afs_osi_Stat(afs_cacheInodep, &tstat)) { tstat.size = -1; @@ -2891,22 +2891,22 @@ afs_UFSGetDSlot(afs_int32 aslot, int indexvalid, int datavalid) } if (!afs_CellNumValid(tdc->f.fid.Cell)) { entryok = 0; - if (datavalid) { + if (type == DSLOT_VALID) { osi_Panic("afs: needed valid dcache but index %d off %d has " "invalid cell num %d\n", (int)aslot, off, (int)tdc->f.fid.Cell); } } - if (datavalid && tdc->f.fid.Fid.Volume == 0) { + if (type == DSLOT_VALID && tdc->f.fid.Fid.Volume == 0) { osi_Panic("afs: invalid zero-volume dcache entry at slot %d off %d", (int)aslot, off); } - if (indexvalid && !datavalid) { - /* we know that the given dslot does exist, but the data in it is not - * valid. this only occurs when we pull a dslot from the free or - * discard list, so be sure not to re-use the data; force invalidation. + if (type == DSLOT_UNUSED) { + /* the requested dslot is known to exist, but contain invalid data + * (this happens when we're using a dslot from the free or discard + * list). be sure not to re-use the data in it, so force invalidation. */ entryok = 0; } diff --git a/src/afs/afs_prototypes.h b/src/afs/afs_prototypes.h index 0ad3e69..636151e 100644 --- a/src/afs/afs_prototypes.h +++ b/src/afs/afs_prototypes.h @@ -252,7 +252,7 @@ extern void afs_FlushDCache(struct dcache *adc); extern void shutdown_dcache(void); extern void afs_CacheTruncateDaemon(void); extern afs_int32 afs_fsfragsize; -extern struct dcache *afs_MemGetDSlot(afs_int32 aslot, int indexvalid, int datavalid); +extern struct dcache *afs_MemGetDSlot(afs_int32 aslot, dslot_state type); extern struct dcache *afs_GetDCache(struct vcache *avc, afs_size_t abyte, struct vrequest *areq, @@ -274,7 +274,7 @@ extern void afs_TryToSmush(struct vcache *avc, extern void updateV2DC(int lockVc, struct vcache *v, struct dcache *d, int src); extern void afs_WriteThroughDSlots(void); -extern struct dcache *afs_UFSGetDSlot(afs_int32 aslot, int indexvalid, int datavalid); +extern struct dcache *afs_UFSGetDSlot(afs_int32 aslot, dslot_state type); extern int afs_WriteDCache(struct dcache *adc, int atime); extern int afs_wakeup(struct vcache *avc); extern int afs_InitCacheFile(char *afile, ino_t ainode); -- 1.9.4