diff options
author | Steve French <sfrench@us.ibm.com> | 2007-02-26 11:46:11 -0500 |
---|---|---|
committer | Steve French <sfrench@us.ibm.com> | 2007-02-26 11:46:11 -0500 |
commit | 3677db10a635a39f63ea509f8f0056d95589ff90 (patch) | |
tree | 5256a408110c91947d9b9543199003fb976948a8 | |
parent | 9654640d0af8f2de40ff3807d3695109d3463f54 (diff) |
[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>
-rw-r--r-- | fs/cifs/CHANGES | 6 | ||||
-rw-r--r-- | fs/cifs/file.c | 43 | ||||
-rw-r--r-- | fs/cifs/inode.c | 50 | ||||
-rw-r--r-- | fs/cifs/readdir.c | 6 |
4 files changed, 82 insertions, 23 deletions
diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index 5fe13593b57f..e08a147c09e1 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES | |||
@@ -1,3 +1,9 @@ | |||
1 | Verison 1.48 | ||
2 | ------------ | ||
3 | Fix mtime bouncing around from local idea of last write times to remote time. | ||
4 | Fix hang (in i_size_read) when simultaneous size update of same remote file | ||
5 | on smp system corrupts sequence number. | ||
6 | |||
1 | Version 1.47 | 7 | Version 1.47 |
2 | ------------ | 8 | ------------ |
3 | Fix oops in list_del during mount caused by unaligned string. | 9 | Fix oops in list_del during mount caused by unaligned string. |
diff --git a/fs/cifs/file.c b/fs/cifs/file.c index a1265c9bfec0..c07ff8317a8b 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c | |||
@@ -879,18 +879,19 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data, | |||
879 | cifs_stats_bytes_written(pTcon, total_written); | 879 | cifs_stats_bytes_written(pTcon, total_written); |
880 | 880 | ||
881 | /* since the write may have blocked check these pointers again */ | 881 | /* since the write may have blocked check these pointers again */ |
882 | if (file->f_path.dentry) { | 882 | if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) { |
883 | if (file->f_path.dentry->d_inode) { | 883 | struct inode *inode = file->f_path.dentry->d_inode; |
884 | struct inode *inode = file->f_path.dentry->d_inode; | 884 | /* Do not update local mtime - server will set its actual value on write |
885 | inode->i_ctime = inode->i_mtime = | 885 | * inode->i_ctime = inode->i_mtime = |
886 | current_fs_time(inode->i_sb); | 886 | * current_fs_time(inode->i_sb);*/ |
887 | if (total_written > 0) { | 887 | if (total_written > 0) { |
888 | if (*poffset > file->f_path.dentry->d_inode->i_size) | 888 | spin_lock(&inode->i_lock); |
889 | i_size_write(file->f_path.dentry->d_inode, | 889 | if (*poffset > file->f_path.dentry->d_inode->i_size) |
890 | i_size_write(file->f_path.dentry->d_inode, | ||
890 | *poffset); | 891 | *poffset); |
891 | } | 892 | spin_unlock(&inode->i_lock); |
892 | mark_inode_dirty_sync(file->f_path.dentry->d_inode); | ||
893 | } | 893 | } |
894 | mark_inode_dirty_sync(file->f_path.dentry->d_inode); | ||
894 | } | 895 | } |
895 | FreeXid(xid); | 896 | FreeXid(xid); |
896 | return total_written; | 897 | return total_written; |
@@ -1012,18 +1013,18 @@ static ssize_t cifs_write(struct file *file, const char *write_data, | |||
1012 | cifs_stats_bytes_written(pTcon, total_written); | 1013 | cifs_stats_bytes_written(pTcon, total_written); |
1013 | 1014 | ||
1014 | /* since the write may have blocked check these pointers again */ | 1015 | /* since the write may have blocked check these pointers again */ |
1015 | if (file->f_path.dentry) { | 1016 | if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) { |
1016 | if (file->f_path.dentry->d_inode) { | ||
1017 | /*BB We could make this contingent on superblock ATIME flag too */ | 1017 | /*BB We could make this contingent on superblock ATIME flag too */ |
1018 | /* file->f_path.dentry->d_inode->i_ctime = | 1018 | /* file->f_path.dentry->d_inode->i_ctime = |
1019 | file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/ | 1019 | file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/ |
1020 | if (total_written > 0) { | 1020 | if (total_written > 0) { |
1021 | if (*poffset > file->f_path.dentry->d_inode->i_size) | 1021 | spin_lock(&file->f_path.dentry->d_inode->i_lock); |
1022 | i_size_write(file->f_path.dentry->d_inode, | 1022 | if (*poffset > file->f_path.dentry->d_inode->i_size) |
1023 | *poffset); | 1023 | i_size_write(file->f_path.dentry->d_inode, |
1024 | } | 1024 | *poffset); |
1025 | mark_inode_dirty_sync(file->f_path.dentry->d_inode); | 1025 | spin_unlock(&file->f_path.dentry->d_inode->i_lock); |
1026 | } | 1026 | } |
1027 | mark_inode_dirty_sync(file->f_path.dentry->d_inode); | ||
1027 | } | 1028 | } |
1028 | FreeXid(xid); | 1029 | FreeXid(xid); |
1029 | return total_written; | 1030 | return total_written; |
@@ -1400,6 +1401,7 @@ static int cifs_commit_write(struct file *file, struct page *page, | |||
1400 | xid = GetXid(); | 1401 | xid = GetXid(); |
1401 | cFYI(1, ("commit write for page %p up to position %lld for %d", | 1402 | cFYI(1, ("commit write for page %p up to position %lld for %d", |
1402 | page, position, to)); | 1403 | page, position, to)); |
1404 | spin_lock(&inode->i_lock); | ||
1403 | if (position > inode->i_size) { | 1405 | if (position > inode->i_size) { |
1404 | i_size_write(inode, position); | 1406 | i_size_write(inode, position); |
1405 | /* if (file->private_data == NULL) { | 1407 | /* if (file->private_data == NULL) { |
@@ -1429,6 +1431,7 @@ static int cifs_commit_write(struct file *file, struct page *page, | |||
1429 | cFYI(1, (" SetEOF (commit write) rc = %d", rc)); | 1431 | cFYI(1, (" SetEOF (commit write) rc = %d", rc)); |
1430 | } */ | 1432 | } */ |
1431 | } | 1433 | } |
1434 | spin_unlock(&inode->i_lock); | ||
1432 | if (!PageUptodate(page)) { | 1435 | if (!PageUptodate(page)) { |
1433 | position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset; | 1436 | position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset; |
1434 | /* can not rely on (or let) writepage write this data */ | 1437 | /* can not rely on (or let) writepage write this data */ |
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 37c6ce87416b..24df13a256e5 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c | |||
@@ -143,10 +143,10 @@ int cifs_get_inode_info_unix(struct inode **pinode, | |||
143 | inode->i_gid = le64_to_cpu(findData.Gid); | 143 | inode->i_gid = le64_to_cpu(findData.Gid); |
144 | inode->i_nlink = le64_to_cpu(findData.Nlinks); | 144 | inode->i_nlink = le64_to_cpu(findData.Nlinks); |
145 | 145 | ||
146 | spin_lock(&inode->i_lock); | ||
146 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { | 147 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { |
147 | /* can not safely change the file size here if the | 148 | /* can not safely change the file size here if the |
148 | client is writing to it due to potential races */ | 149 | client is writing to it due to potential races */ |
149 | |||
150 | i_size_write(inode, end_of_file); | 150 | i_size_write(inode, end_of_file); |
151 | 151 | ||
152 | /* blksize needs to be multiple of two. So safer to default to | 152 | /* 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, | |||
162 | /* for this calculation */ | 162 | /* for this calculation */ |
163 | inode->i_blocks = (512 - 1 + num_of_bytes) >> 9; | 163 | inode->i_blocks = (512 - 1 + num_of_bytes) >> 9; |
164 | } | 164 | } |
165 | spin_unlock(&inode->i_lock); | ||
165 | 166 | ||
166 | if (num_of_bytes < end_of_file) | 167 | if (num_of_bytes < end_of_file) |
167 | cFYI(1, ("allocation size less than end of file")); | 168 | cFYI(1, ("allocation size less than end of file")); |
@@ -496,6 +497,8 @@ int cifs_get_inode_info(struct inode **pinode, | |||
496 | /* BB add code here - | 497 | /* BB add code here - |
497 | validate if device or weird share or device type? */ | 498 | validate if device or weird share or device type? */ |
498 | } | 499 | } |
500 | |||
501 | spin_lock(&inode->i_lock); | ||
499 | if (is_size_safe_to_change(cifsInfo, le64_to_cpu(pfindData->EndOfFile))) { | 502 | if (is_size_safe_to_change(cifsInfo, le64_to_cpu(pfindData->EndOfFile))) { |
500 | /* can not safely shrink the file size here if the | 503 | /* can not safely shrink the file size here if the |
501 | client is writing to it due to potential races */ | 504 | client is writing to it due to potential races */ |
@@ -506,6 +509,7 @@ int cifs_get_inode_info(struct inode **pinode, | |||
506 | inode->i_blocks = (512 - 1 + le64_to_cpu( | 509 | inode->i_blocks = (512 - 1 + le64_to_cpu( |
507 | pfindData->AllocationSize)) >> 9; | 510 | pfindData->AllocationSize)) >> 9; |
508 | } | 511 | } |
512 | spin_unlock(&inode->i_lock); | ||
509 | 513 | ||
510 | inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks); | 514 | inode->i_nlink = le32_to_cpu(pfindData->NumberOfLinks); |
511 | 515 | ||
@@ -834,8 +838,10 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) | |||
834 | 838 | ||
835 | if (!rc) { | 839 | if (!rc) { |
836 | drop_nlink(inode); | 840 | drop_nlink(inode); |
841 | spin_lock(&direntry->d_inode->i_lock); | ||
837 | i_size_write(direntry->d_inode,0); | 842 | i_size_write(direntry->d_inode,0); |
838 | clear_nlink(direntry->d_inode); | 843 | clear_nlink(direntry->d_inode); |
844 | spin_unlock(&direntry->d_inode->i_lock); | ||
839 | } | 845 | } |
840 | 846 | ||
841 | cifsInode = CIFS_I(direntry->d_inode); | 847 | cifsInode = CIFS_I(direntry->d_inode); |
@@ -1128,6 +1134,46 @@ static int cifs_truncate_page(struct address_space *mapping, loff_t from) | |||
1128 | return rc; | 1134 | return rc; |
1129 | } | 1135 | } |
1130 | 1136 | ||
1137 | int cifs_vmtruncate(struct inode * inode, loff_t offset) | ||
1138 | { | ||
1139 | struct address_space *mapping = inode->i_mapping; | ||
1140 | unsigned long limit; | ||
1141 | |||
1142 | if (inode->i_size < offset) | ||
1143 | goto do_expand; | ||
1144 | /* | ||
1145 | * truncation of in-use swapfiles is disallowed - it would cause | ||
1146 | * subsequent swapout to scribble on the now-freed blocks. | ||
1147 | */ | ||
1148 | if (IS_SWAPFILE(inode)) | ||
1149 | goto out_busy; | ||
1150 | spin_lock(&inode->i_lock); | ||
1151 | i_size_write(inode, offset); | ||
1152 | spin_unlock(&inode->i_lock); | ||
1153 | unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1); | ||
1154 | truncate_inode_pages(mapping, offset); | ||
1155 | goto out_truncate; | ||
1156 | |||
1157 | do_expand: | ||
1158 | limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur; | ||
1159 | if (limit != RLIM_INFINITY && offset > limit) | ||
1160 | goto out_sig; | ||
1161 | if (offset > inode->i_sb->s_maxbytes) | ||
1162 | goto out_big; | ||
1163 | i_size_write(inode, offset); | ||
1164 | |||
1165 | out_truncate: | ||
1166 | if (inode->i_op && inode->i_op->truncate) | ||
1167 | inode->i_op->truncate(inode); | ||
1168 | return 0; | ||
1169 | out_sig: | ||
1170 | send_sig(SIGXFSZ, current, 0); | ||
1171 | out_big: | ||
1172 | return -EFBIG; | ||
1173 | out_busy: | ||
1174 | return -ETXTBSY; | ||
1175 | } | ||
1176 | |||
1131 | int cifs_setattr(struct dentry *direntry, struct iattr *attrs) | 1177 | int cifs_setattr(struct dentry *direntry, struct iattr *attrs) |
1132 | { | 1178 | { |
1133 | int xid; | 1179 | int xid; |
@@ -1244,7 +1290,7 @@ int cifs_setattr(struct dentry *direntry, struct iattr *attrs) | |||
1244 | */ | 1290 | */ |
1245 | 1291 | ||
1246 | if (rc == 0) { | 1292 | if (rc == 0) { |
1247 | rc = vmtruncate(direntry->d_inode, attrs->ia_size); | 1293 | rc = cifs_vmtruncate(direntry->d_inode, attrs->ia_size); |
1248 | cifs_truncate_page(direntry->d_inode->i_mapping, | 1294 | cifs_truncate_page(direntry->d_inode->i_mapping, |
1249 | direntry->d_inode->i_size); | 1295 | direntry->d_inode->i_size); |
1250 | } else | 1296 | } else |
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index c444798f0740..44cfb528797d 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c | |||
@@ -3,7 +3,7 @@ | |||
3 | * | 3 | * |
4 | * Directory search handling | 4 | * Directory search handling |
5 | * | 5 | * |
6 | * Copyright (C) International Business Machines Corp., 2004, 2005 | 6 | * Copyright (C) International Business Machines Corp., 2004, 2007 |
7 | * Author(s): Steve French (sfrench@us.ibm.com) | 7 | * Author(s): Steve French (sfrench@us.ibm.com) |
8 | * | 8 | * |
9 | * This library is free software; you can redistribute it and/or modify | 9 | * 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, | |||
226 | atomic_set(&cifsInfo->inUse, 1); | 226 | atomic_set(&cifsInfo->inUse, 1); |
227 | } | 227 | } |
228 | 228 | ||
229 | spin_lock(&tmp_inode->i_lock); | ||
229 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { | 230 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { |
230 | /* can not safely change the file size here if the | 231 | /* can not safely change the file size here if the |
231 | client is writing to it due to potential races */ | 232 | 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, | |||
235 | /* for this calculation, even though the reported blocksize is larger */ | 236 | /* for this calculation, even though the reported blocksize is larger */ |
236 | tmp_inode->i_blocks = (512 - 1 + allocation_size) >> 9; | 237 | tmp_inode->i_blocks = (512 - 1 + allocation_size) >> 9; |
237 | } | 238 | } |
239 | spin_unlock(&tmp_inode->i_lock); | ||
238 | 240 | ||
239 | if (allocation_size < end_of_file) | 241 | if (allocation_size < end_of_file) |
240 | cFYI(1, ("May be sparse file, allocation less than file size")); | 242 | 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, | |||
355 | tmp_inode->i_gid = le64_to_cpu(pfindData->Gid); | 357 | tmp_inode->i_gid = le64_to_cpu(pfindData->Gid); |
356 | tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks); | 358 | tmp_inode->i_nlink = le64_to_cpu(pfindData->Nlinks); |
357 | 359 | ||
360 | spin_lock(&tmp_inode->i_lock); | ||
358 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { | 361 | if (is_size_safe_to_change(cifsInfo, end_of_file)) { |
359 | /* can not safely change the file size here if the | 362 | /* can not safely change the file size here if the |
360 | client is writing to it due to potential races */ | 363 | client is writing to it due to potential races */ |
@@ -364,6 +367,7 @@ static void unix_fill_in_inode(struct inode *tmp_inode, | |||
364 | /* for this calculation, not the real blocksize */ | 367 | /* for this calculation, not the real blocksize */ |
365 | tmp_inode->i_blocks = (512 - 1 + num_of_bytes) >> 9; | 368 | tmp_inode->i_blocks = (512 - 1 + num_of_bytes) >> 9; |
366 | } | 369 | } |
370 | spin_unlock(&tmp_inode->i_lock); | ||
367 | 371 | ||
368 | if (S_ISREG(tmp_inode->i_mode)) { | 372 | if (S_ISREG(tmp_inode->i_mode)) { |
369 | cFYI(1, ("File inode")); | 373 | cFYI(1, ("File inode")); |