mirror of
https://git.openafs.org/openafs.git
synced 2025-02-01 05:57:43 +00:00
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<NUL>" 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. Reviewed-on: http://gerrit.openafs.org/6882 Reviewed-by: Derrick Brashear <shadow@dementix.org> Tested-by: BuildBot <buildbot@rampaginggeek.com> (cherry picked from commit e53221d9a82fd8e3d545704abae51cc844bc31a3) Change-Id: I81b5a3a6ff745f3f53988a1a4e5d3df20f5df6d3 Reviewed-on: http://gerrit.openafs.org/7994 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Derrick Brashear <shadow@dementix.org>
This commit is contained in:
parent
a17223f96e
commit
f899af247c
@ -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 ); */
|
||||
|
Loading…
x
Reference in New Issue
Block a user