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 | |
| 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>
| -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 | } |
