[PATCH] cifs: revert patches to add open on lookup

From: Jeff Layton
Date: Sat May 16 2009 - 22:08:54 EST


This patch reverts the patches for open on lookup functionality
that was added at the beginning of the 2.6.30 cycle. Specifically,
these commits:

a6ce4932fbdbcd8f8e8c6df76812014351c32892
bc8cd4390c9129fbd286b10fa99972dfb68cd069
88dd47fff4891545bfcfdf39146dde8380771766
90e4ee5d311d4e0729daa676b1d7f754265b5874

Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/cifs/cifsglob.h | 2 +-
fs/cifs/dir.c | 154 ++++++++++++++++++----------------------------------
fs/cifs/file.c | 77 ++++++++++++++++----------
3 files changed, 100 insertions(+), 133 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a61ab77..f1f858a 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -350,7 +350,7 @@ struct cifsFileInfo {
bool invalidHandle:1; /* file closed via session abend */
bool messageMode:1; /* for pipes: message vs byte mode */
atomic_t wrtPending; /* handle in use - defer close */
- struct mutex fh_mutex; /* prevents reopen race after dead ses*/
+ struct semaphore fh_sem; /* prevents reopen race after dead ses*/
struct cifs_search_info srch_inf;
};

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 11431ed..e457e14 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -129,62 +129,12 @@ cifs_bp_rename_retry:
return full_path;
}

