[CIFS] Fix locking problem around some cifs uses of i_size write
Could cause hangs on smp systems in i_size_read on a cifs inode whose size has been previously simultaneously updated from different processes. Thanks to Brian Wang for some great testing/debugging on this hard problem. Fixes kernel bugzilla #7903 CC: Shirish Pargoankar <shirishp@us.ibm.com> CC: Shaggy <shaggy@us.ibm.com> Signed-off-by: Steve French <sfrench@us.ibm.com>
This commit is contained in:
@@ -1,3 +1,9 @@
|
|||||||
|
Verison 1.48
|
||||||
|
------------
|
||||||
|
Fix mtime bouncing around from local idea of last write times to remote time.
|
||||||
|
Fix hang (in i_size_read) when simultaneous size update of same remote file
|
||||||
|
on smp system corrupts sequence number.
|
||||||
|
|
||||||
Version 1.47
|
Version 1.47
|
||||||
------------
|
------------
|
||||||
Fix oops in list_del during mount caused by unaligned string.
|
Fix oops in list_del during mount caused by unaligned string.
|
||||||
|
@@ -879,18 +879,19 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
|
|||||||
cifs_stats_bytes_written(pTcon, total_written);
|
cifs_stats_bytes_written(pTcon, total_written);
|
||||||
|
|
||||||
/* since the write may have blocked check these pointers again */
|
/* since the write may have blocked check these pointers again */
|
||||||
if (file->f_path.dentry) {
|
if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
|
||||||
if (file->f_path.dentry->d_inode) {
|
struct inode *inode = file->f_path.dentry->d_inode;
|
||||||
struct inode *inode = file->f_path.dentry->d_inode;
|
/* Do not update local mtime - server will set its actual value on write
|
||||||
inode->i_ctime = inode->i_mtime =
|
* inode->i_ctime = inode->i_mtime =
|
||||||
current_fs_time(inode->i_sb);
|
* current_fs_time(inode->i_sb);*/
|
||||||
if (total_written > 0) {
|
if (total_written > 0) {
|
||||||
if (*poffset > file->f_path.dentry->d_inode->i_size)
|
spin_lock(&inode->i_lock);
|
||||||
i_size_write(file->f_path.dentry->d_inode,
|
if (*poffset > file->f_path.dentry->d_inode->i_size)
|
||||||
|
i_size_write(file->f_path.dentry->d_inode,
|
||||||
*poffset);
|
*poffset);
|
||||||
}
|
spin_unlock(&inode->i_lock);
|
||||||
mark_inode_dirty_sync(file->f_path.dentry->d_inode);
|
|
||||||
}
|
}
|
||||||
|
mark_inode_dirty_sync(file->f_path.dentry->d_inode);
|
||||||
}
|
}
|
||||||
FreeXid(xid);
|
FreeXid(xid);
|
||||||
return total_written;
|
return total_written;
|
||||||
@@ -1012,18 +1013,18 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
|
|||||||
cifs_stats_bytes_written(pTcon, total_written);
|
cifs_stats_bytes_written(pTcon, total_written);
|
||||||
|
|
||||||
/* since the write may have blocked check these pointers again */
|
/* since the write may have blocked check these pointers again */
|
||||||
if (file->f_path.dentry) {
|
if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
|
||||||
if (file->f_path.dentry->d_inode) {
|
|
||||||
/*BB We could make this contingent on superblock ATIME flag too */
|
/*BB We could make this contingent on superblock ATIME flag too */
|
||||||
/* file->f_path.dentry->d_inode->i_ctime =
|
/* file->f_path.dentry->d_inode->i_ctime =
|
||||||
file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/
|
file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/
|
||||||
if (total_written > 0) {
|
if (total_written > 0) {
|
||||||
if (*poffset > file->f_path.dentry->d_inode->i_size)
|
spin_lock(&file->f_path.dentry->d_inode->i_lock);
|
||||||
i_size_write(file->f_path.dentry->d_inode,
|
if (*poffset > file->f_path.dentry->d_inode->i_size)
|
||||||
*poffset);
|
i_size_write(file->f_path.dentry->d_inode,
|
||||||
}
|
*poffset);
|
||||||
mark_inode_dirty_sync(file->f_path.dentry->d_inode);
|
spin_unlock(&file->f_path.dentry->d_inode->i_lock);
|
||||||
}
|
}
|
||||||
|
mark_inode_dirty_sync(file->f_path.dentry->d_inode);
|
||||||
}
|
}
|
||||||
FreeXid(xid);
|
FreeXid(xid);
|
||||||
return total_written;
|
return total_written;
|
||||||
@@ -1400,6 +1401,7 @@ static int cifs_commit_write(struct file *file, struct page *page,
|
|||||||
xid = GetXid();
|
xid = GetXid();
|
||||||
cFYI(1, ("commit write for page %p up to position %lld for %d",
|
cFYI(1, ("commit write for page %p up to position %lld for %d",
|
||||||
page, position, to));
|
page, position, to));
|
||||||
|
spin_lock(&inode->i_lock);
|
||||||
if (position > inode->i_size) {
|
if (position > inode->i_size) {
|
||||||
i_size_write(inode, position);
|
i_size_write(inode, position);
|
||||||
/* if (file->private_data == NULL) {
|
/* if (file->private_data == NULL) {
|
||||||
@@ -1429,6 +1431,7 @@ static int cifs_commit_write(struct file *file, struct page *page,
|
|||||||
cFYI(1, (" SetEOF (commit write) rc = %d", rc));
|
cFYI(1, (" SetEOF (commit write) rc = %d", rc));
|
||||||
} */
|
} */
|
||||||
}
|
}
|
||||||
|
spin_unlock(&inode->i_lock);
|
||||||
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 */
|
||||||
|
@@ -143,10 +143,10 @@ int cifs_get_inode_info_unix(struct inode **pinode,
|
|||||||
inode->i_gid = le64_to_cpu(findData.Gid);
|
inode->i_gid = le64_to_cpu(findData.Gid);
|
||||||
inode->i_nlink = le64_to_cpu(findData.Nlinks);
|
inode->i_nlink = le64_to_cpu(findData.Nlinks);
|
||||||
|
|
||||||
|
spin_lock(&inode->i_lock);
|
||||||
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
||||||
/* can not safely change the file size here if the
|
/* can not safely change the file size here if the
|
||||||
client is writing to it due to potential races */
|
client is writing to it due to potential races */
|
||||||
|
|
||||||
i_size_write(inode, end_of_file);
|
i_size_write(inode, end_of_file);
|
||||||
|
|
||||||
/* blksize needs to be multiple of two. So safer to default to
|
/* blksize needs to be multiple of two. So safer to default to
|
||||||
@@ -162,6 +162,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
|
|||||||
/* for this calculation */
|
/* for this calculation */
|
||||||
inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
|
inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
|
||||||
}
|
}
|
||||||
|
spin_unlock(&inode->i_lock);
|
||||||
|
|
||||||
if (num_of_bytes < end_of_file)
|
if (num_of_bytes < end_of_file)
|
||||||
cFYI(1, ("allocation size less than end of file"));
|
cFYI(1, ("allocation size less than end of file"));
|
||||||
@@ -496,6 +497,8 @@ int cifs_get_inode_info(struct inode **pinode,
|
|||||||
/* BB add code here -
|
/* BB add code here -
|
||||||
validate if device or weird share or device type? */
|
validate if device or weird share or device type? */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
spin_lock(&inode->i_lock);
|
||||||
if (is_size_safe_to_change(cifsInfo, le64_to_cpu(pfindData->EndOfFile))) {
|
if (is_size_safe_to_change(cifsInfo, le64_to_cpu(pfindData->EndOfFile))) {
|
||||||
/* can not safely shrink the file size here if the
|
/* can not safely shrink the file size here if the
|
||||||
client is writing to it due to potential races */
|
client is writing to it due to potential races */
|
||||||
@@ -506,6 +509,7 @@ int cifs_get_inode_info(struct inode **pinode,
|
|||||||
inode->i_blocks = (512 - 1 + le64_to_cpu(
|
inode->i_blocks = (512 - 1 + le64_to_cpu(
|
||||||
pfindData->AllocationSize)) >> 9;
|
pfindData->AllocationSize)) >> 9;
|
||||||
}
|
}
|
||||||
|
spin_unlock(&inode->i_lock);
|
||||||
|
|
||||||
inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks);
|
inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks);
|
||||||
|
|
||||||
@@ -834,8 +838,10 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
|
|||||||
|
|
||||||
if (!rc) {
|
if (!rc) {
|
||||||
drop_nlink(inode);
|
drop_nlink(inode);
|
||||||
|
spin_lock(&direntry->d_inode->i_lock);
|
||||||
i_size_write(direntry->d_inode,0);
|
i_size_write(direntry->d_inode,0);
|
||||||
clear_nlink(direntry->d_inode);
|
clear_nlink(direntry->d_inode);
|
||||||
|
spin_unlock(&direntry->d_inode->i_lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
cifsInode = CIFS_I(direntry->d_inode);
|
cifsInode = CIFS_I(direntry->d_inode);
|
||||||
@@ -1128,6 +1134,46 @@ static int cifs_truncate_page(struct address_space *mapping, loff_t from)
|
|||||||
return rc;
|
return rc;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
int cifs_vmtruncate(struct inode * inode, loff_t offset)
|
||||||
|
{
|
||||||
|
struct address_space *mapping = inode->i_mapping;
|
||||||
|
unsigned long limit;
|
||||||
|
|
||||||
|
if (inode->i_size < offset)
|
||||||
|
goto do_expand;
|
||||||
|
/*
|
||||||
|
* truncation of in-use swapfiles is disallowed - it would cause
|
||||||
|
* subsequent swapout to scribble on the now-freed blocks.
|
||||||
|
*/
|
||||||
|
if (IS_SWAPFILE(inode))
|
||||||
|
goto out_busy;
|
||||||
|
spin_lock(&inode->i_lock);
|
||||||
|
i_size_write(inode, offset);
|
||||||
|
spin_unlock(&inode->i_lock);
|
||||||
|
unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
|
||||||
|
truncate_inode_pages(mapping, offset);
|
||||||
|
goto out_truncate;
|
||||||
|
|
||||||
|
do_expand:
|
||||||
|
limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
|
||||||
|
if (limit != RLIM_INFINITY && offset > limit)
|
||||||
|
goto out_sig;
|
||||||
|
if (offset > inode->i_sb->s_maxbytes)
|
||||||
|
goto out_big;
|
||||||
|
i_size_write(inode, offset);
|
||||||
|
|
||||||
|
out_truncate:
|
||||||
|
if (inode->i_op && inode->i_op->truncate)
|
||||||
|
inode->i_op->truncate(inode);
|
||||||
|
return 0;
|
||||||
|
out_sig:
|
||||||
|
send_sig(SIGXFSZ, current, 0);
|
||||||
|
out_big:
|
||||||
|
return -EFBIG;
|
||||||
|
out_busy:
|
||||||
|
return -ETXTBSY;
|
||||||
|
}
|
||||||
|
|
||||||
int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
|
int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
|
||||||
{
|
{
|
||||||
int xid;
|
int xid;
|
||||||
@@ -1244,7 +1290,7 @@ int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
if (rc == 0) {
|
if (rc == 0) {
|
||||||
rc = vmtruncate(direntry->d_inode, attrs->ia_size);
|
rc = cifs_vmtruncate(direntry->d_inode, attrs->ia_size);
|
||||||
cifs_truncate_page(direntry->d_inode->i_mapping,
|
cifs_truncate_page(direntry->d_inode->i_mapping,
|
||||||
direntry->d_inode->i_size);
|
direntry->d_inode->i_size);
|
||||||
} else
|
} else
|
||||||
|
@@ -3,7 +3,7 @@
|
|||||||
*
|
*
|
||||||
* Directory search handling
|
* Directory search handling
|
||||||
*
|
*
|
||||||
* Copyright (C) International Business Machines Corp., 2004, 2005
|
* Copyright (C) International Business Machines Corp., 2004, 2007
|
||||||
* Author(s): Steve French (sfrench@us.ibm.com)
|
* Author(s): Steve French (sfrench@us.ibm.com)
|
||||||
*
|
*
|
||||||
* This library is free software; you can redistribute it and/or modify
|
* This library is free software; you can redistribute it and/or modify
|
||||||
@@ -226,6 +226,7 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type,
|
|||||||
atomic_set(&cifsInfo->inUse, 1);
|
atomic_set(&cifsInfo->inUse, 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
spin_lock(&tmp_inode->i_lock);
|
||||||
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
||||||
/* can not safely change the file size here if the
|
/* can not safely change the file size here if the
|
||||||
client is writing to it due to potential races */
|
client is writing to it due to potential races */
|
||||||
@@ -235,6 +236,7 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type,
|
|||||||
/* 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 + allocation_size) >> 9;
|
tmp_inode->i_blocks = (512 - 1 + allocation_size) >> 9;
|
||||||
}
|
}
|
||||||
|
spin_unlock(&tmp_inode->i_lock);
|
||||||
|
|
||||||
if (allocation_size < end_of_file)
|
if (allocation_size < end_of_file)
|
||||||
cFYI(1, ("May be sparse file, allocation less than file size"));
|
cFYI(1, ("May be sparse file, allocation less than file size"));
|
||||||
@@ -355,6 +357,7 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
|
|||||||
tmp_inode->i_gid = le64_to_cpu(pfindData->Gid);
|
tmp_inode->i_gid = le64_to_cpu(pfindData->Gid);
|
||||||
tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks);
|
tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks);
|
||||||
|
|
||||||
|
spin_lock(&tmp_inode->i_lock);
|
||||||
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
if (is_size_safe_to_change(cifsInfo, end_of_file)) {
|
||||||
/* can not safely change the file size here if the
|
/* can not safely change the file size here if the
|
||||||
client is writing to it due to potential races */
|
client is writing to it due to potential races */
|
||||||
@@ -364,6 +367,7 @@ static void unix_fill_in_inode(struct inode *tmp_inode,
|
|||||||
/* for this calculation, not the real blocksize */
|
/* for this calculation, not the real blocksize */
|
||||||
tmp_inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
|
tmp_inode->i_blocks = (512 - 1 + num_of_bytes) >> 9;
|
||||||
}
|
}
|
||||||
|
spin_unlock(&tmp_inode->i_lock);
|
||||||
|
|
||||||
if (S_ISREG(tmp_inode->i_mode)) {
|
if (S_ISREG(tmp_inode->i_mode)) {
|
||||||
cFYI(1, ("File inode"));
|
cFYI(1, ("File inode"));
|
||||||
|
Reference in New Issue
Block a user