aboutsummaryrefslogtreecommitdiffstats
path: root/fs/cifs/file.c
diff options
context:
space:
mode:
authorSteve French <sfrench@us.ibm.com>2007-10-01 21:11:08 -0400
committerSteve French <sfrench@us.ibm.com>2007-10-01 21:11:08 -0400
commit9b22b0b726c6e46048767728a0900c8c05f93c21 (patch)
tree12e78fec74b790850f2c0d61bccfb1bb10ea8a0b /fs/cifs/file.c
parent4084973dbae9a24e58598d6cdf60f0e5e4a3cabf (diff)
[CIFS] Reduce chance of list corruption in find_writable_file
When find_writable_file is racing with close and the session to the server goes down, Shaggy noticed that there was a chance that an open file in the list of files off the inode could have been freed by close since cifs_reconnect can block (the spinlock thus not held). This means that we have to start over at the beginning of the list in some cases. There is a 2nd change that needs to be made later (pointed out by Jeremy Allison and Shaggy) in order to prevent cifs_close ever freeing the cifs per file info when a write is pending. Although we delay close from freeing this memory for sufficiently long for all known cases, ultimately on a very, very slow write overlapping a close pending we need to allow close to return (without freeing the cifs file info) and defer freeing the memory to be the responsibility of the (sloooow) write thread (presumably have to look at every place wrtPending is decremented - and add a flag for deferred free for after wrtPending goes to zero). Acked-by: Shaggy <shaggy@us.ibm.com> Acked-by: Shirish Pargaonkar <shirishp@us.ibm.com> Signed-off-by: Steve French <sfrench@us.ibm.com>
Diffstat (limited to 'fs/cifs/file.c')
-rw-r--r--fs/cifs/file.c54
1 files changed, 39 insertions, 15 deletions
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 79254919386..780c0e3e470 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1042,6 +1042,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode)
1042 } 1042 }
1043 1043
1044 read_lock(&GlobalSMBSeslock); 1044 read_lock(&GlobalSMBSeslock);
1045refind_writable:
1045 list_for_each_entry(open_file, &cifs_inode->openFileList, flist) { 1046 list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
1046 if (open_file->closePend) 1047 if (open_file->closePend)
1047 continue; 1048 continue;
@@ -1049,26 +1050,49 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode)
1049 ((open_file->pfile->f_flags & O_RDWR) || 1050 ((open_file->pfile->f_flags & O_RDWR) ||
1050 (open_file->pfile->f_flags & O_WRONLY))) { 1051 (open_file->pfile->f_flags & O_WRONLY))) {
1051 atomic_inc(&open_file->wrtPending); 1052 atomic_inc(&open_file->wrtPending);
1053
1054 if (!open_file->invalidHandle) {
1055 /* found a good writable file */
1056 read_unlock(&GlobalSMBSeslock);
1057 return open_file;
1058 }
1059
1052 read_unlock(&GlobalSMBSeslock); 1060 read_unlock(&GlobalSMBSeslock);
1053 if (open_file->invalidHandle) { 1061 /* Had to unlock since following call can block */
1054 rc = cifs_reopen_file(open_file->pfile, FALSE); 1062 rc = cifs_reopen_file(open_file->pfile, FALSE);
1055 /* if it fails, try another handle - might be */ 1063 if (!rc) {
1056 /* dangerous to hold up writepages with retry */ 1064 if (!open_file->closePend)
1057 if (rc) { 1065 return open_file;
1058 cFYI(1, ("wp failed on reopen file")); 1066 else { /* start over in case this was deleted */
1067 /* since the list could be modified */
1059 read_lock(&GlobalSMBSeslock); 1068 read_lock(&GlobalSMBSeslock);
1060 /* can not use this handle, no write
1061 pending on this one after all */
1062 atomic_dec(&open_file->wrtPending); 1069 atomic_dec(&open_file->wrtPending);
1063 continue; 1070 goto refind_writable;
1064 } 1071 }
1065 } 1072 }
1066 if (open_file->closePend) { 1073
1067 read_lock(&GlobalSMBSeslock); 1074 /* if it fails, try another handle if possible -
1068 atomic_dec(&open_file->wrtPending); 1075 (we can not do this if closePending since
1069 continue; 1076 loop could be modified - in which case we
1070 } 1077 have to start at the beginning of the list
1071 return open_file; 1078 again. Note that it would be bad
1079 to hold up writepages here (rather than
1080 in caller) with continuous retries */
1081 cFYI(1, ("wp failed on reopen file"));
1082 read_lock(&GlobalSMBSeslock);
1083 /* can not use this handle, no write
1084 pending on this one after all */
1085 atomic_dec(&open_file->wrtPending);
1086
1087 if (open_file->closePend) /* list could have changed */
1088 goto refind_writable;
1089 /* else we simply continue to the next entry. Thus
1090 we do not loop on reopen errors. If we
1091 can not reopen the file, for example if we
1092 reconnected to a server with another client
1093 racing to delete or lock the file we would not
1094 make progress if we restarted before the beginning
1095 of the loop here. */
1072 } 1096 }
1073 } 1097 }
1074 read_unlock(&GlobalSMBSeslock); 1098 read_unlock(&GlobalSMBSeslock);