-static void
-cifs_fill_fileinfo(struct inode *newinode, __u16 fileHandle,
- struct cifsTconInfo *tcon, bool write_only)
-{
- int oplock = 0;
- struct cifsFileInfo *pCifsFile;
- struct cifsInodeInfo *pCifsInode;
-
- pCifsFile = kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
-
- if (pCifsFile == NULL)
- return;
-
- if (oplockEnabled)
- oplock = REQ_OPLOCK;
-
- pCifsFile->netfid = fileHandle;
- pCifsFile->pid = current->tgid;
- pCifsFile->pInode = newinode;
- pCifsFile->invalidHandle = false;
- pCifsFile->closePend = false;
- mutex_init(&pCifsFile->fh_mutex);
- mutex_init(&pCifsFile->lock_mutex);
- INIT_LIST_HEAD(&pCifsFile->llist);
- atomic_set(&pCifsFile->wrtPending, 0);
-
- /* set the following in open now
- pCifsFile->pfile = file; */
- write_lock(&GlobalSMBSeslock);
- list_add(&pCifsFile->tlist, &tcon->openFileList);
- pCifsInode = CIFS_I(newinode);
- if (pCifsInode) {
- /* if readable file instance put first in list*/
- if (write_only)
- list_add_tail(&pCifsFile->flist,
- &pCifsInode->openFileList);
- else
- list_add(&pCifsFile->flist, &pCifsInode->openFileList);
-
- if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
- pCifsInode->clientCanCacheAll = true;
- pCifsInode->clientCanCacheRead = true;
- cFYI(1, ("Exclusive Oplock inode %p", newinode));
- } else if ((oplock & 0xF) == OPLOCK_READ)
- pCifsInode->clientCanCacheRead = true;
- }
- write_unlock(&GlobalSMBSeslock);
-}
-
int cifs_posix_open(char *full_path, struct inode **pinode,
struct super_block *sb, int mode, int oflags,
int *poplock, __u16 *pnetfid, int xid)
{
int rc;
__u32 oplock;
- bool write_only = false;
FILE_UNIX_BASIC_INFO *presp_data;
__u32 posix_flags = 0;
struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
@@ -222,8 +172,6 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
if (oflags & O_DIRECT)
posix_flags |= SMB_O_DIRECT;

- if (!(oflags & FMODE_READ))
- write_only = true;

rc = CIFSPOSIXCreate(xid, cifs_sb->tcon, posix_flags, mode,
pnetfid, presp_data, &oplock, full_path,
@@ -252,8 +200,6 @@ int cifs_posix_open(char *full_path, struct inode **pinode,

posix_fill_in_inode(*pinode, presp_data, 1);

- cifs_fill_fileinfo(*pinode, *pnetfid, cifs_sb->tcon, write_only);
-
posix_open_ret:
kfree(presp_data);
return rc;
@@ -281,7 +227,6 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
int create_options = CREATE_NOT_DIR;
int oplock = 0;
int oflags;
- bool posix_create = false;
/*
* BB below access is probably too much for mknod to request
* but we have to do query and setpathinfo so requesting
@@ -296,6 +241,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
char *full_path = NULL;
FILE_ALL_INFO *buf = NULL;
struct inode *newinode = NULL;
+ struct cifsInodeInfo *pCifsInode;
int disposition = FILE_OVERWRITE_IF;
bool write_only = false;

@@ -329,13 +275,11 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
negotation. EREMOTE indicates DFS junction, which is not
handled in posix open */

- if (rc == 0) {
- posix_create = true;
- if (newinode == NULL) /* query inode info */
- goto cifs_create_get_file_info;
- else /* success, no need to query */
- goto cifs_create_set_dentry;
- } else if ((rc != -EIO) && (rc != -EREMOTE) &&
+ if ((rc == 0) && (newinode == NULL))
+ goto cifs_create_get_file_info; /* query inode info */
+ else if (rc == 0) /* success, no need to query */
+ goto cifs_create_set_dentry;
+ else if ((rc != -EIO) && (rc != -EREMOTE) &&
(rc != -EOPNOTSUPP)) /* path not found or net err */
goto cifs_create_out;
/* else fallthrough to retry, using older open call, this is
@@ -467,9 +411,45 @@ cifs_create_set_dentry:
if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN))) {
/* mknod case - do not leave file open */
CIFSSMBClose(xid, tcon, fileHandle);
- } else if (!(posix_create) && (newinode)) {
- cifs_fill_fileinfo(newinode, fileHandle,
- cifs_sb->tcon, write_only);
+ } else if (newinode) {
+ struct cifsFileInfo *pCifsFile =
+ kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
+
+ if (pCifsFile == NULL)
+ goto cifs_create_out;
+ pCifsFile->netfid = fileHandle;
+ pCifsFile->pid = current->tgid;
+ pCifsFile->pInode = newinode;
+ pCifsFile->invalidHandle = false;
+ pCifsFile->closePend = false;
+ init_MUTEX(&pCifsFile->fh_sem);
+ mutex_init(&pCifsFile->lock_mutex);
+ INIT_LIST_HEAD(&pCifsFile->llist);
+ atomic_set(&pCifsFile->wrtPending, 0);
+
+ /* set the following in open now
+ pCifsFile->pfile = file; */
+ write_lock(&GlobalSMBSeslock);
+ list_add(&pCifsFile->tlist, &tcon->openFileList);
+ pCifsInode = CIFS_I(newinode);
+ if (pCifsInode) {
+ /* if readable file instance put first in list*/
+ if (write_only) {
+ list_add_tail(&pCifsFile->flist,
+ &pCifsInode->openFileList);
+ } else {
+ list_add(&pCifsFile->flist,
+ &pCifsInode->openFileList);
+ }
+ if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
+ pCifsInode->clientCanCacheAll = true;
+ pCifsInode->clientCanCacheRead = true;
+ cFYI(1, ("Exclusive Oplock inode %p",
+ newinode));
+ } else if ((oplock & 0xF) == OPLOCK_READ)
+ pCifsInode->clientCanCacheRead = true;
+ }
+ write_unlock(&GlobalSMBSeslock);
}
cifs_create_out:
kfree(buf);
@@ -602,21 +582,17 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, int mode,
return rc;
}

+
struct dentry *
cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
struct nameidata *nd)
{
int xid;
int rc = 0; /* to get around spurious gcc warning, set to zero here */
- int oplock = 0;
- int mode;
- __u16 fileHandle = 0;
- bool posix_open = false;
struct cifs_sb_info *cifs_sb;
struct cifsTconInfo *pTcon;
struct inode *newInode = NULL;
char *full_path = NULL;
- struct file *filp;

xid = GetXid();

@@ -658,37 +634,12 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
}
cFYI(1, ("Full path: %s inode = 0x%p", full_path, direntry->d_inode));

- if (pTcon->unix_ext) {
- if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
- (nd->flags & LOOKUP_OPEN)) {
- if (!((nd->intent.open.flags & O_CREAT) &&
- (nd->intent.open.flags & O_EXCL))) {
- mode = nd->intent.open.create_mode &
- ~current_umask();
- rc = cifs_posix_open(full_path, &newInode,
- parent_dir_inode->i_sb, mode,
- nd->intent.open.flags, &oplock,
- &fileHandle, xid);
- /*
- * This code works around a bug in
- * samba posix open in samba versions 3.3.1
- * and earlier where create works
- * but open fails with invalid parameter.
- * If either of these error codes are
- * returned, follow the normal lookup.
- * Otherwise, the error during posix open
- * is handled.
- */
- if ((rc != -EINVAL) && (rc != -EOPNOTSUPP))
- posix_open = true;
- }
- }
- if (!posix_open)
- rc = cifs_get_inode_info_unix(&newInode, full_path,
- parent_dir_inode->i_sb, xid);
- } else
+ if (pTcon->unix_ext)
+ rc = cifs_get_inode_info_unix(&newInode, full_path,
+ parent_dir_inode->i_sb, xid);
+ else
rc = cifs_get_inode_info(&newInode, full_path, NULL,
- parent_dir_inode->i_sb, xid, NULL);
+ parent_dir_inode->i_sb, xid, NULL);

