diff options
author | Jeff Layton <jlayton@redhat.com> | 2010-06-16 13:40:16 -0400 |
---|---|---|
committer | Jeff Layton <jlayton@redhat.com> | 2010-06-16 13:40:16 -0400 |
commit | 6ca9f3bae8b1854794dfa63cdd3b88b7dfe24c13 (patch) | |
tree | b2f880172b2e23ca4f57bef5e2141c062351d6ec /fs | |
parent | 2422f676fb78942d054f7e7a2c3ceaeb7945d814 (diff) |
cifs: pass instantiated filp back after open call
The current scheme of sticking open files on a list and assuming that
cifs_open will scoop them off of it is broken and leads to "Busy
inodes after umount..." errors at unmount time.
The problem is that there is no guarantee that cifs_open will always
be called after a ->lookup or ->create operation. If there are
permissions or other problems, then it's quite likely that it *won't*
be called.
Fix this by fully instantiating the filp whenever the file is created
and pass that filp back to the VFS. If there is a problem, the VFS
can clean up the references.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-and-Tested-by: Suresh Jayaraman <sjayaraman@suse.de>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/cifs/dir.c | 35 | ||||
-rw-r--r-- | fs/cifs/file.c | 44 |
2 files changed, 29 insertions, 50 deletions
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index f49afb9980e7..e7ae78b66fa1 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c | |||
@@ -25,6 +25,7 @@ | |||
25 | #include <linux/slab.h> | 25 | #include <linux/slab.h> |
26 | #include <linux/namei.h> | 26 | #include <linux/namei.h> |
27 | #include <linux/mount.h> | 27 | #include <linux/mount.h> |
28 | #include <linux/file.h> | ||
28 | #include "cifsfs.h" | 29 | #include "cifsfs.h" |
29 | #include "cifspdu.h" | 30 | #include "cifspdu.h" |
30 | #include "cifsglob.h" | 31 | #include "cifsglob.h" |
@@ -184,6 +185,8 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, | |||
184 | } | 185 | } |
185 | write_unlock(&GlobalSMBSeslock); | 186 | write_unlock(&GlobalSMBSeslock); |
186 | 187 | ||
188 | file->private_data = pCifsFile; | ||
189 | |||
187 | return pCifsFile; | 190 | return pCifsFile; |
188 | } | 191 | } |
189 | 192 | ||
@@ -463,14 +466,22 @@ cifs_create_set_dentry: | |||
463 | 466 | ||
464 | if (newinode && nd && (nd->flags & LOOKUP_OPEN)) { | 467 | if (newinode && nd && (nd->flags & LOOKUP_OPEN)) { |
465 | struct cifsFileInfo *pfile_info; | 468 | struct cifsFileInfo *pfile_info; |
466 | /* | 469 | struct file *filp; |
467 | * cifs_fill_filedata() takes care of setting cifsFileInfo | 470 | |
468 | * pointer to file->private_data. | 471 | filp = lookup_instantiate_filp(nd, direntry, generic_file_open); |
469 | */ | 472 | if (IS_ERR(filp)) { |
470 | pfile_info = cifs_new_fileinfo(newinode, fileHandle, NULL, | 473 | rc = PTR_ERR(filp); |
474 | CIFSSMBClose(xid, tcon, fileHandle); | ||
475 | goto cifs_create_out; | ||
476 | } | ||
477 | |||
478 | pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp, | ||
471 | nd->path.mnt, oflags); | 479 | nd->path.mnt, oflags); |
472 | if (pfile_info == NULL) | 480 | if (pfile_info == NULL) { |
481 | fput(filp); | ||
482 | CIFSSMBClose(xid, tcon, fileHandle); | ||
473 | rc = -ENOMEM; | 483 | rc = -ENOMEM; |
484 | } | ||
474 | } else { | 485 | } else { |
475 | CIFSSMBClose(xid, tcon, fileHandle); | 486 | CIFSSMBClose(xid, tcon, fileHandle); |
476 | } | 487 | } |
@@ -717,15 +728,23 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, | |||
717 | direntry->d_op = &cifs_dentry_ops; | 728 | direntry->d_op = &cifs_dentry_ops; |
718 | d_add(direntry, newInode); | 729 | d_add(direntry, newInode); |
719 | if (posix_open) { | 730 | if (posix_open) { |
720 | cfile = cifs_new_fileinfo(newInode, fileHandle, NULL, | 731 | filp = lookup_instantiate_filp(nd, direntry, |
732 | generic_file_open); | ||
733 | if (IS_ERR(filp)) { | ||
734 | rc = PTR_ERR(filp); | ||
735 | CIFSSMBClose(xid, pTcon, fileHandle); | ||
736 | goto lookup_out; | ||
737 | } | ||
738 | |||
739 | cfile = cifs_new_fileinfo(newInode, fileHandle, filp, | ||
721 | nd->path.mnt, | 740 | nd->path.mnt, |
722 | nd->intent.open.flags); | 741 | nd->intent.open.flags); |
723 | if (cfile == NULL) { | 742 | if (cfile == NULL) { |
743 | fput(filp); | ||
724 | CIFSSMBClose(xid, pTcon, fileHandle); | 744 | CIFSSMBClose(xid, pTcon, fileHandle); |
725 | rc = -ENOMEM; | 745 | rc = -ENOMEM; |
726 | goto lookup_out; | 746 | goto lookup_out; |
727 | } | 747 | } |
728 | filp = lookup_instantiate_filp(nd, direntry, NULL); | ||
729 | } | 748 | } |
730 | /* since paths are not looked up by component - the parent | 749 | /* since paths are not looked up by component - the parent |
731 | directories are presumed to be good here */ | 750 | directories are presumed to be good here */ |
diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 542e0c874d64..9cbf0f097a39 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c | |||
@@ -162,38 +162,6 @@ psx_client_can_cache: | |||
162 | return 0; | 162 | return 0; |
163 | } | 163 | } |
164 | 164 | ||
165 | static struct cifsFileInfo * | ||
166 | cifs_fill_filedata(struct file *file) | ||
167 | { | ||
168 | struct list_head *tmp; | ||
169 | struct cifsFileInfo *pCifsFile = NULL; | ||
170 | struct cifsInodeInfo *pCifsInode = NULL; | ||
171 | |||
172 | /* search inode for this file and fill in file->private_data */ | ||
173 | pCifsInode = CIFS_I(file->f_path.dentry->d_inode); | ||
174 | read_lock(&GlobalSMBSeslock); | ||
175 | list_for_each(tmp, &pCifsInode->openFileList) { | ||
176 | pCifsFile = list_entry(tmp, struct cifsFileInfo, flist); | ||
177 | if ((pCifsFile->pfile == NULL) && | ||
178 | (pCifsFile->pid == current->tgid)) { | ||
179 | /* mode set in cifs_create */ | ||
180 | |||
181 | /* needed for writepage */ | ||
182 | pCifsFile->pfile = file; | ||
183 | file->private_data = pCifsFile; | ||
184 | break; | ||
185 | } | ||
186 | } | ||
187 | read_unlock(&GlobalSMBSeslock); | ||
188 | |||
189 | if (file->private_data != NULL) { | ||
190 | return pCifsFile; | ||
191 | } else if ((file->f_flags & O_CREAT) && (file->f_flags & O_EXCL)) | ||
192 | cERROR(1, "could not find file instance for " | ||
193 | "new file %p", file); | ||
194 | return NULL; | ||
195 | } | ||
196 | |||
197 | /* all arguments to this function must be checked for validity in caller */ | 165 | /* all arguments to this function must be checked for validity in caller */ |
198 | static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, | 166 | static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, |
199 | struct cifsInodeInfo *pCifsInode, struct cifsFileInfo *pCifsFile, | 167 | struct cifsInodeInfo *pCifsInode, struct cifsFileInfo *pCifsFile, |
@@ -256,7 +224,7 @@ int cifs_open(struct inode *inode, struct file *file) | |||
256 | __u32 oplock; | 224 | __u32 oplock; |
257 | struct cifs_sb_info *cifs_sb; | 225 | struct cifs_sb_info *cifs_sb; |
258 | struct cifsTconInfo *tcon; | 226 | struct cifsTconInfo *tcon; |
259 | struct cifsFileInfo *pCifsFile; | 227 | struct cifsFileInfo *pCifsFile = NULL; |
260 | struct cifsInodeInfo *pCifsInode; | 228 | struct cifsInodeInfo *pCifsInode; |
261 | char *full_path = NULL; | 229 | char *full_path = NULL; |
262 | int desiredAccess; | 230 | int desiredAccess; |
@@ -270,12 +238,6 @@ int cifs_open(struct inode *inode, struct file *file) | |||
270 | tcon = cifs_sb->tcon; | 238 | tcon = cifs_sb->tcon; |
271 | 239 | ||
272 | pCifsInode = CIFS_I(file->f_path.dentry->d_inode); | 240 | pCifsInode = CIFS_I(file->f_path.dentry->d_inode); |
273 | pCifsFile = cifs_fill_filedata(file); | ||
274 | if (pCifsFile) { | ||
275 | rc = 0; | ||
276 | FreeXid(xid); | ||
277 | return rc; | ||
278 | } | ||
279 | 241 | ||
280 | full_path = build_path_from_dentry(file->f_path.dentry); | 242 | full_path = build_path_from_dentry(file->f_path.dentry); |
281 | if (full_path == NULL) { | 243 | if (full_path == NULL) { |
@@ -315,7 +277,6 @@ int cifs_open(struct inode *inode, struct file *file) | |||
315 | rc = -ENOMEM; | 277 | rc = -ENOMEM; |
316 | goto out; | 278 | goto out; |
317 | } | 279 | } |
318 | file->private_data = pCifsFile; | ||
319 | 280 | ||
320 | cifs_posix_open_inode_helper(inode, file, pCifsInode, | 281 | cifs_posix_open_inode_helper(inode, file, pCifsInode, |
321 | oplock, netfid); | 282 | oplock, netfid); |
@@ -401,8 +362,7 @@ int cifs_open(struct inode *inode, struct file *file) | |||
401 | 362 | ||
402 | pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt, | 363 | pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt, |
403 | file->f_flags); | 364 | file->f_flags); |
404 | file->private_data = pCifsFile; | 365 | if (pCifsFile == NULL) { |
405 | if (file->private_data == NULL) { | ||
406 | rc = -ENOMEM; | 366 | rc = -ENOMEM; |
407 | goto out; | 367 | goto out; |
408 | } | 368 | } |