diff mbox

cifs: fix locking and list handling code in cifs_open and its helper

Message ID 1253886817-15046-1-git-send-email-jlayton@redhat.com
State New
Headers show

Commit Message

Jeff Layton Sept. 25, 2009, 1:53 p.m. UTC 
The patch to remove cifs_init_private introduced a locking imbalance. It
didn't remove the leftover list addition code and the unlocking in that
function. cifs_new_fileinfo does the list addition now, so there should
be no need to do it outside of that function.

pCifsInode will never be NULL, so we don't need to check for that. This
patch also gets rid of the ugly locking and unlocking across function
calls.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Steve French <sfrench@us.ibm.com>
---
 fs/cifs/file.c |   22 +++-------------------
 1 files changed, 3 insertions(+), 19 deletions(-)

Comments

Steve French Sept. 25, 2009, 5:59 p.m. UTC | #1 
On Fri, Sep 25, 2009 at 8:53 AM, Jeff Layton <jlayton@redhat.com> wrote:
> The patch to remove cifs_init_private introduced a locking imbalance. It
> didn't remove the leftover list addition code and the unlocking in that
> function. cifs_new_fileinfo does the list addition now, so there should
> be no need to do it outside of that function.
>
> pCifsInode will never be NULL, so we don't need to check for that. This
> patch also gets rid of the ugly locking and unlocking across function
> calls.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> Acked-by: Steve French <sfrench@us.ibm.com>

merged
diff mbox

Patch

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index fee993c..429337e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -201,17 +201,6 @@  static inline int cifs_open_inode_helper(struct inode *inode, struct file *file,
 	struct timespec temp;
 	int rc;
 
-	/* want handles we can use to read with first
-	   in the list so we do not have to walk the
-	   list to search for one in write_begin */
-	if ((file->f_flags & O_ACCMODE) == O_WRONLY) {
-		list_add_tail(&pCifsFile->flist,
-			      &pCifsInode->openFileList);
-	} else {
-		list_add(&pCifsFile->flist,
-			 &pCifsInode->openFileList);
-	}
-	write_unlock(&GlobalSMBSeslock);
 	if (pCifsInode->clientCanCacheRead) {
 		/* we have the inode open somewhere else
 		   no need to discard cache data */
@@ -397,6 +386,7 @@  int cifs_open(struct inode *inode, struct file *file)
 		cFYI(1, ("cifs_open returned 0x%x", rc));
 		goto out;
 	}
+
 	pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt,
 					file->f_flags);
 	file->private_data = pCifsFile;
@@ -405,14 +395,8 @@  int cifs_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
-	if (pCifsInode) {
-		rc = cifs_open_inode_helper(inode, file, pCifsInode,
-					    pCifsFile, tcon,
-					    &oplock, buf, full_path, xid);
-	} else {
-		write_unlock(&GlobalSMBSeslock);
-	}
+	rc = cifs_open_inode_helper(inode, file, pCifsInode, pCifsFile, tcon,
+				    &oplock, buf, full_path, xid);
 
 	if (oplock & CIFS_CREATE_ACTION) {
 		/* time to set mode which we can not set earlier due to