OpenAFS Master Repository branch, openafs-stable-1_6_x, updated. openafs-stable-1_6_1pre2-206-g3ce1c53
Gerrit Code Review
gerrit@openafs.org
Wed, 5 Dec 2012 01:30:49 -0800 (PST)
The following commit has been merged in the openafs-stable-1_6_x branch:
commit 3ce1c5394fd96dde5cfb4ad16b90489324bf51a6
Author: Andrew Deason <adeason@sinenomine.net>
Date: Wed Oct 31 15:55:35 2012 -0500
afs: Never use GetNewDSlot after init
Currently there are two ways to get a dcache via a slot number:
afs_GetNewDSot and afs_GetValidDSlot. afs_GetValidDSlot assumes that
the given slot number refers to a dcache entry that is valid on disk;
with afs_GetNewDSlot, the given slot may not be valid, and if it is
not, an empty 'template' dcache is returned.
afs_GetNewDSlot is useful for initializing cache files, since if a
given dcache slot exists on disk and contains valid data, we use the
dcache like normal. If it does not already exist or does not contain
valid data, we fill in the missing data after afs_GetNewDSlot returns.
However, for all other uses, afs_GetNewDSlot is incorrect, and causes
various serious problems. After we have initialized our dcache
entries, any attempt to read a dcache by slot number should succeed,
since the number of dcache entries never changes after we are started,
and we initialized all of them during client startup.
Some code outside of afs_InitCacheFile was still using
afs_GetNewDSlot; code that reads in a dslot from the free or discard
list. In these cases, if there is any error reading the dcache slot
from disk, we will be given a dcache that has some of its fields not
filled in properly. Notably, we assume that the entry is not on the
global hash table (we set tdc->f.fid.Fid.Volume to 0), and the
tdc->f.inode field is not initialized at all, leaving it set to
whatever was in memory for that tdc before we tried to read the slot
from disk.
This can cause cache corruption, since tdc->f.inode can point to the
inoder for a different existing cache file, so writing to that dcache
modifies the data for another cached file.
To avoid this, modify the non-afs_InitCacheFile callers of
afs_GetNewDSlot to avoid afs_GetNewDSlot. Since these callers read
from the free/discard list, the contents of the dcache entries are not
valid (the cell, volume, dv, etc are not valid), though they must
exist on disk (we have a valid inode number for them). So, create a
new function, afs_GetUnusedDSlot, to get a dcache that must exist on
disk, but does not represent any valid data. Use this for callers that
must get a dslot from the free/discard list.
Add some comments to try and help explain what is going on.
Reviewed-on: http://gerrit.openafs.org/8370
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 20b0c65a289e2b55fb6922c8f60e873f1f4c6f97)
Change-Id: I0ed66c155ea5574fd88c288bdf9feb98161e5c45
Reviewed-on: http://gerrit.openafs.org/8545
Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Paul Smeddle <paul.smeddle@gmail.com>
src/afs/afs_chunkops.h | 15 ++++++++++++---
src/afs/afs_dcache.c | 34 ++++++++++++++++++++++------------
src/afs/afs_prototypes.h | 4 ++--
3 files changed, 36 insertions(+), 17 deletions(-)
--
OpenAFS Master Repository