From patchwork Tue Jun 1 14:54:49 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 54221 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by ozlabs.org (Postfix) with ESMTP id 8B974B7D1D for ; Wed, 2 Jun 2010 00:55:16 +1000 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 69872AD0B0; Tue, 1 Jun 2010 08:55:16 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.8 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from cdptpa-omtalb.mail.rr.com (cdptpa-omtalb.mail.rr.com [75.180.132.123]) by lists.samba.org (Postfix) with ESMTP id A8F71AD0B0 for ; Tue, 1 Jun 2010 08:54:54 -0600 (MDT) X-Authority-Analysis: v=1.1 cv=q7FD15Xr545AYYxLuRxh8NAlF1RX961CvM641wXl56g= c=1 sm=0 a=mQgCK12mx8MA:10 a=yQWWgrYGNuUA:10 a=ld/erqUjW76FpBUqCqkKeA==:17 a=20KFwNOVAAAA:8 a=fIi_SD_gIto39FH2dVAA:9 a=cOozHuxcHyVgvw4w7LQA:7 a=qBfrZr62eG1JUfWmsOtrmrWiPQEA:4 a=jEp0ucaQiEUA:10 a=ld/erqUjW76FpBUqCqkKeA==:117 X-Cloudmark-Score: 0 X-Originating-IP: 71.70.153.3 Received: from [71.70.153.3] ([71.70.153.3:56558] helo=mail.poochiereds.net) by cdptpa-oedge04.mail.rr.com (envelope-from ) (ecelerity 2.2.2.39 r()) with ESMTP id 60/3B-04511-C3F150C4; Tue, 01 Jun 2010 14:54:53 +0000 Received: by mail.poochiereds.net (Postfix, from userid 4447) id 85801580CE; Tue, 1 Jun 2010 10:54:52 -0400 (EDT) From: Jeff Layton To: smfrench@gmail.com Date: Tue, 1 Jun 2010 10:54:49 -0400 Message-Id: <1275404092-8400-6-git-send-email-jlayton@redhat.com> X-Mailer: git-send-email 1.6.6.1 In-Reply-To: <1275404092-8400-1-git-send-email-jlayton@redhat.com> References: <1275404092-8400-1-git-send-email-jlayton@redhat.com> Cc: linux-cifs-client@lists.samba.org Subject: [linux-cifs-client] [PATCH 5/8] cifs: pass instantiated filp back after open call X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org 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 --- fs/cifs/dir.c | 35 +++++++++++++++++++++++++++-------- 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 f49afb9..e7ae78b 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "cifsfs.h" #include "cifspdu.h" #include "cifsglob.h" @@ -184,6 +185,8 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, } write_unlock(&GlobalSMBSeslock); + file->private_data = pCifsFile; + return pCifsFile; } @@ -463,14 +466,22 @@ cifs_create_set_dentry: if (newinode && nd && (nd->flags & LOOKUP_OPEN)) { struct cifsFileInfo *pfile_info; - /* - * cifs_fill_filedata() takes care of setting cifsFileInfo - * pointer to file->private_data. - */ - pfile_info = cifs_new_fileinfo(newinode, fileHandle, NULL, + struct file *filp; + + filp = lookup_instantiate_filp(nd, direntry, generic_file_open); + if (IS_ERR(filp)) { + rc = PTR_ERR(filp); + CIFSSMBClose(xid, tcon, fileHandle); + goto cifs_create_out; + } + + pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp, nd->path.mnt, oflags); - if (pfile_info == NULL) + if (pfile_info == NULL) { + fput(filp); + CIFSSMBClose(xid, tcon, fileHandle); rc = -ENOMEM; + } } else { CIFSSMBClose(xid, tcon, fileHandle); } @@ -717,15 +728,23 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, direntry->d_op = &cifs_dentry_ops; d_add(direntry, newInode); if (posix_open) { - cfile = cifs_new_fileinfo(newInode, fileHandle, NULL, + filp = lookup_instantiate_filp(nd, direntry, + generic_file_open); + if (IS_ERR(filp)) { + rc = PTR_ERR(filp); + CIFSSMBClose(xid, pTcon, fileHandle); + goto lookup_out; + } + + cfile = cifs_new_fileinfo(newInode, fileHandle, filp, nd->path.mnt, nd->intent.open.flags); if (cfile == NULL) { + fput(filp); CIFSSMBClose(xid, pTcon, fileHandle); rc = -ENOMEM; goto lookup_out; } - filp = lookup_instantiate_filp(nd, direntry, NULL); } /* since paths are not looked up by component - the parent directories are presumed to be good here */ diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 542e0c8..9cbf0f0 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -162,38 +162,6 @@ psx_client_can_cache: return 0; } -static struct cifsFileInfo * -cifs_fill_filedata(struct file *file) -{ - struct list_head *tmp; - struct cifsFileInfo *pCifsFile = NULL; - struct cifsInodeInfo *pCifsInode = NULL; - - /* search inode for this file and fill in file->private_data */ - pCifsInode = CIFS_I(file->f_path.dentry->d_inode); - read_lock(&GlobalSMBSeslock); - list_for_each(tmp, &pCifsInode->openFileList) { - pCifsFile = list_entry(tmp, struct cifsFileInfo, flist); - if ((pCifsFile->pfile == NULL) && - (pCifsFile->pid == current->tgid)) { - /* mode set in cifs_create */ - - /* needed for writepage */ - pCifsFile->pfile = file; - file->private_data = pCifsFile; - break; - } - } - read_unlock(&GlobalSMBSeslock); - - if (file->private_data != NULL) { - return pCifsFile; - } else if ((file->f_flags & O_CREAT) && (file->f_flags & O_EXCL)) - cERROR(1, "could not find file instance for " - "new file %p", file); - return NULL; -} - /* all arguments to this function must be checked for validity in caller */ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, struct cifsInodeInfo *pCifsInode, struct cifsFileInfo *pCifsFile, @@ -256,7 +224,7 @@ int cifs_open(struct inode *inode, struct file *file) __u32 oplock; struct cifs_sb_info *cifs_sb; struct cifsTconInfo *tcon; - struct cifsFileInfo *pCifsFile; + struct cifsFileInfo *pCifsFile = NULL; struct cifsInodeInfo *pCifsInode; char *full_path = NULL; int desiredAccess; @@ -270,12 +238,6 @@ int cifs_open(struct inode *inode, struct file *file) tcon = cifs_sb->tcon; pCifsInode = CIFS_I(file->f_path.dentry->d_inode); - pCifsFile = cifs_fill_filedata(file); - if (pCifsFile) { - rc = 0; - FreeXid(xid); - return rc; - } full_path = build_path_from_dentry(file->f_path.dentry); if (full_path == NULL) { @@ -315,7 +277,6 @@ int cifs_open(struct inode *inode, struct file *file) rc = -ENOMEM; goto out; } - file->private_data = pCifsFile; cifs_posix_open_inode_helper(inode, file, pCifsInode, oplock, netfid); @@ -401,8 +362,7 @@ int cifs_open(struct inode *inode, struct file *file) pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt, file->f_flags); - file->private_data = pCifsFile; - if (file->private_data == NULL) { + if (pCifsFile == NULL) { rc = -ENOMEM; goto out; }