Commit d31ea435 authored by Steve French's avatar Steve French

Fix i_size corruption in case of overlapped readdir changing cached file size...

Fix i_size corruption in case of overlapped readdir changing cached file size and local cached write extending file 
parent 6f060189
...@@ -848,7 +848,7 @@ cifs_commit_write(struct file *file, struct page *page, unsigned offset, ...@@ -848,7 +848,7 @@ cifs_commit_write(struct file *file, struct page *page, unsigned offset,
cFYI(1,(" SetEOF (commit write) rc = %d",rc)); cFYI(1,(" SetEOF (commit write) rc = %d",rc));
}*/ }*/
} }
if (!PageUptodate(page)) { if (!PageUptodate(page)) {
position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset; position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset;
/* can not rely on (or let) writepage write this data */ /* can not rely on (or let) writepage write this data */
if(to < offset) { if(to < offset) {
...@@ -1284,6 +1284,49 @@ cifs_readpage(struct file *file, struct page *page) ...@@ -1284,6 +1284,49 @@ cifs_readpage(struct file *file, struct page *page)
return rc; return rc;
} }
/* We do not want to update the file size from server for inodes
open for write - to avoid races with writepage extending
the file - in the future we could consider allowing
refreshing the inode only on increases in the file size
but this is tricky to do without racing with writebehind
page caching in the current Linux kernel design */
int is_size_safe_to_change(struct cifsInodeInfo * cifsInode)
{
struct list_head *tmp;
struct list_head *tmp1;
struct cifsFileInfo *open_file = NULL;
int rc = TRUE;
if(cifsInode == NULL)
return rc;
read_lock(&GlobalSMBSeslock);
list_for_each_safe(tmp, tmp1, &cifsInode->openFileList) {
open_file = list_entry(tmp,struct cifsFileInfo, flist);
if(open_file == NULL)
break;
if(open_file->closePend)
continue;
/* We check if file is open for writing,
BB we could supplement this with a check to see if file size
changes have been flushed to server - ie inode metadata dirty */
if((open_file->pfile) &&
((open_file->pfile->f_flags & O_RDWR) ||
(open_file->pfile->f_flags & O_WRONLY))) {
rc = FALSE;
break;
}
if(tmp->next == NULL) {
cFYI(1,("File instance %p removed",tmp));
break;
}
}
read_unlock(&GlobalSMBSeslock);
return rc;
}
void void
fill_in_inode(struct inode *tmp_inode, fill_in_inode(struct inode *tmp_inode,
FILE_DIRECTORY_INFO * pfindData, int *pobject_type) FILE_DIRECTORY_INFO * pfindData, int *pobject_type)
...@@ -1343,11 +1386,15 @@ fill_in_inode(struct inode *tmp_inode, ...@@ -1343,11 +1386,15 @@ fill_in_inode(struct inode *tmp_inode,
atomic_set(&cifsInfo->inUse,1); atomic_set(&cifsInfo->inUse,1);
} }
i_size_write(tmp_inode,pfindData->EndOfFile); if(is_size_safe_to_change(cifsInfo)) {
/* can not safely change the file size here if the
client is writing to it due to potential races */
i_size_write(tmp_inode,pfindData->EndOfFile);
/* 512 bytes (2**9) is the fake blocksize that must be used */ /* 512 bytes (2**9) is the fake blocksize that must be used */
/* for this calculation, even though the reported blocksize is larger */ /* for this calculation, even though the reported blocksize is larger */
tmp_inode->i_blocks = (512 - 1 + pfindData->AllocationSize) >> 9; tmp_inode->i_blocks = (512 - 1 + pfindData->AllocationSize) >> 9;
}
if (pfindData->AllocationSize < pfindData->EndOfFile) if (pfindData->AllocationSize < pfindData->EndOfFile)
cFYI(1, ("Possible sparse file: allocation size less than end of file ")); cFYI(1, ("Possible sparse file: allocation size less than end of file "));
...@@ -1419,12 +1466,17 @@ unix_fill_in_inode(struct inode *tmp_inode, ...@@ -1419,12 +1466,17 @@ unix_fill_in_inode(struct inode *tmp_inode,
tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks); tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks);
pfindData->NumOfBytes = le64_to_cpu(pfindData->NumOfBytes); pfindData->NumOfBytes = le64_to_cpu(pfindData->NumOfBytes);
pfindData->EndOfFile = le64_to_cpu(pfindData->EndOfFile);
i_size_write(tmp_inode,pfindData->EndOfFile); if(is_size_safe_to_change(cifsInfo)) {
/* can not safely change the file size here if the
client is writing to it due to potential races */
pfindData->EndOfFile = le64_to_cpu(pfindData->EndOfFile);
i_size_write(tmp_inode,pfindData->EndOfFile);
/* 512 bytes (2**9) is the fake blocksize that must be used */ /* 512 bytes (2**9) is the fake blocksize that must be used */
/* for this calculation, not the real blocksize */ /* for this calculation, not the real blocksize */
tmp_inode->i_blocks = (512 - 1 + pfindData->NumOfBytes) >> 9; tmp_inode->i_blocks = (512 - 1 + pfindData->NumOfBytes) >> 9;
}
if (S_ISREG(tmp_inode->i_mode)) { if (S_ISREG(tmp_inode->i_mode)) {
cFYI(1, ("File inode")); cFYI(1, ("File inode"));
...@@ -1568,9 +1620,9 @@ cifs_filldir_unix(struct qstr *pqstring, ...@@ -1568,9 +1620,9 @@ cifs_filldir_unix(struct qstr *pqstring,
pqstring->len = strnlen(pUnixFindData->FileName, MAX_PATHCONF); pqstring->len = strnlen(pUnixFindData->FileName, MAX_PATHCONF);
construct_dentry(pqstring, file, &tmp_inode, &tmp_dentry); construct_dentry(pqstring, file, &tmp_inode, &tmp_dentry);
if((tmp_inode == NULL) || (tmp_dentry == NULL)) { if((tmp_inode == NULL) || (tmp_dentry == NULL)) {
return -ENOMEM; return -ENOMEM;
} }
unix_fill_in_inode(tmp_inode, pUnixFindData, &object_type); unix_fill_in_inode(tmp_inode, pUnixFindData, &object_type);
rc = filldir(direntry, pUnixFindData->FileName, pqstring->len, rc = filldir(direntry, pUnixFindData->FileName, pqstring->len,
......
...@@ -30,6 +30,8 @@ ...@@ -30,6 +30,8 @@
#include "cifs_debug.h" #include "cifs_debug.h"
#include "cifs_fs_sb.h" #include "cifs_fs_sb.h"
extern int is_size_safe_to_change(struct cifsInodeInfo *);
int int
cifs_get_inode_info_unix(struct inode **pinode, cifs_get_inode_info_unix(struct inode **pinode,
const unsigned char *search_path, const unsigned char *search_path,
...@@ -43,9 +45,6 @@ cifs_get_inode_info_unix(struct inode **pinode, ...@@ -43,9 +45,6 @@ cifs_get_inode_info_unix(struct inode **pinode,
struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
char *tmp_path; char *tmp_path;
/* BB add caching check so we do not go to server to overwrite inode info to cached file
where the local file sizes are correct and the server info is stale BB */
xid = GetXid(); xid = GetXid();
pTcon = cifs_sb->tcon; pTcon = cifs_sb->tcon;
...@@ -125,7 +124,12 @@ cifs_get_inode_info_unix(struct inode **pinode, ...@@ -125,7 +124,12 @@ cifs_get_inode_info_unix(struct inode **pinode,
inode->i_nlink = le64_to_cpu(findData.Nlinks); inode->i_nlink = le64_to_cpu(findData.Nlinks);
findData.NumOfBytes = le64_to_cpu(findData.NumOfBytes); findData.NumOfBytes = le64_to_cpu(findData.NumOfBytes);
findData.EndOfFile = le64_to_cpu(findData.EndOfFile); findData.EndOfFile = le64_to_cpu(findData.EndOfFile);
i_size_write(inode,findData.EndOfFile);
if(is_size_safe_to_change(cifsInfo)) {
/* can not safely change the file size here if the
client is writing to it due to potential races */
i_size_write(inode,findData.EndOfFile);
/* blksize needs to be multiple of two. So safer to default to blksize /* blksize needs to be multiple of two. So safer to default to blksize
and blkbits set in superblock so 2**blkbits and blksize will match */ and blkbits set in superblock so 2**blkbits and blksize will match */
/* inode->i_blksize = /* inode->i_blksize =
...@@ -141,7 +145,8 @@ cifs_get_inode_info_unix(struct inode **pinode, ...@@ -141,7 +145,8 @@ cifs_get_inode_info_unix(struct inode **pinode,
/* 512 bytes (2**9) is the fake blocksize that must be used */ /* 512 bytes (2**9) is the fake blocksize that must be used */
/* for this calculation */ /* for this calculation */
inode->i_blocks = (512 - 1 + findData.NumOfBytes) >> 9; inode->i_blocks = (512 - 1 + findData.NumOfBytes) >> 9;
}
if (findData.NumOfBytes < findData.EndOfFile) if (findData.NumOfBytes < findData.EndOfFile)
cFYI(1, ("Server inconsistency Error: it says allocation size less than end of file ")); cFYI(1, ("Server inconsistency Error: it says allocation size less than end of file "));
...@@ -283,12 +288,18 @@ cifs_get_inode_info(struct inode **pinode, const unsigned char *search_path, ...@@ -283,12 +288,18 @@ cifs_get_inode_info(struct inode **pinode, const unsigned char *search_path,
inode->i_mode &= ~(S_IWUGO); inode->i_mode &= ~(S_IWUGO);
/* BB add code here - validate if device or weird share or device type? */ /* BB add code here - validate if device or weird share or device type? */
} }
i_size_write(inode,le64_to_cpu(pfindData->EndOfFile)); if(is_size_safe_to_change(cifsInfo)) {
pfindData->AllocationSize = le64_to_cpu(pfindData->AllocationSize); /* can not safely change the file size here if the
client is writing to it due to potential races */
i_size_write(inode,le64_to_cpu(pfindData->EndOfFile));
/* 512 bytes (2**9) is the fake blocksize that must be used */ /* 512 bytes (2**9) is the fake blocksize that must be used */
/* for this calculation */ /* for this calculation */
inode->i_blocks = (512 - 1 + pfindData->AllocationSize) >> 9; inode->i_blocks = (512 - 1 + pfindData->AllocationSize)
>> 9;
}
pfindData->AllocationSize = le64_to_cpu(pfindData->AllocationSize);
inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks); inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment