* [PATCH 01/13] cifs: fix flags handling in cifs_posix_open
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-15 19:33 ` Jeff Layton
[not found] ` <1287171248-464-2-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:33 ` [PATCH 02/13] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
` (11 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
The way flags are passed and converted for cifs_posix_open is rather
non-sensical. Some callers call cifs_posix_convert_flags on the flags
before they pass them to cifs_posix_open, whereas some don't. Two flag
conversion steps is just confusing though.
Change the function instead to clearly expect input in f_flags format,
and fix the callers to pass that in. Then, have cifs_posix_open call
cifs_convert_posix_flags to do the conversion. Move cifs_posix_open to
file.c as well so we can keep cifs_convert_posix_flags as a static
function.
Fix it also to not ignore O_CREAT, O_EXCL and O_TRUNC, and instead have
cifs_reopen_file mask those bits off before calling cifs_posix_open.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsproto.h | 2 +-
fs/cifs/dir.c | 99 +++------------------------------------------
fs/cifs/file.c | 113 +++++++++++++++++++++++++++++++++++++++-----------
3 files changed, 95 insertions(+), 119 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 7f416ab..4f7edba 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -111,7 +111,7 @@ extern struct cifsFileInfo *cifs_new_fileinfo(struct inode *newinode,
unsigned int oflags, __u32 oplock);
extern int cifs_posix_open(char *full_path, struct inode **pinode,
struct super_block *sb,
- int mode, int oflags,
+ int mode, unsigned int f_flags,
__u32 *poplock, __u16 *pnetfid, int xid);
void cifs_fill_uniqueid(struct super_block *sb, struct cifs_fattr *fattr);
extern void cifs_unix_basic_to_fattr(struct cifs_fattr *fattr,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c205ec9..8c1af71 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -181,93 +181,6 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
return pCifsFile;
}
-int cifs_posix_open(char *full_path, struct inode **pinode,
- struct super_block *sb, int mode, int oflags,
- __u32 *poplock, __u16 *pnetfid, int xid)
-{
- int rc;
- FILE_UNIX_BASIC_INFO *presp_data;
- __u32 posix_flags = 0;
- struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
- struct cifs_fattr fattr;
- struct tcon_link *tlink;
- struct cifsTconInfo *tcon;
-
- cFYI(1, "posix open %s", full_path);
-
- presp_data = kzalloc(sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL);
- if (presp_data == NULL)
- return -ENOMEM;
-
-/* So far cifs posix extensions can only map the following flags.
- There are other valid fmode oflags such as FMODE_LSEEK, FMODE_PREAD, but
- so far we do not seem to need them, and we can treat them as local only */
- if ((oflags & (FMODE_READ | FMODE_WRITE)) ==
- (FMODE_READ | FMODE_WRITE))
- posix_flags = SMB_O_RDWR;
- else if (oflags & FMODE_READ)
- posix_flags = SMB_O_RDONLY;
- else if (oflags & FMODE_WRITE)
- posix_flags = SMB_O_WRONLY;
- if (oflags & O_CREAT)
- posix_flags |= SMB_O_CREAT;
- if (oflags & O_EXCL)
- posix_flags |= SMB_O_EXCL;
- if (oflags & O_TRUNC)
- posix_flags |= SMB_O_TRUNC;
- /* be safe and imply O_SYNC for O_DSYNC */
- if (oflags & O_DSYNC)
- posix_flags |= SMB_O_SYNC;
- if (oflags & O_DIRECTORY)
- posix_flags |= SMB_O_DIRECTORY;
- if (oflags & O_NOFOLLOW)
- posix_flags |= SMB_O_NOFOLLOW;
- if (oflags & O_DIRECT)
- posix_flags |= SMB_O_DIRECT;
-
- mode &= ~current_umask();
-
- tlink = cifs_sb_tlink(cifs_sb);
- if (IS_ERR(tlink)) {
- rc = PTR_ERR(tlink);
- goto posix_open_ret;
- }
-
- tcon = tlink_tcon(tlink);
- rc = CIFSPOSIXCreate(xid, tcon, posix_flags, mode, pnetfid, presp_data,
- poplock, full_path, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- cifs_put_tlink(tlink);
-
- if (rc)
- goto posix_open_ret;
-
- if (presp_data->Type == cpu_to_le32(-1))
- goto posix_open_ret; /* open ok, caller does qpathinfo */
-
- if (!pinode)
- goto posix_open_ret; /* caller does not need info */
-
- cifs_unix_basic_to_fattr(&fattr, presp_data, cifs_sb);
-
- /* get new inode and set it up */
- if (*pinode == NULL) {
- cifs_fill_uniqueid(sb, &fattr);
- *pinode = cifs_iget(sb, &fattr);
- if (!*pinode) {
- rc = -ENOMEM;
- goto posix_open_ret;
- }
- } else {
- cifs_fattr_to_inode(*pinode, &fattr);
- }
-
-posix_open_ret:
- kfree(presp_data);
- return rc;
-}
-
static void setup_cifs_dentry(struct cifsTconInfo *tcon,
struct dentry *direntry,
struct inode *newinode)
@@ -321,9 +234,9 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
oplock = REQ_OPLOCK;
if (nd && (nd->flags & LOOKUP_OPEN))
- oflags = nd->intent.open.flags;
+ oflags = nd->intent.open.file->f_flags;
else
- oflags = FMODE_READ | SMB_O_CREAT;
+ oflags = O_RDONLY | O_CREAT;
full_path = build_path_from_dentry(direntry);
if (full_path == NULL) {
@@ -359,9 +272,9 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
/* if the file is going to stay open, then we
need to set the desired access properly */
desiredAccess = 0;
- if (oflags & FMODE_READ)
+ if (OPEN_FMODE(oflags) & FMODE_READ)
desiredAccess |= GENERIC_READ; /* is this too little? */
- if (oflags & FMODE_WRITE)
+ if (OPEN_FMODE(oflags) & FMODE_WRITE)
desiredAccess |= GENERIC_WRITE;
if ((oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
@@ -716,11 +629,11 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
if (pTcon->unix_ext) {
if (nd && !(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
(nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open &&
- (nd->intent.open.flags & O_CREAT)) {
+ (nd->intent.open.file->f_flags & O_CREAT)) {
rc = cifs_posix_open(full_path, &newInode,
parent_dir_inode->i_sb,
nd->intent.open.create_mode,
- nd->intent.open.flags, &oplock,
+ nd->intent.open.file->f_flags, &oplock,
&fileHandle, xid);
/*
* The check below works around a bug in POSIX
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c10f085..b1ca6a4 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -60,34 +60,32 @@ static inline int cifs_convert_flags(unsigned int flags)
FILE_READ_DATA);
}
-static inline fmode_t cifs_posix_convert_flags(unsigned int flags)
+static u32 cifs_posix_convert_flags(unsigned int flags)
{
- fmode_t posix_flags = 0;
+ u32 posix_flags = 0;
if ((flags & O_ACCMODE) == O_RDONLY)
- posix_flags = FMODE_READ;
+ posix_flags = SMB_O_RDONLY;
else if ((flags & O_ACCMODE) == O_WRONLY)
- posix_flags = FMODE_WRITE;
- else if ((flags & O_ACCMODE) == O_RDWR) {
- /* GENERIC_ALL is too much permission to request
- can cause unnecessary access denied on create */
- /* return GENERIC_ALL; */
- posix_flags = FMODE_READ | FMODE_WRITE;
- }
- /* can not map O_CREAT or O_EXCL or O_TRUNC flags when
- reopening a file. They had their effect on the original open */
- if (flags & O_APPEND)
- posix_flags |= (fmode_t)O_APPEND;
+ posix_flags = SMB_O_WRONLY;
+ else if ((flags & O_ACCMODE) == O_RDWR)
+ posix_flags = SMB_O_RDWR;
+
+ if (flags & O_CREAT)
+ posix_flags |= SMB_O_CREAT;
+ if (flags & O_EXCL)
+ posix_flags |= SMB_O_EXCL;
+ if (flags & O_TRUNC)
+ posix_flags |= SMB_O_TRUNC;
+ /* be safe and imply O_SYNC for O_DSYNC */
if (flags & O_DSYNC)
- posix_flags |= (fmode_t)O_DSYNC;
- if (flags & __O_SYNC)
- posix_flags |= (fmode_t)__O_SYNC;
+ posix_flags |= SMB_O_SYNC;
if (flags & O_DIRECTORY)
- posix_flags |= (fmode_t)O_DIRECTORY;
+ posix_flags |= SMB_O_DIRECTORY;
if (flags & O_NOFOLLOW)
- posix_flags |= (fmode_t)O_NOFOLLOW;
+ posix_flags |= SMB_O_NOFOLLOW;
if (flags & O_DIRECT)
- posix_flags |= (fmode_t)O_DIRECT;
+ posix_flags |= SMB_O_DIRECT;
return posix_flags;
}
@@ -159,6 +157,68 @@ client_can_cache:
return rc;
}
+int cifs_posix_open(char *full_path, struct inode **pinode,
+ struct super_block *sb, int mode, unsigned int f_flags,
+ __u32 *poplock, __u16 *pnetfid, int xid)
+{
+ int rc;
+ FILE_UNIX_BASIC_INFO *presp_data;
+ __u32 posix_flags = 0;
+ struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+ struct cifs_fattr fattr;
+ struct tcon_link *tlink;
+ struct cifsTconInfo *tcon;
+
+ cFYI(1, "posix open %s", full_path);
+
+ presp_data = kzalloc(sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL);
+ if (presp_data == NULL)
+ return -ENOMEM;
+
+ tlink = cifs_sb_tlink(cifs_sb);
+ if (IS_ERR(tlink)) {
+ rc = PTR_ERR(tlink);
+ goto posix_open_ret;
+ }
+
+ tcon = tlink_tcon(tlink);
+ mode &= ~current_umask();
+
+ posix_flags = cifs_posix_convert_flags(f_flags);
+ rc = CIFSPOSIXCreate(xid, tcon, posix_flags, mode, pnetfid, presp_data,
+ poplock, full_path, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
+ cifs_put_tlink(tlink);
+
+ if (rc)
+ goto posix_open_ret;
+
+ if (presp_data->Type == cpu_to_le32(-1))
+ goto posix_open_ret; /* open ok, caller does qpathinfo */
+
+ if (!pinode)
+ goto posix_open_ret; /* caller does not need info */
+
+ cifs_unix_basic_to_fattr(&fattr, presp_data, cifs_sb);
+
+ /* get new inode and set it up */
+ if (*pinode == NULL) {
+ cifs_fill_uniqueid(sb, &fattr);
+ *pinode = cifs_iget(sb, &fattr);
+ if (!*pinode) {
+ rc = -ENOMEM;
+ goto posix_open_ret;
+ }
+ } else {
+ cifs_fattr_to_inode(*pinode, &fattr);
+ }
+
+posix_open_ret:
+ kfree(presp_data);
+ return rc;
+}
+
int cifs_open(struct inode *inode, struct file *file)
{
int rc = -EACCES;
@@ -205,12 +265,10 @@ int cifs_open(struct inode *inode, struct file *file)
(tcon->ses->capabilities & CAP_UNIX) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
- int oflags = (int) cifs_posix_convert_flags(file->f_flags);
- oflags |= SMB_O_CREAT;
/* can not refresh inode info since size could be stale */
rc = cifs_posix_open(full_path, &inode, inode->i_sb,
cifs_sb->mnt_file_mode /* ignored */,
- oflags, &oplock, &netfid, xid);
+ file->f_flags, &oplock, &netfid, xid);
if (rc == 0) {
cFYI(1, "posix open succeeded");
@@ -426,8 +484,13 @@ reopen_error_exit:
if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
- int oflags = (int) cifs_posix_convert_flags(file->f_flags);
- /* can not refresh inode info since size could be stale */
+
+ /*
+ * O_CREAT, O_EXCL and O_TRUNC already had their effect on the
+ * original open. Must mask them off for a reopen.
+ */
+ unsigned int oflags = file->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);
+
rc = cifs_posix_open(full_path, NULL, inode->i_sb,
cifs_sb->mnt_file_mode /* ignored */,
oflags, &oplock, &netfid, xid);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 02/13] cifs: eliminate oflags option from cifs_new_fileinfo
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:33 ` [PATCH 01/13] cifs: fix flags handling in cifs_posix_open Jeff Layton
@ 2010-10-15 19:33 ` Jeff Layton
[not found] ` <1287171248-464-3-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:33 ` [PATCH 03/13] cifs: eliminate the inode argument " Jeff Layton
` (10 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Eliminate the poor, misunderstood "oflags" option from cifs_new_fileinfo.
The callers mostly pass in the filp->f_flags here.
That's not correct however since we're checking that value for
the presence of FMODE_READ. Luckily that only affects how the f_list is
ordered. What it really wants here is the file->f_mode. Just use that
field from the filp to determine it.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsproto.h | 3 +--
fs/cifs/dir.c | 9 ++++-----
fs/cifs/file.c | 5 ++---
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 4f7edba..781676e 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -107,8 +107,7 @@ extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
extern struct cifsFileInfo *cifs_new_fileinfo(struct inode *newinode,
__u16 fileHandle, struct file *file,
- struct tcon_link *tlink,
- unsigned int oflags, __u32 oplock);
+ struct tcon_link *tlink, __u32 oplock);
extern int cifs_posix_open(char *full_path, struct inode **pinode,
struct super_block *sb,
int mode, unsigned int f_flags,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 8c1af71..ce1fa30 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -132,7 +132,7 @@ cifs_bp_rename_retry:
struct cifsFileInfo *
cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
- struct tcon_link *tlink, unsigned int oflags, __u32 oplock)
+ struct tcon_link *tlink, __u32 oplock)
{
struct dentry *dentry = file->f_path.dentry;
struct cifsFileInfo *pCifsFile;
@@ -161,7 +161,7 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
pCifsInode = CIFS_I(newinode);
if (pCifsInode) {
/* if readable file instance put first in list*/
- if (oflags & FMODE_READ)
+ if (file->f_mode & FMODE_READ)
list_add(&pCifsFile->flist, &pCifsInode->openFileList);
else
list_add_tail(&pCifsFile->flist,
@@ -396,7 +396,7 @@ cifs_create_set_dentry:
}
pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp,
- tlink, oflags, oplock);
+ tlink, oplock);
if (pfile_info == NULL) {
fput(filp);
CIFSSMBClose(xid, tcon, fileHandle);
@@ -670,8 +670,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
}
cfile = cifs_new_fileinfo(newInode, fileHandle, filp,
- tlink, nd->intent.open.flags,
- oplock);
+ tlink, oplock);
if (cfile == NULL) {
fput(filp);
CIFSSMBClose(xid, pTcon, fileHandle);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index b1ca6a4..774e3ac 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -273,7 +273,7 @@ int cifs_open(struct inode *inode, struct file *file)
cFYI(1, "posix open succeeded");
pCifsFile = cifs_new_fileinfo(inode, netfid, file,
- tlink, oflags, oplock);
+ tlink, oplock);
if (pCifsFile == NULL) {
CIFSSMBClose(xid, tcon, netfid);
rc = -ENOMEM;
@@ -365,8 +365,7 @@ int cifs_open(struct inode *inode, struct file *file)
if (rc != 0)
goto out;
- pCifsFile = cifs_new_fileinfo(inode, netfid, file, tlink,
- file->f_flags, oplock);
+ pCifsFile = cifs_new_fileinfo(inode, netfid, file, tlink, oplock);
if (pCifsFile == NULL) {
rc = -ENOMEM;
goto out;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 03/13] cifs: eliminate the inode argument from cifs_new_fileinfo
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:33 ` [PATCH 01/13] cifs: fix flags handling in cifs_posix_open Jeff Layton
2010-10-15 19:33 ` [PATCH 02/13] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
@ 2010-10-15 19:33 ` Jeff Layton
2010-10-15 19:33 ` [PATCH 04/13] cifs: clean up cifs_reopen_file Jeff Layton
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
It already takes a file pointer. The inode associated with that had damn
well better be the same one we're passing in anyway. Thus, there's no
need for a separate argument here.
Also, get rid of the bogus check for a null pCifsInode pointer. The
CIFS_I macro uses container_of(), and that will virtually never return a
NULL pointer anyway.
Finally, move the setting of the canCache* flags outside of the lock.
Other places in the code don't hold that lock when setting it, so I
assume it's not really needed here either.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
fs/cifs/cifsproto.h | 3 +--
fs/cifs/dir.c | 40 ++++++++++++++++++----------------------
fs/cifs/file.c | 6 +++---
3 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 781676e..ca6a0d9 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -105,8 +105,7 @@ extern u64 cifs_UnixTimeToNT(struct timespec);
extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time,
int offset);
-extern struct cifsFileInfo *cifs_new_fileinfo(struct inode *newinode,
- __u16 fileHandle, struct file *file,
+extern struct cifsFileInfo *cifs_new_fileinfo(__u16 fileHandle, struct file *file,
struct tcon_link *tlink, __u32 oplock);
extern int cifs_posix_open(char *full_path, struct inode **pinode,
struct super_block *sb,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index ce1fa30..36cf3e2 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -131,12 +131,13 @@ cifs_bp_rename_retry:
}
struct cifsFileInfo *
-cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
+cifs_new_fileinfo(__u16 fileHandle, struct file *file,
struct tcon_link *tlink, __u32 oplock)
{
struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+ struct cifsInodeInfo *pCifsInode = CIFS_I(inode);
struct cifsFileInfo *pCifsFile;
- struct cifsInodeInfo *pCifsInode;
pCifsFile = kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
if (pCifsFile == NULL)
@@ -158,24 +159,20 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
write_lock(&GlobalSMBSeslock);
list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
- pCifsInode = CIFS_I(newinode);
- if (pCifsInode) {
- /* if readable file instance put first in list*/
- if (file->f_mode & FMODE_READ)
- list_add(&pCifsFile->flist, &pCifsInode->openFileList);
- else
- list_add_tail(&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;
- }
+ /* if readable file instance put first in list*/
+ if (file->f_mode & FMODE_READ)
+ list_add(&pCifsFile->flist, &pCifsInode->openFileList);
+ else
+ list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList);
write_unlock(&GlobalSMBSeslock);
+ if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
+ pCifsInode->clientCanCacheAll = true;
+ pCifsInode->clientCanCacheRead = true;
+ cFYI(1, "Exclusive Oplock inode %p", inode);
+ } else if ((oplock & 0xF) == OPLOCK_READ)
+ pCifsInode->clientCanCacheRead = true;
+
file->private_data = pCifsFile;
return pCifsFile;
@@ -395,8 +392,7 @@ cifs_create_set_dentry:
goto cifs_create_out;
}
- pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp,
- tlink, oplock);
+ pfile_info = cifs_new_fileinfo(fileHandle, filp, tlink, oplock);
if (pfile_info == NULL) {
fput(filp);
CIFSSMBClose(xid, tcon, fileHandle);
@@ -669,8 +665,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
goto lookup_out;
}
- cfile = cifs_new_fileinfo(newInode, fileHandle, filp,
- tlink, oplock);
+ cfile = cifs_new_fileinfo(fileHandle, filp, tlink,
+ oplock);
if (cfile == NULL) {
fput(filp);
CIFSSMBClose(xid, pTcon, fileHandle);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 774e3ac..394cf28 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -272,8 +272,8 @@ int cifs_open(struct inode *inode, struct file *file)
if (rc == 0) {
cFYI(1, "posix open succeeded");
- pCifsFile = cifs_new_fileinfo(inode, netfid, file,
- tlink, oplock);
+ pCifsFile = cifs_new_fileinfo(netfid, file, tlink,
+ oplock);
if (pCifsFile == NULL) {
CIFSSMBClose(xid, tcon, netfid);
rc = -ENOMEM;
@@ -365,7 +365,7 @@ int cifs_open(struct inode *inode, struct file *file)
if (rc != 0)
goto out;
- pCifsFile = cifs_new_fileinfo(inode, netfid, file, tlink, oplock);
+ pCifsFile = cifs_new_fileinfo(netfid, file, tlink, oplock);
if (pCifsFile == NULL) {
rc = -ENOMEM;
goto out;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 04/13] cifs: clean up cifs_reopen_file
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2010-10-15 19:33 ` [PATCH 03/13] cifs: eliminate the inode argument " Jeff Layton
@ 2010-10-15 19:33 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 05/13] cifs: cifs_write argument change and cleanup Jeff Layton
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Add a f_flags field that holds the f_flags field from the filp. We'll
need this info in case the filp ever goes away before the cifsFileInfo
does. Have cifs_reopen_file use that value instead of filp->f_flags
too and have it take a cifsFileInfo arg instead of a filp.
While we're at it, get rid of some bogus cargo-cult NULL pointer
checks in that function and reduce the level of indentation.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
fs/cifs/cifsglob.h | 1 +
fs/cifs/dir.c | 1 +
fs/cifs/file.c | 129 ++++++++++++++++++++++------------------------------
3 files changed, 57 insertions(+), 74 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 9fc77a4..2eb054b 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -390,6 +390,7 @@ struct cifsFileInfo {
/* lock scope id (0 if none) */
struct file *pfile; /* needed for writepage */
struct dentry *dentry;
+ unsigned int f_flags;
struct tcon_link *tlink;
struct mutex lock_mutex;
struct list_head llist; /* list of byte range locks we have. */
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 36cf3e2..3c23d88 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -147,6 +147,7 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
pCifsFile->pid = current->tgid;
pCifsFile->uid = current_fsuid();
pCifsFile->dentry = dget(dentry);
+ pCifsFile->f_flags = file->f_flags;
pCifsFile->pfile = file;
pCifsFile->invalidHandle = false;
pCifsFile->closePend = false;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 394cf28..dd922a5 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -412,14 +412,13 @@ static int cifs_relock_file(struct cifsFileInfo *cifsFile)
return rc;
}
-static int cifs_reopen_file(struct file *file, bool can_flush)
+static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
{
int rc = -EACCES;
int xid;
__u32 oplock;
struct cifs_sb_info *cifs_sb;
struct cifsTconInfo *tcon;
- struct cifsFileInfo *pCifsFile;
struct cifsInodeInfo *pCifsInode;
struct inode *inode;
char *full_path = NULL;
@@ -427,11 +426,6 @@ static int cifs_reopen_file(struct file *file, bool can_flush)
int disposition = FILE_OPEN;
__u16 netfid;
- if (file->private_data)
- pCifsFile = file->private_data;
- else
- return -EBADF;
-
xid = GetXid();
mutex_lock(&pCifsFile->fh_mutex);
if (!pCifsFile->invalidHandle) {
@@ -441,21 +435,7 @@ static int cifs_reopen_file(struct file *file, bool can_flush)
return rc;
}
- if (file->f_path.dentry == NULL) {
- cERROR(1, "no valid name if dentry freed");
- dump_stack();
- rc = -EBADF;
- goto reopen_error_exit;
- }
-
- inode = file->f_path.dentry->d_inode;
- if (inode == NULL) {
- cERROR(1, "inode not valid");
- dump_stack();
- rc = -EBADF;
- goto reopen_error_exit;
- }
-
+ inode = pCifsFile->dentry->d_inode;
cifs_sb = CIFS_SB(inode->i_sb);
tcon = tlink_tcon(pCifsFile->tlink);
@@ -463,17 +443,16 @@ static int cifs_reopen_file(struct file *file, bool can_flush)
those that already have the rename sem can end up causing writepage
to get called and if the server was down that means we end up here,
and we can never tell if the caller already has the rename_sem */
- full_path = build_path_from_dentry(file->f_path.dentry);
+ full_path = build_path_from_dentry(pCifsFile->dentry);
if (full_path == NULL) {
rc = -ENOMEM;
-reopen_error_exit:
mutex_unlock(&pCifsFile->fh_mutex);
FreeXid(xid);
return rc;
}
cFYI(1, "inode = 0x%p file flags 0x%x for %s",
- inode, file->f_flags, full_path);
+ inode, pCifsFile->f_flags, full_path);
if (oplockEnabled)
oplock = REQ_OPLOCK;
@@ -488,7 +467,8 @@ reopen_error_exit:
* O_CREAT, O_EXCL and O_TRUNC already had their effect on the
* original open. Must mask them off for a reopen.
*/
- unsigned int oflags = file->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);
+ unsigned int oflags = pCifsFile->f_flags &
+ ~(O_CREAT | O_EXCL | O_TRUNC);
rc = cifs_posix_open(full_path, NULL, inode->i_sb,
cifs_sb->mnt_file_mode /* ignored */,
@@ -501,7 +481,7 @@ reopen_error_exit:
in the reconnect path it is important to retry hard */
}
- desiredAccess = cifs_convert_flags(file->f_flags);
+ desiredAccess = cifs_convert_flags(pCifsFile->f_flags);
/* Can not refresh inode by passing in file_info buf to be returned
by SMBOpen and then calling get_inode_info with returned buf
@@ -517,49 +497,50 @@ reopen_error_exit:
mutex_unlock(&pCifsFile->fh_mutex);
cFYI(1, "cifs_open returned 0x%x", rc);
cFYI(1, "oplock: %d", oplock);
- } else {
+ goto reopen_error_exit;
+ }
+
reopen_success:
- pCifsFile->netfid = netfid;
- pCifsFile->invalidHandle = false;
- mutex_unlock(&pCifsFile->fh_mutex);
- pCifsInode = CIFS_I(inode);
- if (pCifsInode) {
- if (can_flush) {
- rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0)
- CIFS_I(inode)->write_behind_rc = rc;
- /* temporarily disable caching while we
- go to server to get inode info */
- pCifsInode->clientCanCacheAll = false;
- pCifsInode->clientCanCacheRead = false;
- if (tcon->unix_ext)
- rc = cifs_get_inode_info_unix(&inode,
- full_path, inode->i_sb, xid);
- else
- rc = cifs_get_inode_info(&inode,
- full_path, NULL, inode->i_sb,
- xid, NULL);
- } /* else we are writing out data to server already
- and could deadlock if we tried to flush data, and
- since we do not know if we have data that would
- invalidate the current end of file on the server
- we can not go to the server to get the new inod
- info */
- if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
- pCifsInode->clientCanCacheAll = true;
- pCifsInode->clientCanCacheRead = true;
- cFYI(1, "Exclusive Oplock granted on inode %p",
- file->f_path.dentry->d_inode);
- } else if ((oplock & 0xF) == OPLOCK_READ) {
- pCifsInode->clientCanCacheRead = true;
- pCifsInode->clientCanCacheAll = false;
- } else {
- pCifsInode->clientCanCacheRead = false;
- pCifsInode->clientCanCacheAll = false;
- }
- cifs_relock_file(pCifsFile);
- }
+ pCifsFile->netfid = netfid;
+ pCifsFile->invalidHandle = false;
+ mutex_unlock(&pCifsFile->fh_mutex);
+ pCifsInode = CIFS_I(inode);
+
+ if (can_flush) {
+ rc = filemap_write_and_wait(inode->i_mapping);
+ if (rc != 0)
+ CIFS_I(inode)->write_behind_rc = rc;
+
+ pCifsInode->clientCanCacheAll = false;
+ pCifsInode->clientCanCacheRead = false;
+ if (tcon->unix_ext)
+ rc = cifs_get_inode_info_unix(&inode,
+ full_path, inode->i_sb, xid);
+ else
+ rc = cifs_get_inode_info(&inode,
+ full_path, NULL, inode->i_sb,
+ xid, NULL);
+ } /* else we are writing out data to server already
+ and could deadlock if we tried to flush data, and
+ since we do not know if we have data that would
+ invalidate the current end of file on the server
+ we can not go to the server to get the new inod
+ info */
+ if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
+ pCifsInode->clientCanCacheAll = true;
+ pCifsInode->clientCanCacheRead = true;
+ cFYI(1, "Exclusive Oplock granted on inode %p",
+ pCifsFile->dentry->d_inode);
+ } else if ((oplock & 0xF) == OPLOCK_READ) {
+ pCifsInode->clientCanCacheRead = true;
+ pCifsInode->clientCanCacheAll = false;
+ } else {
+ pCifsInode->clientCanCacheRead = false;
+ pCifsInode->clientCanCacheAll = false;
}
+ cifs_relock_file(pCifsFile);
+
+reopen_error_exit:
kfree(full_path);
FreeXid(xid);
return rc;
@@ -998,7 +979,7 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
filemap_fdatawait from here so tell
reopen_file not to flush data to server
now */
- rc = cifs_reopen_file(file, false);
+ rc = cifs_reopen_file(open_file, false);
if (rc != 0)
break;
}
@@ -1096,7 +1077,7 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
filemap_fdatawait from here so tell
reopen_file not to flush data to
server now */
- rc = cifs_reopen_file(file, false);
+ rc = cifs_reopen_file(open_file, false);
if (rc != 0)
break;
}
@@ -1244,7 +1225,7 @@ refind_writable:
read_unlock(&GlobalSMBSeslock);
/* Had to unlock since following call can block */
- rc = cifs_reopen_file(open_file->pfile, false);
+ rc = cifs_reopen_file(open_file, false);
if (!rc) {
if (!open_file->closePend)
return open_file;
@@ -1792,7 +1773,7 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
int buf_type = CIFS_NO_BUFFER;
if ((open_file->invalidHandle) &&
(!open_file->closePend)) {
- rc = cifs_reopen_file(file, true);
+ rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
}
@@ -1878,7 +1859,7 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
while (rc == -EAGAIN) {
if ((open_file->invalidHandle) &&
(!open_file->closePend)) {
- rc = cifs_reopen_file(file, true);
+ rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
}
@@ -2043,7 +2024,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
while (rc == -EAGAIN) {
if ((open_file->invalidHandle) &&
(!open_file->closePend)) {
- rc = cifs_reopen_file(file, true);
+ rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 05/13] cifs: cifs_write argument change and cleanup
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2010-10-15 19:33 ` [PATCH 04/13] cifs: clean up cifs_reopen_file Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 06/13] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Have cifs_write take a cifsFileInfo pointer instead of a filp. Since
cifsFileInfo holds references on the dentry, and that holds one to
the inode, we can eliminate some unneeded NULL pointer checks.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
fs/cifs/file.c | 51 +++++++++++++++++----------------------------------
1 files changed, 17 insertions(+), 34 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index dd922a5..d5f3007 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1027,8 +1027,9 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
return total_written;
}
-static ssize_t cifs_write(struct file *file, const char *write_data,
- size_t write_size, loff_t *poffset)
+static ssize_t cifs_write(struct cifsFileInfo *open_file,
+ const char *write_data, size_t write_size,
+ loff_t *poffset)
{
int rc = 0;
unsigned int bytes_written = 0;
@@ -1036,17 +1037,14 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
struct cifs_sb_info *cifs_sb;
struct cifsTconInfo *pTcon;
int xid, long_op;
- struct cifsFileInfo *open_file;
- struct cifsInodeInfo *cifsi = CIFS_I(file->f_path.dentry->d_inode);
+ struct dentry *dentry = open_file->dentry;
+ struct cifsInodeInfo *cifsi = CIFS_I(dentry->d_inode);
- cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
+ cifs_sb = CIFS_SB(dentry->d_sb);
cFYI(1, "write %zd bytes to offset %lld of %s", write_size,
- *poffset, file->f_path.dentry->d_name.name);
+ *poffset, dentry->d_name.name);
- if (file->private_data == NULL)
- return -EBADF;
- open_file = file->private_data;
pTcon = tlink_tcon(open_file->tlink);
xid = GetXid();
@@ -1056,15 +1054,6 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
total_written += bytes_written) {
rc = -EAGAIN;
while (rc == -EAGAIN) {
- if (file->private_data == NULL) {
- /* file has been closed on us */
- FreeXid(xid);
- /* if we have gotten here we have written some data
- and blocked, and the file has been freed on us
- while we blocked so return what we managed to
- write */
- return total_written;
- }
if (open_file->closePend) {
FreeXid(xid);
if (total_written)
@@ -1124,20 +1113,13 @@ static ssize_t cifs_write(struct file *file, const char *write_data,
cifs_stats_bytes_written(pTcon, total_written);
- /* since the write may have blocked check these pointers again */
- if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
-/*BB We could make this contingent on superblock ATIME flag too */
-/* file->f_path.dentry->d_inode->i_ctime =
- file->f_path.dentry->d_inode->i_mtime = CURRENT_TIME;*/
- if (total_written > 0) {
- spin_lock(&file->f_path.dentry->d_inode->i_lock);
- if (*poffset > file->f_path.dentry->d_inode->i_size)
- i_size_write(file->f_path.dentry->d_inode,
- *poffset);
- spin_unlock(&file->f_path.dentry->d_inode->i_lock);
- }
- mark_inode_dirty_sync(file->f_path.dentry->d_inode);
+ if (total_written > 0) {
+ spin_lock(&dentry->d_inode->i_lock);
+ if (*poffset > dentry->d_inode->i_size)
+ i_size_write(dentry->d_inode, *poffset);
+ spin_unlock(&dentry->d_inode->i_lock);
}
+ mark_inode_dirty_sync(dentry->d_inode);
FreeXid(xid);
return total_written;
}
@@ -1308,8 +1290,8 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
open_file = find_writable_file(CIFS_I(mapping->host), false);
if (open_file) {
- bytes_written = cifs_write(open_file->pfile, write_data,
- to-from, &offset);
+ bytes_written = cifs_write(open_file, write_data,
+ to - from, &offset);
cifsFileInfo_put(open_file);
/* Does mm or vfs already set times? */
inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb);
@@ -1624,7 +1606,8 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
/* BB check if anything else missing out of ppw
such as updating last write time */
page_data = kmap(page);
- rc = cifs_write(file, page_data + offset, copied, &pos);
+ rc = cifs_write(file->private_data, page_data + offset,
+ copied, &pos);
/* if (rc < 0) should we set writebehind rc? */
kunmap(page);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 06/13] cifs: eliminate pfile pointer from cifsFileInfo
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 05/13] cifs: cifs_write argument change and cleanup Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 07/13] cifs: move cifs_new_fileinfo to file.c Jeff Layton
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
All the remaining users of cifsFileInfo->pfile just use it to get
at the f_flags/f_mode. Now that we store that separately in the
cifsFileInfo, there's no need to consult the pfile at all from
a cifsFileInfo pointer.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
fs/cifs/cifsglob.h | 1 -
fs/cifs/dir.c | 1 -
fs/cifs/file.c | 11 +++--------
3 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 2eb054b..2ecf675 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -388,7 +388,6 @@ struct cifsFileInfo {
__u16 netfid; /* file id from remote */
/* BB add lock scope info here if needed */ ;
/* lock scope id (0 if none) */
- struct file *pfile; /* needed for writepage */
struct dentry *dentry;
unsigned int f_flags;
struct tcon_link *tlink;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 3c23d88..600eac1 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -148,7 +148,6 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
pCifsFile->uid = current_fsuid();
pCifsFile->dentry = dget(dentry);
pCifsFile->f_flags = file->f_flags;
- pCifsFile->pfile = file;
pCifsFile->invalidHandle = false;
pCifsFile->closePend = false;
pCifsFile->tlink = cifs_get_tlink(tlink);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index d5f3007..7935816 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1144,8 +1144,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
continue;
if (fsuid_only && open_file->uid != current_fsuid())
continue;
- if (open_file->pfile && ((open_file->pfile->f_flags & O_RDWR) ||
- (open_file->pfile->f_flags & O_RDONLY))) {
+ if (OPEN_FMODE(open_file->f_flags) & FMODE_READ) {
if (!open_file->invalidHandle) {
/* found a good file */
/* lock it so it will not be closed on us */
@@ -1194,9 +1193,7 @@ refind_writable:
continue;
if (fsuid_only && open_file->uid != current_fsuid())
continue;
- if (open_file->pfile &&
- ((open_file->pfile->f_flags & O_RDWR) ||
- (open_file->pfile->f_flags & O_WRONLY))) {
+ if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
cifsFileInfo_get(open_file);
if (!open_file->invalidHandle) {
@@ -2160,9 +2157,7 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (open_file->closePend)
continue;
- if (open_file->pfile &&
- ((open_file->pfile->f_flags & O_RDWR) ||
- (open_file->pfile->f_flags & O_WRONLY))) {
+ if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
read_unlock(&GlobalSMBSeslock);
return 1;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 07/13] cifs: move cifs_new_fileinfo to file.c
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 06/13] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
[not found] ` <1287171248-464-8-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:34 ` [PATCH 08/13] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
` (5 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
It's currently in dir.c which makes little sense...
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
fs/cifs/dir.c | 48 ------------------------------------------------
fs/cifs/file.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 48 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 600eac1..3840edd 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -130,54 +130,6 @@ cifs_bp_rename_retry:
return full_path;
}
-struct cifsFileInfo *
-cifs_new_fileinfo(__u16 fileHandle, struct file *file,
- struct tcon_link *tlink, __u32 oplock)
-{
- struct dentry *dentry = file->f_path.dentry;
- struct inode *inode = dentry->d_inode;
- struct cifsInodeInfo *pCifsInode = CIFS_I(inode);
- struct cifsFileInfo *pCifsFile;
-
- pCifsFile = kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
- if (pCifsFile == NULL)
- return pCifsFile;
-
- pCifsFile->netfid = fileHandle;
- pCifsFile->pid = current->tgid;
- pCifsFile->uid = current_fsuid();
- pCifsFile->dentry = dget(dentry);
- pCifsFile->f_flags = file->f_flags;
- pCifsFile->invalidHandle = false;
- pCifsFile->closePend = false;
- pCifsFile->tlink = cifs_get_tlink(tlink);
- mutex_init(&pCifsFile->fh_mutex);
- mutex_init(&pCifsFile->lock_mutex);
- INIT_LIST_HEAD(&pCifsFile->llist);
- atomic_set(&pCifsFile->count, 1);
- INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
-
- write_lock(&GlobalSMBSeslock);
- list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
- /* if readable file instance put first in list*/
- if (file->f_mode & FMODE_READ)
- list_add(&pCifsFile->flist, &pCifsInode->openFileList);
- else
- list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList);
- write_unlock(&GlobalSMBSeslock);
-
- if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
- pCifsInode->clientCanCacheAll = true;
- pCifsInode->clientCanCacheRead = true;
- cFYI(1, "Exclusive Oplock inode %p", inode);
- } else if ((oplock & 0xF) == OPLOCK_READ)
- pCifsInode->clientCanCacheRead = true;
-
- file->private_data = pCifsFile;
-
- return pCifsFile;
-}
-
static void setup_cifs_dentry(struct cifsTconInfo *tcon,
struct dentry *direntry,
struct inode *newinode)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 7935816..293e9b7 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -219,6 +219,53 @@ posix_open_ret:
return rc;
}
+struct cifsFileInfo *
+cifs_new_fileinfo(__u16 fileHandle, struct file *file,
+ struct tcon_link *tlink, __u32 oplock)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+ struct cifsInodeInfo *pCifsInode = CIFS_I(inode);
+ struct cifsFileInfo *pCifsFile;
+
+ pCifsFile = kzalloc(sizeof(struct cifsFileInfo), GFP_KERNEL);
+ if (pCifsFile == NULL)
+ return pCifsFile;
+
+ pCifsFile->netfid = fileHandle;
+ pCifsFile->pid = current->tgid;
+ pCifsFile->uid = current_fsuid();
+ pCifsFile->dentry = dget(dentry);
+ pCifsFile->f_flags = file->f_flags;
+ pCifsFile->invalidHandle = false;
+ pCifsFile->closePend = false;
+ pCifsFile->tlink = cifs_get_tlink(tlink);
+ mutex_init(&pCifsFile->fh_mutex);
+ mutex_init(&pCifsFile->lock_mutex);
+ INIT_LIST_HEAD(&pCifsFile->llist);
+ atomic_set(&pCifsFile->count, 1);
+ INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
+
+ write_lock(&GlobalSMBSeslock);
+ list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
+ /* if readable file instance put first in list*/
+ if (file->f_mode & FMODE_READ)
+ list_add(&pCifsFile->flist, &pCifsInode->openFileList);
+ else
+ list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList);
+ write_unlock(&GlobalSMBSeslock);
+
+ if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
+ pCifsInode->clientCanCacheAll = true;
+ pCifsInode->clientCanCacheRead = true;
+ cFYI(1, "Exclusive Oplock inode %p", inode);
+ } else if ((oplock & 0xF) == OPLOCK_READ)
+ pCifsInode->clientCanCacheRead = true;
+
+ file->private_data = pCifsFile;
+ return pCifsFile;
+}
+
int cifs_open(struct inode *inode, struct file *file)
{
int rc = -EACCES;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 08/13] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (6 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 07/13] cifs: move cifs_new_fileinfo to file.c Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
[not found] ` <1287171248-464-9-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:34 ` [PATCH 09/13] cifs: move cifsFileInfo_put to file.c Jeff Layton
` (4 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Convert this lock to a regular spinlock
A rwlock_t offers little value here. It's more expensive than a regular
spinlock unless you have a fairly large section of code that runs under
the read lock and can benefit from the concurrency.
Additionally, we need to ensure that the refcounting for files isn't
racy and to do that we need to lock areas that can increment it for
write. That means that the areas that can actually use a read_lock are
very few and relatively infrequently used.
While we're at it, change the name to something easier to type, and fix
a bug in find_writable_file. cifsFileInfo_put can sleep and shouldn't be
called while holding the lock.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsfs.c | 2 +-
fs/cifs/cifsglob.h | 2 +-
fs/cifs/cifssmb.c | 4 +-
fs/cifs/file.c | 54 ++++++++++++++++++++++++++--------------------------
fs/cifs/misc.c | 8 +++---
fs/cifs/readdir.c | 6 ++--
6 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 3facf25..c25e928 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -942,8 +942,8 @@ init_cifs(void)
GlobalTotalActiveXid = 0;
GlobalMaxActiveXid = 0;
memset(Local_System_Name, 0, 15);
- rwlock_init(&GlobalSMBSeslock);
rwlock_init(&cifs_tcp_ses_lock);
+ spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
if (cifs_max_pending < 2) {
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 2ecf675..087bc4b 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -722,7 +722,7 @@ GLOBAL_EXTERN rwlock_t cifs_tcp_ses_lock;
* If cifs_tcp_ses_lock and the lock below are both needed to be held, then
* the cifs_tcp_ses_lock must be grabbed first and released last.
*/
-GLOBAL_EXTERN rwlock_t GlobalSMBSeslock;
+GLOBAL_EXTERN spinlock_t cifs_file_list_lock;
/* Outstanding dir notify requests */
GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index a420c7b..bfb59a6 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -91,13 +91,13 @@ static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
struct list_head *tmp1;
/* list all files open on tree connection and mark them invalid */
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
list_for_each_safe(tmp, tmp1, &pTcon->openFileList) {
open_file = list_entry(tmp, struct cifsFileInfo, tlist);
open_file->invalidHandle = true;
open_file->oplock_break_cancelled = true;
}
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
/* BB Add call to invalidate_inodes(sb) for all superblocks mounted
to this tcon */
}
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 293e9b7..26048dc 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -246,14 +246,14 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
atomic_set(&pCifsFile->count, 1);
INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
/* if readable file instance put first in list*/
if (file->f_mode & FMODE_READ)
list_add(&pCifsFile->flist, &pCifsInode->openFileList);
else
list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList);
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
pCifsInode->clientCanCacheAll = true;
@@ -607,13 +607,13 @@ int cifs_close(struct inode *inode, struct file *file)
pTcon = tlink_tcon(pSMBFile->tlink);
if (pSMBFile) {
struct cifsLockInfo *li, *tmp;
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
pSMBFile->closePend = true;
if (pTcon) {
/* no sense reconnecting to close a file that is
already closed */
if (!pTcon->need_reconnect) {
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
timeout = 2;
while ((atomic_read(&pSMBFile->count) != 1)
&& (timeout <= 2048)) {
@@ -633,9 +633,9 @@ int cifs_close(struct inode *inode, struct file *file)
rc = CIFSSMBClose(xid, pTcon,
pSMBFile->netfid);
} else
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
} else
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
/* Delete any outstanding lock records.
We'll lose them when the file is closed anyway. */
@@ -646,16 +646,16 @@ int cifs_close(struct inode *inode, struct file *file)
}
mutex_unlock(&pSMBFile->lock_mutex);
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
list_del(&pSMBFile->flist);
list_del(&pSMBFile->tlist);
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
cifsFileInfo_put(file->private_data);
file->private_data = NULL;
} else
rc = -EBADF;
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
if (list_empty(&(CIFS_I(inode)->openFileList))) {
cFYI(1, "closing last open instance for inode %p", inode);
/* if the file is not open we do not know if we can cache info
@@ -663,7 +663,7 @@ int cifs_close(struct inode *inode, struct file *file)
CIFS_I(inode)->clientCanCacheRead = false;
CIFS_I(inode)->clientCanCacheAll = false;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
if ((rc == 0) && CIFS_I(inode)->write_behind_rc)
rc = CIFS_I(inode)->write_behind_rc;
FreeXid(xid);
@@ -685,18 +685,18 @@ int cifs_closedir(struct inode *inode, struct file *file)
struct cifsTconInfo *pTcon = tlink_tcon(pCFileStruct->tlink);
cFYI(1, "Freeing private data in close dir");
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
if (!pCFileStruct->srch_inf.endOfSearch &&
!pCFileStruct->invalidHandle) {
pCFileStruct->invalidHandle = true;
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
rc = CIFSFindClose(xid, pTcon, pCFileStruct->netfid);
cFYI(1, "Closing uncompleted readdir with rc %d",
rc);
/* not much we can do if it fails anyway, ignore rc */
rc = 0;
} else
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
ptmp = pCFileStruct->srch_inf.ntwrk_buf_start;
if (ptmp) {
cFYI(1, "closedir free smb buf in srch struct");
@@ -1182,7 +1182,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
fsuid_only = false;
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
/* we could simply get the first_list_entry since write-only entries
are always at the end of the list but since the first entry might
have a close pending, we go through the whole list */
@@ -1196,7 +1196,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
/* found a good file */
/* lock it so it will not be closed on us */
cifsFileInfo_get(open_file);
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return open_file;
} /* else might as well continue, and look for
another, or simply have the caller reopen it
@@ -1204,7 +1204,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
} else /* write only file */
break; /* write only files are last so must be done */
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return NULL;
}
#endif
@@ -1231,7 +1231,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
fsuid_only = false;
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
refind_writable:
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (open_file->closePend)
@@ -1245,11 +1245,11 @@ refind_writable:
if (!open_file->invalidHandle) {
/* found a good writable file */
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return open_file;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
/* Had to unlock since following call can block */
rc = cifs_reopen_file(open_file, false);
if (!rc) {
@@ -1257,7 +1257,7 @@ refind_writable:
return open_file;
else { /* start over in case this was deleted */
/* since the list could be modified */
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
cifsFileInfo_put(open_file);
goto refind_writable;
}
@@ -1271,7 +1271,7 @@ refind_writable:
to hold up writepages here (rather than
in caller) with continuous retries */
cFYI(1, "wp failed on reopen file");
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
/* can not use this handle, no write
pending on this one after all */
cifsFileInfo_put(open_file);
@@ -1292,7 +1292,7 @@ refind_writable:
any_available = true;
goto refind_writable;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return NULL;
}
@@ -2200,16 +2200,16 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
{
struct cifsFileInfo *open_file;
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (open_file->closePend)
continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return 1;
}
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return 0;
}
@@ -2373,8 +2373,8 @@ void cifs_oplock_break(struct work_struct *work)
* finished grabbing reference for us. Make sure it's done by
* waiting for GlobalSMSSeslock.
*/
- write_lock(&GlobalSMBSeslock);
- write_unlock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
+ spin_unlock(&cifs_file_list_lock);
cifs_oplock_break_put(cfile);
}
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 9bac3e7..de6073c 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -560,7 +560,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
continue;
cifs_stats_inc(&tcon->num_oplock_brks);
- read_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
list_for_each(tmp2, &tcon->openFileList) {
netfile = list_entry(tmp2, struct cifsFileInfo,
tlist);
@@ -572,7 +572,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
* closed anyway.
*/
if (netfile->closePend) {
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock);
return true;
}
@@ -594,11 +594,11 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
cifs_oplock_break_get(netfile);
netfile->oplock_break_cancelled = false;
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock);
return true;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
read_unlock(&cifs_tcp_ses_lock);
cFYI(1, "No matching file for oplock break");
return true;
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 7bdcb35..c8961c5 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -529,14 +529,14 @@ static int find_cifs_entry(const int xid, struct cifsTconInfo *pTcon,
(index_to_find < first_entry_in_buffer)) {
/* close and restart search */
cFYI(1, "search backing up - close and restart search");
- write_lock(&GlobalSMBSeslock);
+ spin_lock(&cifs_file_list_lock);
if (!cifsFile->srch_inf.endOfSearch &&
!cifsFile->invalidHandle) {
cifsFile->invalidHandle = true;
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
CIFSFindClose(xid, pTcon, cifsFile->netfid);
} else
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
if (cifsFile->srch_inf.ntwrk_buf_start) {
cFYI(1, "freeing SMB ff cache buf on search rewind");
if (cifsFile->srch_inf.smallBuf)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 09/13] cifs: move cifsFileInfo_put to file.c
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (7 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 08/13] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 10/13] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
...and make it non-inlined in preparation for the move of most of
cifs_close to it.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsglob.h | 10 +---------
fs/cifs/file.c | 10 ++++++++++
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 087bc4b..d5beb42 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -408,15 +408,7 @@ static inline void cifsFileInfo_get(struct cifsFileInfo *cifs_file)
atomic_inc(&cifs_file->count);
}
-/* Release a reference on the file private data */
-static inline void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
-{
- if (atomic_dec_and_test(&cifs_file->count)) {
- cifs_put_tlink(cifs_file->tlink);
- dput(cifs_file->dentry);
- kfree(cifs_file);
- }
-}
+void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
/*
* One of these for each file inode
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 26048dc..a3634e4 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -266,6 +266,16 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
return pCifsFile;
}
+/* Release a reference on the file private data */
+void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
+{
+ if (atomic_dec_and_test(&cifs_file->count)) {
+ cifs_put_tlink(cifs_file->tlink);
+ dput(cifs_file->dentry);
+ kfree(cifs_file);
+ }
+}
+
int cifs_open(struct inode *inode, struct file *file)
{
int rc = -EACCES;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 10/13] cifs: move close processing from cifs_close to cifsFileInfo_put
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (8 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 09/13] cifs: move cifsFileInfo_put to file.c Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 11/13] cifs: convert cifsFileInfo->count to non-atomic counter Jeff Layton
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Now that it's feasible for a cifsFileInfo to outlive the filp under
which it was created, move the close processing into cifsFileInfo_put.
This means that the last user of the filehandle always does the actual
on the wire close call. This also allows us to get rid of the closePend
flag from cifsFileInfo. If we have an active reference to the file
then it's never going to have a close pending.
cifs_close is converted to simply put the filehandle.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsglob.h | 1 -
fs/cifs/file.c | 187 +++++++++++++++++-----------------------------------
fs/cifs/misc.c | 10 ---
3 files changed, 60 insertions(+), 138 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d5beb42..df4629e 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -393,7 +393,6 @@ struct cifsFileInfo {
struct tcon_link *tlink;
struct mutex lock_mutex;
struct list_head llist; /* list of byte range locks we have. */
- bool closePend:1; /* file is marked to close */
bool invalidHandle:1; /* file closed via session abend */
bool oplock_break_cancelled:1;
atomic_t count; /* reference count */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a3634e4..8c81e7b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -238,7 +238,6 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
pCifsFile->dentry = dget(dentry);
pCifsFile->f_flags = file->f_flags;
pCifsFile->invalidHandle = false;
- pCifsFile->closePend = false;
pCifsFile->tlink = cifs_get_tlink(tlink);
mutex_init(&pCifsFile->fh_mutex);
mutex_init(&pCifsFile->lock_mutex);
@@ -266,14 +265,55 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
return pCifsFile;
}
-/* Release a reference on the file private data */
+/*
+ * Release a reference on the file private data. This may involve closing
+ * the filehandle out on the server.
+ */
void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
{
- if (atomic_dec_and_test(&cifs_file->count)) {
- cifs_put_tlink(cifs_file->tlink);
- dput(cifs_file->dentry);
- kfree(cifs_file);
+ struct cifsTconInfo *tcon = tlink_tcon(cifs_file->tlink);
+ struct cifsInodeInfo *cifsi = CIFS_I(cifs_file->dentry->d_inode);
+ struct cifsLockInfo *li, *tmp;
+
+ spin_lock(&cifs_file_list_lock);
+ if (!atomic_dec_and_test(&cifs_file->count)) {
+ spin_unlock(&cifs_file_list_lock);
+ return;
+ }
+
+ /* remove it from the lists */
+ list_del(&cifs_file->flist);
+ list_del(&cifs_file->tlist);
+
+ if (list_empty(&cifsi->openFileList)) {
+ cFYI(1, "closing last open instance for inode %p",
+ cifs_file->dentry->d_inode);
+ cifsi->clientCanCacheRead = false;
+ cifsi->clientCanCacheAll = false;
+ }
+ spin_unlock(&cifs_file_list_lock);
+
+ if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
+ int xid, rc;
+
+ xid = GetXid();
+ rc = CIFSSMBClose(xid, tcon, cifs_file->netfid);
+ FreeXid(xid);
+ }
+
+ /* Delete any outstanding lock records. We'll lose them when the file
+ * is closed anyway.
+ */
+ mutex_lock(&cifs_file->lock_mutex);
+ list_for_each_entry_safe(li, tmp, &cifs_file->llist, llist) {
+ list_del(&li->llist);
+ kfree(li);
}
+ mutex_unlock(&cifs_file->lock_mutex);
+
+ cifs_put_tlink(cifs_file->tlink);
+ dput(cifs_file->dentry);
+ kfree(cifs_file);
}
int cifs_open(struct inode *inode, struct file *file)
@@ -605,79 +645,11 @@ reopen_error_exit:
int cifs_close(struct inode *inode, struct file *file)
{
- int rc = 0;
- int xid, timeout;
- struct cifs_sb_info *cifs_sb;
- struct cifsTconInfo *pTcon;
- struct cifsFileInfo *pSMBFile = file->private_data;
-
- xid = GetXid();
-
- cifs_sb = CIFS_SB(inode->i_sb);
- pTcon = tlink_tcon(pSMBFile->tlink);
- if (pSMBFile) {
- struct cifsLockInfo *li, *tmp;
- spin_lock(&cifs_file_list_lock);
- pSMBFile->closePend = true;
- if (pTcon) {
- /* no sense reconnecting to close a file that is
- already closed */
- if (!pTcon->need_reconnect) {
- spin_unlock(&cifs_file_list_lock);
- timeout = 2;
- while ((atomic_read(&pSMBFile->count) != 1)
- && (timeout <= 2048)) {
- /* Give write a better chance to get to
- server ahead of the close. We do not
- want to add a wait_q here as it would
- increase the memory utilization as
- the struct would be in each open file,
- but this should give enough time to
- clear the socket */
- cFYI(DBG2, "close delay, write pending");
- msleep(timeout);
- timeout *= 4;
- }
- if (!pTcon->need_reconnect &&
- !pSMBFile->invalidHandle)
- rc = CIFSSMBClose(xid, pTcon,
- pSMBFile->netfid);
- } else
- spin_unlock(&cifs_file_list_lock);
- } else
- spin_unlock(&cifs_file_list_lock);
-
- /* Delete any outstanding lock records.
- We'll lose them when the file is closed anyway. */
- mutex_lock(&pSMBFile->lock_mutex);
- list_for_each_entry_safe(li, tmp, &pSMBFile->llist, llist) {
- list_del(&li->llist);
- kfree(li);
- }
- mutex_unlock(&pSMBFile->lock_mutex);
+ cifsFileInfo_put(file->private_data);
+ file->private_data = NULL;
- spin_lock(&cifs_file_list_lock);
- list_del(&pSMBFile->flist);
- list_del(&pSMBFile->tlist);
- spin_unlock(&cifs_file_list_lock);
- cifsFileInfo_put(file->private_data);
- file->private_data = NULL;
- } else
- rc = -EBADF;
-
- spin_lock(&cifs_file_list_lock);
- if (list_empty(&(CIFS_I(inode)->openFileList))) {
- cFYI(1, "closing last open instance for inode %p", inode);
- /* if the file is not open we do not know if we can cache info
- on this inode, much less write behind and read ahead */
- CIFS_I(inode)->clientCanCacheRead = false;
- CIFS_I(inode)->clientCanCacheAll = false;
- }
- spin_unlock(&cifs_file_list_lock);
- if ((rc == 0) && CIFS_I(inode)->write_behind_rc)
- rc = CIFS_I(inode)->write_behind_rc;
- FreeXid(xid);
- return rc;
+ /* return code from the ->release op is always ignored */
+ return 0;
}
int cifs_closedir(struct inode *inode, struct file *file)
@@ -1024,13 +996,6 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
we blocked so return what we managed to write */
return total_written;
}
- if (open_file->closePend) {
- FreeXid(xid);
- if (total_written)
- return total_written;
- else
- return -EBADF;
- }
if (open_file->invalidHandle) {
/* we could deadlock if we called
filemap_fdatawait from here so tell
@@ -1111,13 +1076,6 @@ static ssize_t cifs_write(struct cifsFileInfo *open_file,
total_written += bytes_written) {
rc = -EAGAIN;
while (rc == -EAGAIN) {
- if (open_file->closePend) {
- FreeXid(xid);
- if (total_written)
- return total_written;
- else
- return -EBADF;
- }
if (open_file->invalidHandle) {
/* we could deadlock if we called
filemap_fdatawait from here so tell
@@ -1197,8 +1155,6 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
are always at the end of the list but since the first entry might
have a close pending, we go through the whole list */
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
- if (open_file->closePend)
- continue;
if (fsuid_only && open_file->uid != current_fsuid())
continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_READ) {
@@ -1244,8 +1200,6 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
spin_lock(&cifs_file_list_lock);
refind_writable:
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
- if (open_file->closePend)
- continue;
if (!any_available && open_file->pid != current->tgid)
continue;
if (fsuid_only && open_file->uid != current_fsuid())
@@ -1260,34 +1214,18 @@ refind_writable:
}
spin_unlock(&cifs_file_list_lock);
+
/* Had to unlock since following call can block */
rc = cifs_reopen_file(open_file, false);
- if (!rc) {
- if (!open_file->closePend)
- return open_file;
- else { /* start over in case this was deleted */
- /* since the list could be modified */
- spin_lock(&cifs_file_list_lock);
- cifsFileInfo_put(open_file);
- goto refind_writable;
- }
- }
+ if (!rc)
+ return open_file;
- /* if it fails, try another handle if possible -
- (we can not do this if closePending since
- loop could be modified - in which case we
- have to start at the beginning of the list
- again. Note that it would be bad
- to hold up writepages here (rather than
- in caller) with continuous retries */
+ /* if it fails, try another handle if possible */
cFYI(1, "wp failed on reopen file");
- spin_lock(&cifs_file_list_lock);
- /* can not use this handle, no write
- pending on this one after all */
cifsFileInfo_put(open_file);
- if (open_file->closePend) /* list could have changed */
- goto refind_writable;
+ spin_lock(&cifs_file_list_lock);
+
/* else we simply continue to the next entry. Thus
we do not loop on reopen errors. If we
can not reopen the file, for example if we
@@ -1808,8 +1746,7 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
smb_read_data = NULL;
while (rc == -EAGAIN) {
int buf_type = CIFS_NO_BUFFER;
- if ((open_file->invalidHandle) &&
- (!open_file->closePend)) {
+ if (open_file->invalidHandle) {
rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
@@ -1894,8 +1831,7 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
}
rc = -EAGAIN;
while (rc == -EAGAIN) {
- if ((open_file->invalidHandle) &&
- (!open_file->closePend)) {
+ if (open_file->invalidHandle) {
rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
@@ -2059,8 +1995,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
read_size, contig_pages);
rc = -EAGAIN;
while (rc == -EAGAIN) {
- if ((open_file->invalidHandle) &&
- (!open_file->closePend)) {
+ if (open_file->invalidHandle) {
rc = cifs_reopen_file(open_file, true);
if (rc != 0)
break;
@@ -2212,8 +2147,6 @@ static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
spin_lock(&cifs_file_list_lock);
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
- if (open_file->closePend)
- continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
spin_unlock(&cifs_file_list_lock);
return 1;
@@ -2372,7 +2305,7 @@ void cifs_oplock_break(struct work_struct *work)
* not bother sending an oplock release if session to server still is
* disconnected since oplock already released by the server
*/
- if (!cfile->closePend && !cfile->oplock_break_cancelled) {
+ if (!cfile->oplock_break_cancelled) {
rc = CIFSSMBLock(0, tlink_tcon(cfile->tlink), cfile->netfid, 0,
0, 0, 0, LOCKING_ANDX_OPLOCK_RELEASE, false);
cFYI(1, "Oplock release rc = %d", rc);
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index de6073c..b86b423 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -567,16 +567,6 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
if (pSMB->Fid != netfile->netfid)
continue;
- /*
- * don't do anything if file is about to be
- * closed anyway.
- */
- if (netfile->closePend) {
- spin_unlock(&cifs_file_list_lock);
- read_unlock(&cifs_tcp_ses_lock);
- return true;
- }
-
cFYI(1, "file id match, oplock break");
pCifsInode = CIFS_I(netfile->dentry->d_inode);
pCifsInode->clientCanCacheAll = false;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 11/13] cifs: convert cifsFileInfo->count to non-atomic counter
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (9 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 10/13] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
[not found] ` <1287171248-464-12-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-15 19:34 ` [PATCH 12/13] cifs: wait for writeback to complete in cifs_flush Jeff Layton
2010-10-15 19:34 ` [PATCH 13/13] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
The count for cifsFileInfo is currently an atomic, but that just adds
complexity for little value. We generally need to hold cifs_file_list_lock
to traverse the lists anyway so we might as well make this counter
non-atomic and simply use the cifs_file_list_lock to protect it.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsglob.h | 9 ++++++---
fs/cifs/file.c | 8 +++++---
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index df4629e..a97e0e5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -395,16 +395,19 @@ struct cifsFileInfo {
struct list_head llist; /* list of byte range locks we have. */
bool invalidHandle:1; /* file closed via session abend */
bool oplock_break_cancelled:1;
- atomic_t count; /* reference count */
+ int count; /* refcount -- protected by cifs_file_list_lock */
struct mutex fh_mutex; /* prevents reopen race after dead ses*/
struct cifs_search_info srch_inf;
struct work_struct oplock_break; /* work for oplock breaks */
};
-/* Take a reference on the file private data */
+/*
+ * Take a reference on the file private data. Must be called with
+ * cifs_file_list_lock held.
+ */
static inline void cifsFileInfo_get(struct cifsFileInfo *cifs_file)
{
- atomic_inc(&cifs_file->count);
+ ++cifs_file->count;
}
void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8c81e7b..baf4b50 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -232,6 +232,7 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
if (pCifsFile == NULL)
return pCifsFile;
+ pCifsFile->count = 1;
pCifsFile->netfid = fileHandle;
pCifsFile->pid = current->tgid;
pCifsFile->uid = current_fsuid();
@@ -242,7 +243,6 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
mutex_init(&pCifsFile->fh_mutex);
mutex_init(&pCifsFile->lock_mutex);
INIT_LIST_HEAD(&pCifsFile->llist);
- atomic_set(&pCifsFile->count, 1);
INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
spin_lock(&cifs_file_list_lock);
@@ -267,7 +267,8 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
/*
* Release a reference on the file private data. This may involve closing
- * the filehandle out on the server.
+ * the filehandle out on the server. Must be called without holding
+ * cifs_file_list_lock.
*/
void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
{
@@ -276,7 +277,7 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
struct cifsLockInfo *li, *tmp;
spin_lock(&cifs_file_list_lock);
- if (!atomic_dec_and_test(&cifs_file->count)) {
+ if (--cifs_file->count > 0) {
spin_unlock(&cifs_file_list_lock);
return;
}
@@ -2322,6 +2323,7 @@ void cifs_oplock_break(struct work_struct *work)
cifs_oplock_break_put(cfile);
}
+/* must be called while holding cifs_file_list_lock */
void cifs_oplock_break_get(struct cifsFileInfo *cfile)
{
cifs_sb_active(cfile->dentry->d_sb);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 12/13] cifs: wait for writeback to complete in cifs_flush
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (10 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 11/13] cifs: convert cifsFileInfo->count to non-atomic counter Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
2010-10-15 19:34 ` [PATCH 13/13] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
12 siblings, 0 replies; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
The f_op->flush operation is the last chance to return a writeback
related error when closing a file. Ensure that we don't miss reporting
any errors by waiting for writeback to complete in cifs_flush before
proceeding.
There's no reason to do this when the file isn't open for write
however.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/file.c | 21 +++++++--------------
1 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index baf4b50..128e07b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1688,20 +1688,13 @@ int cifs_flush(struct file *file, fl_owner_t id)
struct inode *inode = file->f_path.dentry->d_inode;
int rc = 0;
- /* Rather than do the steps manually:
- lock the inode for writing
- loop through pages looking for write behind data (dirty pages)
- coalesce into contiguous 16K (or smaller) chunks to write to server
- send to server (prefer in parallel)
- deal with writebehind errors
- unlock inode for writing
- filemapfdatawrite appears easier for the time being */
-
- rc = filemap_fdatawrite(inode->i_mapping);
- /* reset wb rc if we were able to write out dirty pages */
- if (!rc) {
- rc = CIFS_I(inode)->write_behind_rc;
- CIFS_I(inode)->write_behind_rc = 0;
+ if (file->f_mode & FMODE_WRITE) {
+ rc = filemap_write_and_wait(inode->i_mapping);
+ /* reset wb rc if we were able to write out dirty pages */
+ if (!rc) {
+ rc = CIFS_I(inode)->write_behind_rc;
+ CIFS_I(inode)->write_behind_rc = 0;
+ }
}
cFYI(1, "Flush inode %p file %p rc %d", inode, file, rc);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 13/13] cifs: eliminate cifsInodeInfo->write_behind_rc
[not found] ` <1287171248-464-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (11 preceding siblings ...)
2010-10-15 19:34 ` [PATCH 12/13] cifs: wait for writeback to complete in cifs_flush Jeff Layton
@ 2010-10-15 19:34 ` Jeff Layton
[not found] ` <1287171248-464-14-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
12 siblings, 1 reply; 23+ messages in thread
From: Jeff Layton @ 2010-10-15 19:34 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
write_behind_rc is redundant and just adds complexity to the code. What
we really want to do instead is to use mapping_set_error to reset the
flags on the mapping when we find a writeback error and can't report it
to userspace yet.
For cifs_flush and cifs_fsync, we shouldn't reset the flags since errors
returned there do get reported to userspace.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/cifsfs.c | 1 -
fs/cifs/cifsglob.h | 1 -
fs/cifs/file.c | 34 ++++++++--------------------------
fs/cifs/inode.c | 15 +++++----------
4 files changed, 13 insertions(+), 38 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index c25e928..6e3e2e8 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -323,7 +323,6 @@ cifs_alloc_inode(struct super_block *sb)
return NULL;
cifs_inode->cifsAttrs = 0x20; /* default */
cifs_inode->time = 0;
- cifs_inode->write_behind_rc = 0;
/* Until the file is open and we have gotten oplock
info back from the server, can not assume caching of
file data or metadata */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a97e0e5..89c3de0 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -420,7 +420,6 @@ struct cifsInodeInfo {
struct list_head lockList;
/* BB add in lists for dirty pages i.e. write caching info for oplock */
struct list_head openFileList;
- int write_behind_rc;
__u32 cifsAttrs; /* e.g. DOS archive bit, sparse, compressed, system */
unsigned long time; /* jiffies of last update/check of inode */
bool clientCanCacheRead:1; /* read oplock */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 128e07b..3a97055 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -131,8 +131,7 @@ static inline int cifs_open_inode_helper(struct inode *inode,
/* BB no need to lock inode until after invalidate
since namei code should already have it locked? */
rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0)
- pCifsInode->write_behind_rc = rc;
+ mapping_set_error(inode->i_mapping, rc);
}
cFYI(1, "invalidating remote inode since open detected it "
"changed");
@@ -606,8 +605,7 @@ reopen_success:
if (can_flush) {
rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0)
- CIFS_I(inode)->write_behind_rc = rc;
+ mapping_set_error(inode->i_mapping, rc);
pCifsInode->clientCanCacheAll = false;
pCifsInode->clientCanCacheRead = false;
@@ -1489,12 +1487,7 @@ retry:
if (rc || bytes_written < bytes_to_write) {
cERROR(1, "Write2 ret %d, wrote %d",
rc, bytes_written);
- /* BB what if continued retry is
- requested via mount flags? */
- if (rc == -ENOSPC)
- set_bit(AS_ENOSPC, &mapping->flags);
- else
- set_bit(AS_EIO, &mapping->flags);
+ mapping_set_error(mapping, rc);
} else {
cifs_stats_bytes_written(tcon, bytes_written);
}
@@ -1639,10 +1632,8 @@ int cifs_fsync(struct file *file, int datasync)
rc = filemap_write_and_wait(inode->i_mapping);
if (rc == 0) {
- rc = CIFS_I(inode)->write_behind_rc;
- CIFS_I(inode)->write_behind_rc = 0;
tcon = tlink_tcon(smbfile->tlink);
- if (!rc && tcon && smbfile &&
+ if (rc == 0 &&
!(CIFS_SB(inode->i_sb)->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
}
@@ -1688,14 +1679,8 @@ int cifs_flush(struct file *file, fl_owner_t id)
struct inode *inode = file->f_path.dentry->d_inode;
int rc = 0;
- if (file->f_mode & FMODE_WRITE) {
+ if (file->f_mode & FMODE_WRITE)
rc = filemap_write_and_wait(inode->i_mapping);
- /* reset wb rc if we were able to write out dirty pages */
- if (!rc) {
- rc = CIFS_I(inode)->write_behind_rc;
- CIFS_I(inode)->write_behind_rc = 0;
- }
- }
cFYI(1, "Flush inode %p file %p rc %d", inode, file, rc);
@@ -2274,7 +2259,7 @@ void cifs_oplock_break(struct work_struct *work)
oplock_break);
struct inode *inode = cfile->dentry->d_inode;
struct cifsInodeInfo *cinode = CIFS_I(inode);
- int rc, waitrc = 0;
+ int rc = 0;
if (inode && S_ISREG(inode->i_mode)) {
if (cinode->clientCanCacheRead)
@@ -2283,13 +2268,10 @@ void cifs_oplock_break(struct work_struct *work)
break_lease(inode, O_WRONLY);
rc = filemap_fdatawrite(inode->i_mapping);
if (cinode->clientCanCacheRead == 0) {
- waitrc = filemap_fdatawait(inode->i_mapping);
+ rc = filemap_fdatawait(inode->i_mapping);
+ mapping_set_error(inode->i_mapping, rc);
invalidate_remote_inode(inode);
}
- if (!rc)
- rc = waitrc;
- if (rc)
- cinode->write_behind_rc = rc;
cFYI(1, "Oplock flush inode %p rc %d", inode, rc);
}
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index cfe2339..ed5558b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1688,8 +1688,7 @@ cifs_invalidate_mapping(struct inode *inode)
/* write back any cached data */
if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
rc = filemap_write_and_wait(inode->i_mapping);
- if (rc)
- cifs_i->write_behind_rc = rc;
+ mapping_set_error(inode->i_mapping, rc);
}
invalidate_remote_inode(inode);
cifs_fscache_reset_inode_cookie(inode);
@@ -1949,10 +1948,8 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0) {
- cifsInode->write_behind_rc = rc;
- rc = 0;
- }
+ mapping_set_error(inode->i_mapping, rc);
+ rc = 0;
if (attrs->ia_valid & ATTR_SIZE) {
rc = cifs_set_file_size(inode, attrs, xid, full_path);
@@ -2093,10 +2090,8 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
- if (rc != 0) {
- cifsInode->write_behind_rc = rc;
- rc = 0;
- }
+ mapping_set_error(inode->i_mapping, rc);
+ rc = 0;
if (attrs->ia_valid & ATTR_SIZE) {
rc = cifs_set_file_size(inode, attrs, xid, full_path);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 23+ messages in thread