if ((rc == 0) && (newInode != NULL)) {
if (pTcon->nocase)
@@ -696,8 +647,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
else
direntry->d_op = &cifs_dentry_ops;
d_add(direntry, newInode);
- if (posix_open)
- filp = lookup_instantiate_filp(nd, direntry, NULL);
+
/* since paths are not looked up by component - the parent
directories are presumed to be good here */
renew_parental_timestamps(direntry);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 38c06f8..dfd3e6c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -46,7 +46,7 @@ static inline struct cifsFileInfo *cifs_init_private(
memset(private_data, 0, sizeof(struct cifsFileInfo));
private_data->netfid = netfid;
private_data->pid = current->tgid;
- mutex_init(&private_data->fh_mutex);
+ init_MUTEX(&private_data->fh_sem);
mutex_init(&private_data->lock_mutex);
INIT_LIST_HEAD(&private_data->llist);
private_data->pfile = file; /* needed for writepage */
@@ -129,12 +129,15 @@ static inline int cifs_posix_open_inode_helper(struct inode *inode,
struct file *file, struct cifsInodeInfo *pCifsInode,
struct cifsFileInfo *pCifsFile, int oplock, u16 netfid)
{
+ struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+/* struct timespec temp; */ /* BB REMOVEME BB */

file->private_data = kmalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
if (file->private_data == NULL)
return -ENOMEM;
pCifsFile = cifs_init_private(file->private_data, inode, file, netfid);
write_lock(&GlobalSMBSeslock);
+ list_add(&pCifsFile->tlist, &cifs_sb->tcon->openFileList);

pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
if (pCifsInode == NULL) {
@@ -142,6 +145,17 @@ static inline int cifs_posix_open_inode_helper(struct inode *inode,
return -EINVAL;
}

+ /* 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);
+ }
+
if (pCifsInode->clientCanCacheRead) {
/* we have the inode open somewhere else
no need to discard cache data */
@@ -270,32 +284,35 @@ int cifs_open(struct inode *inode, struct file *file)
cifs_sb = CIFS_SB(inode->i_sb);
tcon = cifs_sb->tcon;

- /* 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;
+ if (file->f_flags & O_CREAT) {
+ /* 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) {
+ rc = 0;
+ FreeXid(xid);
+ return rc;
+ } else {
+ if (file->f_flags & O_EXCL)
+ cERROR(1, ("could not find file instance for "
+ "new file %p", file));
}
}
- read_unlock(&GlobalSMBSeslock);
-
- if (file->private_data != NULL) {
- rc = 0;
- FreeXid(xid);
- return rc;
- } else if ((file->f_flags & O_CREAT) && (file->f_flags & O_EXCL))
- cERROR(1, ("could not find file instance for "
- "new file %p", file));

full_path = build_path_from_dentry(file->f_path.dentry);
if (full_path == NULL) {
@@ -483,9 +500,9 @@ static int cifs_reopen_file(struct file *file, bool can_flush)
return -EBADF;

xid = GetXid();
- mutex_unlock(&pCifsFile->fh_mutex);
+ down(&pCifsFile->fh_sem);
if (!pCifsFile->invalidHandle) {
- mutex_lock(&pCifsFile->fh_mutex);
+ up(&pCifsFile->fh_sem);
FreeXid(xid);
return 0;
}
@@ -516,7 +533,7 @@ static int cifs_reopen_file(struct file *file, bool can_flush)
if (full_path == NULL) {
rc = -ENOMEM;
reopen_error_exit:
- mutex_lock(&pCifsFile->fh_mutex);
+ up(&pCifsFile->fh_sem);
FreeXid(xid);
return rc;
}
@@ -558,14 +575,14 @@ reopen_error_exit:
cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc) {
- mutex_lock(&pCifsFile->fh_mutex);
+ up(&pCifsFile->fh_sem);
cFYI(1, ("cifs_open returned 0x%x", rc));
cFYI(1, ("oplock: %d", oplock));
} else {
reopen_success:
pCifsFile->netfid = netfid;
pCifsFile->invalidHandle = false;
- mutex_lock(&pCifsFile->fh_mutex);
+ up(&pCifsFile->fh_sem);
pCifsInode = CIFS_I(inode);
if (pCifsInode) {
if (can_flush) {
--
1.6.2.2


--MP_/amU9RW46AX6yJd=+tMWbg0o--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/