From e53221d9a82fd8e3d545704abae51cc844bc31a3 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Fri, 2 Mar 2012 17:22:12 -0600 Subject: [PATCH] afs: Do not limit fetches based on vcache length Currently, when we go to the fileserver to fetch some data, we try to make sure that we do not ask for data beyond the end of the file. For example, if our chunk size is 1M, and we need to get the first chunk for a file that is 4 bytes long, we will only ask the fileserver for 4 bytes. This can cause issues when the file is being extended at the same time as when we are trying to read the file. Consider the following example. There is a file named X that has contents "abcd" at dv 1, and we issue a FetchData64 request for X, only requesting 4 bytes. Right before the fileserver gets the FetchData64 request, another client writes the contents "12345" to file X. The client will then fetch the contents "1234" for that file, at dv 2, and store that as the contents of the first chunk for file X. On subsequent reads for file X, applications will now get "1234" as the contents, since the size of the file will be updated to 5, but the cache manager thinks that "1234" is the correct contents for the first chunk of X at dv 2. The cache manager will continue to think so until the cache entry is evicted or invalidated for whatever reason. To avoid this scenario, always request a full chunk of data if we have any data to fetch and the file has not been locally truncated. We can still avoid the fetch at all if it looks like we're fetching beyond end-of-file, since we know that at least at some point that was correct information about the file. If this results in us trying to fetch beyond end-of-file, the fileserver will respond with the correct length anyway. We still need to restrict the fetch request length based on avc->f.truncPos, since the dcache data after avc->f.truncPos needs to stay empty, since we don't track truncated data any other way. If we also avoided this restriction, extending a file via truncation after reducing a file's length via truncation could cause the old file data to appear again, instead of filling the new file range with NULs. Note that on at least Linux, with this fix an application can still read the contents "1234" on the first read in the above example, and "12345" on subsequent reads. This is just due to when we give the VFS updates about file metadata, and could be remedied by updating file metadata immediately from the FetchStatus information from the FetchData64 call. However, just reading the contents "1234" in the above example seems like a somewhat plausible outcome; at the very least, it is an improvement. Change-Id: I158593502ac96ba2c856a0b5997355a53d4173aa Reviewed-on: http://gerrit.openafs.org/6882 Reviewed-by: Derrick Brashear Tested-by: BuildBot --- src/afs/afs_dcache.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 3fc0bb546..be80c7af1 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -2111,6 +2111,32 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, afs_AdjustSize(tdc, size); /* changes chunkBytes */ /* max size for transfer still in size */ } + + if (size) { + /* For the actual fetch, do not limit the request to the + * length of the file. If this results in a read past EOF on + * the server, the server will just reply with less data than + * requested. If we limit ourselves to only requesting data up + * to the avc file length, we open ourselves up to races if the + * file is extended on the server at about the same time. + * + * However, we must restrict ourselves to the avc->f.truncPos + * length, since this represents an outstanding local + * truncation of the file that will be committed to the + * fileserver when we actually write the fileserver contents. + * If we do not restrict the fetch length based on + * avc->f.truncPos, a different truncate operation extending + * the file length could cause the old data after + * avc->f.truncPos to reappear, instead of extending the file + * with NUL bytes. */ + size = AFS_CHUNKSIZE(abyte); + if (Position + size > avc->f.truncPos) { + size = avc->f.truncPos - Position; + } + if (size < 0) { + size = 0; + } + } } if (afs_mariner && !tdc->f.chunk) afs_MarinerLog("fetch$Fetching", avc); /* , Position, size, afs_indexCounter ); */ -- 2.39.5