* [PATCH 01/14] cifs: keep dentry reference in cifsFileInfo instead of inode reference
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 02/14] cifs: don't use vfsmount to pin superblock for oplock breaks Jeff Layton
` (14 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
cifsFileInfo is a bit problematic. It contains a reference back to the
struct file itself. This makes it difficult for a cifsFileInfo to exist
without a corresponding struct file.
It would be better instead of the cifsFileInfo just held info pertaining
to the open file on the server instead without any back refrences to the
struct file. This would allow it to exist after the filp to which it was
originally attached was closed.
Much of the use of the file pointer in this struct is to get at the
dentry. Begin divorcing the cifsFileInfo from the struct file by
keeping a reference to the dentry. Since the dentry will have a
reference to the inode, we can eliminate the "pInode" field too and
convert the igrab/iput to dget/dput.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsglob.h | 4 ++--
fs/cifs/dir.c | 3 ++-
fs/cifs/file.c | 2 +-
fs/cifs/misc.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 3c955ee..bbb6552 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -387,7 +387,7 @@ struct cifsFileInfo {
/* BB add lock scope info here if needed */ ;
/* lock scope id (0 if none) */
struct file *pfile; /* needed for writepage */
- struct inode *pInode; /* needed for oplock break */
+ struct dentry *dentry;
struct vfsmount *mnt;
struct tcon_link *tlink;
struct mutex lock_mutex;
@@ -412,7 +412,7 @@ static inline void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
{
if (atomic_dec_and_test(&cifs_file->count)) {
cifs_put_tlink(cifs_file->tlink);
- iput(cifs_file->pInode);
+ dput(cifs_file->dentry);
kfree(cifs_file);
}
}
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index e249b56..6887c41 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -135,6 +135,7 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
struct vfsmount *mnt, struct tcon_link *tlink,
unsigned int oflags, __u32 oplock)
{
+ struct dentry *dentry = file->f_path.dentry;
struct cifsFileInfo *pCifsFile;
struct cifsInodeInfo *pCifsInode;
@@ -145,7 +146,7 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
pCifsFile->netfid = fileHandle;
pCifsFile->pid = current->tgid;
pCifsFile->uid = current_fsuid();
- pCifsFile->pInode = igrab(newinode);
+ pCifsFile->dentry = dget(dentry);
pCifsFile->mnt = mnt;
pCifsFile->pfile = file;
pCifsFile->invalidHandle = false;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 80856f1..c302b9c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2335,7 +2335,7 @@ void cifs_oplock_break(struct work_struct *work)
{
struct cifsFileInfo *cfile = container_of(work, struct cifsFileInfo,
oplock_break);
- struct inode *inode = cfile->pInode;
+ struct inode *inode = cfile->dentry->d_inode;
struct cifsInodeInfo *cinode = CIFS_I(inode);
int rc, waitrc = 0;
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 252f276..9bac3e7 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -578,7 +578,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
}
cFYI(1, "file id match, oplock break");
- pCifsInode = CIFS_I(netfile->pInode);
+ pCifsInode = CIFS_I(netfile->dentry->d_inode);
pCifsInode->clientCanCacheAll = false;
if (pSMB->OplockLevel == 0)
pCifsInode->clientCanCacheRead = false;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/14] cifs: don't use vfsmount to pin superblock for oplock breaks
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-04 17:52 ` [PATCH 01/14] cifs: keep dentry reference in cifsFileInfo instead of inode reference Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 03/14] cifs: eliminate cifs_posix_open_inode_helper Jeff Layton
` (13 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Filesystems aren't really supposed to do anything with a vfsmount. It's
considered a layering violation since vfsmounts are entirely managed at
the VFS layer.
CIFS currently keeps an active reference to a vfsmount in order to
prevent the superblock vanishing before an oplock break has completed.
What we really want to do instead is to keep sb->s_active high until the
oplock break has completed. This patch borrows the scheme that NFS uses
for handling sillyrenames.
An atomic_t is added to the cifs_sb_info. When it transitions from 0 to
1, an extra reference to the superblock is taken (by bumping the
s_active value). When it transitions from 1 to 0, that reference is
dropped and a the superblock teardown may proceed if there are no more
references to it.
Also, the vfsmount pointer is removed from cifsFileInfo and from
cifs_new_fileinfo, and some bogus forward declarations are removed from
cifsfs.h.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifs_fs_sb.h | 1 +
fs/cifs/cifsfs.c | 18 ++++++++++++++++++
fs/cifs/cifsfs.h | 6 ++----
fs/cifs/cifsglob.h | 1 -
fs/cifs/cifsproto.h | 2 +-
fs/cifs/dir.c | 10 +++-------
fs/cifs/file.c | 9 ++++-----
7 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 9ffe484..f68de99 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -48,6 +48,7 @@ struct cifs_sb_info {
struct nls_table *local_nls;
unsigned int rsize;
unsigned int wsize;
+ atomic_t active;
uid_t mnt_uid;
gid_t mnt_gid;
mode_t mnt_file_mode;
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 906b098..241f3d0 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -83,6 +83,24 @@ extern mempool_t *cifs_sm_req_poolp;
extern mempool_t *cifs_req_poolp;
extern mempool_t *cifs_mid_poolp;
+void
+cifs_sb_active(struct super_block *sb)
+{
+ struct cifs_sb_info *server = CIFS_SB(sb);
+
+ if (atomic_inc_return(&server->active) == 1)
+ atomic_inc(&sb->s_active);
+}
+
+void
+cifs_sb_deactive(struct super_block *sb)
+{
+ struct cifs_sb_info *server = CIFS_SB(sb);
+
+ if (atomic_dec_and_test(&server->active))
+ deactivate_super(sb);
+}
+
static int
cifs_read_super(struct super_block *sb, void *data,
const char *devname, int silent)
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index d82f5fb..2fb3829 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -42,10 +42,8 @@ extern const struct address_space_operations cifs_addr_ops;
extern const struct address_space_operations cifs_addr_ops_smallbuf;
/* Functions related to super block operations */
-/* extern const struct super_operations cifs_super_ops;*/
-extern void cifs_read_inode(struct inode *);
-/*extern void cifs_delete_inode(struct inode *);*/ /* BB not needed yet */
-/* extern void cifs_write_inode(struct inode *); */ /* BB not needed yet */
+extern void cifs_sb_active(struct super_block *sb);
+extern void cifs_sb_deactive(struct super_block *sb);
/* Functions related to inodes */
extern const struct inode_operations cifs_dir_inode_ops;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index bbb6552..a019820 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -388,7 +388,6 @@ struct cifsFileInfo {
/* lock scope id (0 if none) */
struct file *pfile; /* needed for writepage */
struct dentry *dentry;
- struct vfsmount *mnt;
struct tcon_link *tlink;
struct mutex lock_mutex;
struct list_head llist; /* list of byte range locks we have. */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 29a2ee8..7f416ab 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -107,7 +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 vfsmount *mnt, struct tcon_link *tlink,
+ struct tcon_link *tlink,
unsigned int oflags, __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 6887c41..c205ec9 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -132,8 +132,7 @@ cifs_bp_rename_retry:
struct cifsFileInfo *
cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
- struct vfsmount *mnt, struct tcon_link *tlink,
- unsigned int oflags, __u32 oplock)
+ struct tcon_link *tlink, unsigned int oflags, __u32 oplock)
{
struct dentry *dentry = file->f_path.dentry;
struct cifsFileInfo *pCifsFile;
@@ -147,7 +146,6 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
pCifsFile->pid = current->tgid;
pCifsFile->uid = current_fsuid();
pCifsFile->dentry = dget(dentry);
- pCifsFile->mnt = mnt;
pCifsFile->pfile = file;
pCifsFile->invalidHandle = false;
pCifsFile->closePend = false;
@@ -485,8 +483,7 @@ cifs_create_set_dentry:
}
pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp,
- nd->path.mnt, tlink, oflags,
- oplock);
+ tlink, oflags, oplock);
if (pfile_info == NULL) {
fput(filp);
CIFSSMBClose(xid, tcon, fileHandle);
@@ -760,8 +757,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
}
cfile = cifs_new_fileinfo(newInode, fileHandle, filp,
- nd->path.mnt, tlink,
- nd->intent.open.flags,
+ tlink, nd->intent.open.flags,
oplock);
if (cfile == NULL) {
fput(filp);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c302b9c..84d1435 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -282,7 +282,6 @@ int cifs_open(struct inode *inode, struct file *file)
}
pCifsFile = cifs_new_fileinfo(inode, netfid, file,
- file->f_path.mnt,
tlink, oflags, oplock);
if (pCifsFile == NULL) {
CIFSSMBClose(xid, tcon, netfid);
@@ -375,8 +374,8 @@ int cifs_open(struct inode *inode, struct file *file)
if (rc != 0)
goto out;
- pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt,
- tlink, file->f_flags, oplock);
+ pCifsFile = cifs_new_fileinfo(inode, netfid, file, tlink,
+ file->f_flags, oplock);
if (pCifsFile == NULL) {
rc = -ENOMEM;
goto out;
@@ -2381,13 +2380,13 @@ void cifs_oplock_break(struct work_struct *work)
void cifs_oplock_break_get(struct cifsFileInfo *cfile)
{
- mntget(cfile->mnt);
cifsFileInfo_get(cfile);
+ cifs_sb_active(cfile->dentry->d_sb);
}
void cifs_oplock_break_put(struct cifsFileInfo *cfile)
{
- mntput(cfile->mnt);
+ cifs_sb_deactive(cfile->dentry->d_sb);
cifsFileInfo_put(cfile);
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 03/14] cifs: eliminate cifs_posix_open_inode_helper
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-04 17:52 ` [PATCH 01/14] cifs: keep dentry reference in cifsFileInfo instead of inode reference Jeff Layton
2010-10-04 17:52 ` [PATCH 02/14] cifs: don't use vfsmount to pin superblock for oplock breaks Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 04/14] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
` (12 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
This function is redundant. The only thing it does is set the canCache
flags, but those get set in cifs_new_fileinfo anyway.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/file.c | 67 --------------------------------------------------------
1 files changed, 0 insertions(+), 67 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 84d1435..944983e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -106,64 +106,6 @@ static inline int cifs_get_disposition(unsigned int flags)
return FILE_OPEN;
}
-/* all arguments to this function must be checked for validity in caller */
-static inline int
-cifs_posix_open_inode_helper(struct inode *inode, struct file *file,
- struct cifsInodeInfo *pCifsInode, __u32 oplock,
- u16 netfid)
-{
-
- write_lock(&GlobalSMBSeslock);
-
- pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
- if (pCifsInode == NULL) {
- write_unlock(&GlobalSMBSeslock);
- return -EINVAL;
- }
-
- if (pCifsInode->clientCanCacheRead) {
- /* we have the inode open somewhere else
- no need to discard cache data */
- goto psx_client_can_cache;
- }
-
- /* BB FIXME need to fix this check to move it earlier into posix_open
- BB fIX following section BB FIXME */
-
- /* if not oplocked, invalidate inode pages if mtime or file
- size changed */
-/* temp = cifs_NTtimeToUnix(le64_to_cpu(buf->LastWriteTime));
- if (timespec_equal(&file->f_path.dentry->d_inode->i_mtime, &temp) &&
- (file->f_path.dentry->d_inode->i_size ==
- (loff_t)le64_to_cpu(buf->EndOfFile))) {
- cFYI(1, "inode unchanged on server");
- } else {
- if (file->f_path.dentry->d_inode->i_mapping) {
- rc = filemap_write_and_wait(file->f_path.dentry->d_inode->i_mapping);
- if (rc != 0)
- CIFS_I(file->f_path.dentry->d_inode)->write_behind_rc = rc;
- }
- cFYI(1, "invalidating remote inode since open detected it "
- "changed");
- invalidate_remote_inode(file->f_path.dentry->d_inode);
- } */
-
-psx_client_can_cache:
- 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;
-
- /* will have to change the unlock if we reenable the
- filemap_fdatawrite (which does not seem necessary */
- write_unlock(&GlobalSMBSeslock);
- return 0;
-}
-
-/* all arguments to this function must be checked for validity in caller */
static inline int cifs_open_inode_helper(struct inode *inode,
struct cifsTconInfo *pTcon, __u32 oplock, FILE_ALL_INFO *buf,
char *full_path, int xid)
@@ -271,15 +213,6 @@ int cifs_open(struct inode *inode, struct file *file)
oflags, &oplock, &netfid, xid);
if (rc == 0) {
cFYI(1, "posix open succeeded");
- /* no need for special case handling of setting mode
- on read only files needed here */
-
- rc = cifs_posix_open_inode_helper(inode, file,
- pCifsInode, oplock, netfid);
- if (rc != 0) {
- CIFSSMBClose(xid, tcon, netfid);
- goto out;
- }
pCifsFile = cifs_new_fileinfo(inode, netfid, file,
tlink, oflags, oplock);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 04/14] cifs: eliminate oflags option from cifs_new_fileinfo
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 03/14] cifs: eliminate cifs_posix_open_inode_helper Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 05/14] cifs: eliminate the inode argument " Jeff Layton
` (11 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Eliminate the poor, misunderstood "oflags" option from cifs_new_fileinfo.
The callers mostly pass in the filp->f_flags here, except for one place
that passes in the flags after they've been converted to their SMB_O_*
equivalents.
None of these are 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 we really want here is the file->f_mode. Since we pass in a file
pointer already anyway, just use the f_mode there for this purpose.
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 7f416ab..bed004c 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, int oflags,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c205ec9..b399e5b 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,
@@ -483,7 +483,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);
@@ -757,8 +757,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 944983e..08b0e25 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -215,7 +215,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;
@@ -307,8 +307,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] 19+ messages in thread* [PATCH 05/14] cifs: eliminate the inode argument from cifs_new_fileinfo
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 04/14] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 06/14] cifs: clean up cifs_reopen_file Jeff Layton
` (10 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsproto.h | 3 +--
fs/cifs/dir.c | 42 ++++++++++++++++++++----------------------
fs/cifs/file.c | 6 +++---
3 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index bed004c..a2bdfa2 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 b399e5b..c53ff95 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,22 +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);
+
+ 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;
+
write_unlock(&GlobalSMBSeslock);
file->private_data = pCifsFile;
@@ -482,8 +481,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);
@@ -756,8 +754,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 08b0e25..c2077a2 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -214,8 +214,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;
@@ -307,7 +307,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] 19+ messages in thread* [PATCH 06/14] cifs: clean up cifs_reopen_file
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 05/14] cifs: eliminate the inode argument " Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 07/14] cifs: cifs_write argument change and cleanup Jeff Layton
` (9 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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>
---
fs/cifs/cifsglob.h | 1 +
fs/cifs/dir.c | 1 +
fs/cifs/file.c | 128 ++++++++++++++++++++++-----------------------------
3 files changed, 57 insertions(+), 73 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a019820..d0ced6e 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -388,6 +388,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 c53ff95..fc872c4 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 c2077a2..0056405c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -354,14 +354,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;
@@ -369,11 +368,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) {
@@ -383,21 +377,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);
@@ -405,17 +385,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;
@@ -425,7 +404,7 @@ 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);
+ int oflags = (int) cifs_posix_convert_flags(pCifsFile->f_flags);
/* can not refresh inode info since size could be stale */
rc = cifs_posix_open(full_path, NULL, inode->i_sb,
cifs_sb->mnt_file_mode /* ignored */,
@@ -438,7 +417,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
@@ -454,49 +433,52 @@ 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 (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",
+ 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;
@@ -935,7 +917,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;
}
@@ -1033,7 +1015,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;
}
@@ -1181,7 +1163,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;
@@ -1729,7 +1711,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;
}
@@ -1815,7 +1797,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;
}
@@ -1980,7 +1962,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] 19+ messages in thread* [PATCH 07/14] cifs: cifs_write argument change and cleanup
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 06/14] cifs: clean up cifs_reopen_file Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 08/14] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
` (8 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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>
---
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 0056405c..a1817ed 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -965,8 +965,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;
@@ -974,17 +975,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();
@@ -994,15 +992,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)
@@ -1062,20 +1051,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;
}
@@ -1246,8 +1228,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);
@@ -1562,7 +1544,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] 19+ messages in thread* [PATCH 08/14] cifs: eliminate pfile pointer from cifsFileInfo
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (6 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 07/14] cifs: cifs_write argument change and cleanup Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 09/14] cifs: move cifs_new_fileinfo to file.c Jeff Layton
` (7 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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>
---
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 d0ced6e..7e2503f 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -386,7 +386,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 fc872c4..5dc8eed 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 a1817ed..f63072c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1082,8 +1082,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 */
@@ -1132,9 +1131,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) {
@@ -2098,9 +2095,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] 19+ messages in thread* [PATCH 09/14] cifs: move cifs_new_fileinfo to file.c
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (7 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 08/14] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 10/14] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
` (6 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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>
---
fs/cifs/dir.c | 50 --------------------------------------------------
fs/cifs/file.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 5dc8eed..983c483 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -130,56 +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);
-
- 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;
-
- write_unlock(&GlobalSMBSeslock);
-
- file->private_data = pCifsFile;
-
- 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)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index f63072c..2804528 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -159,6 +159,54 @@ client_can_cache:
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);
+
+ 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;
+
+ write_unlock(&GlobalSMBSeslock);
+ 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] 19+ messages in thread* [PATCH 10/14] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (8 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 09/14] cifs: move cifs_new_fileinfo to file.c Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
[not found] ` <1286214781-626-11-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-04 17:52 ` [PATCH 11/14] cifs: move cifsFileInfo_put to file.c Jeff Layton
` (5 subsequent siblings)
15 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
To prevent races we need to be able to do an atomic_dec_and_lock with
it, and that won't work with a rwlock.
While we're at it, change it to have a more descriptive name without
camel-case.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@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 241f3d0..6e3a69a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -939,8 +939,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 7e2503f..5dc988c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -720,7 +720,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 13c854e..9ca6933 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 2804528..c2f8fc5 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -186,7 +186,7 @@ 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*/
@@ -202,7 +202,7 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
} else if ((oplock & 0xF) == OPLOCK_READ)
pCifsInode->clientCanCacheRead = true;
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
file->private_data = pCifsFile;
return pCifsFile;
}
@@ -546,13 +546,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)) {
@@ -572,9 +572,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. */
@@ -585,16 +585,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
@@ -602,7 +602,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);
@@ -624,18 +624,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");
@@ -1121,7 +1121,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 */
@@ -1135,7 +1135,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
@@ -1143,7 +1143,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
@@ -1170,7 +1170,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)
@@ -1184,11 +1184,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) {
@@ -1196,7 +1196,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;
}
@@ -1210,7 +1210,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);
@@ -1231,7 +1231,7 @@ refind_writable:
any_available = true;
goto refind_writable;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return NULL;
}
@@ -2139,16 +2139,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;
}
@@ -2312,8 +2312,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 078c625..dbc1c64 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] 19+ messages in thread* [PATCH 11/14] cifs: move cifsFileInfo_put to file.c
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (9 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 10/14] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:52 ` [PATCH 12/14] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
` (4 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: 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>
---
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 5dc988c..522cefa 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -406,15 +406,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 c2f8fc5..c7a3e40 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -207,6 +207,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] 19+ messages in thread* [PATCH 12/14] cifs: move close processing from cifs_close to cifsFileInfo_put
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (10 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 11/14] cifs: move cifsFileInfo_put to file.c Jeff Layton
@ 2010-10-04 17:52 ` Jeff Layton
2010-10-04 17:53 ` [PATCH 13/14] cifs: wait for writeback to complete in cifs_flush Jeff Layton
` (3 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:52 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Now that it's possible 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>
---
fs/cifs/cifsglob.h | 1 -
fs/cifs/file.c | 189 +++++++++++++++++-----------------------------------
fs/cifs/misc.c | 10 ---
3 files changed, 62 insertions(+), 138 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 522cefa..06024c5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -391,7 +391,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 c7a3e40..00c7262 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -178,7 +178,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);
@@ -207,14 +206,58 @@ 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;
+
+ if (!atomic_dec_and_lock(&cifs_file->count, &cifs_file_list_lock))
+ return;
+
+ /* count can be incremented inside the lock, so must check again */
+ if (atomic_read(&cifs_file->count) > 0) {
+ 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)
@@ -544,79 +587,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);
+ cifsFileInfo_put(file->private_data);
+ file->private_data = NULL;
- /* 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);
-
- 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)
@@ -963,13 +938,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
@@ -1050,13 +1018,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
@@ -1136,8 +1097,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) {
@@ -1183,8 +1142,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())
@@ -1201,32 +1158,15 @@ 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
@@ -1747,8 +1687,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;
@@ -1833,8 +1772,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;
@@ -1998,8 +1936,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;
@@ -2151,8 +2088,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;
@@ -2311,7 +2246,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] 19+ messages in thread* [PATCH 13/14] cifs: wait for writeback to complete in cifs_flush
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (11 preceding siblings ...)
2010-10-04 17:52 ` [PATCH 12/14] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
@ 2010-10-04 17:53 ` Jeff Layton
2010-10-04 17:53 ` [PATCH 14/14] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
` (2 subsequent siblings)
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:53 UTC (permalink / raw)
To: 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.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/file.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 00c7262..4298c19 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1628,16 +1628,7 @@ 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);
+ 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;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 14/14] cifs: eliminate cifsInodeInfo->write_behind_rc
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (12 preceding siblings ...)
2010-10-04 17:53 ` [PATCH 13/14] cifs: wait for writeback to complete in cifs_flush Jeff Layton
@ 2010-10-04 17:53 ` Jeff Layton
2010-10-04 20:31 ` [PATCH 00/14] cifs: clean up management of open filehandles Dave Kleikamp
2010-10-05 10:30 ` Suresh Jayaraman
15 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-10-04 17:53 UTC (permalink / raw)
To: 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>
---
fs/cifs/cifsfs.c | 1 -
fs/cifs/cifsglob.h | 1 -
fs/cifs/file.c | 31 +++++++------------------------
fs/cifs/inode.c | 15 +++++----------
4 files changed, 12 insertions(+), 36 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 6e3a69a..8ec650e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -320,7 +320,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 06024c5..08187ca 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -415,7 +415,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 4298c19..4425a43 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -133,8 +133,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");
@@ -545,8 +544,7 @@ reopen_success:
if (pCifsInode) {
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);
/* temporarily disable caching while we
go to server to get inode info */
pCifsInode->clientCanCacheAll = false;
@@ -1429,12 +1427,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);
}
@@ -1579,10 +1572,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);
}
@@ -1629,11 +1620,6 @@ int cifs_flush(struct file *file, fl_owner_t id)
int rc = 0;
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);
@@ -2212,7 +2198,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)
@@ -2221,13 +2207,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 d6db805..c6aa014 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);
@@ -1941,10 +1940,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);
@@ -2085,10 +2082,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] 19+ messages in thread* Re: [PATCH 00/14] cifs: clean up management of open filehandles
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (13 preceding siblings ...)
2010-10-04 17:53 ` [PATCH 14/14] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
@ 2010-10-04 20:31 ` Dave Kleikamp
2010-10-05 10:30 ` Suresh Jayaraman
15 siblings, 0 replies; 19+ messages in thread
From: Dave Kleikamp @ 2010-10-04 20:31 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Mon, 2010-10-04 at 13:52 -0400, Jeff Layton wrote:
> This patchset fixes a number of problems with the existing CIFS code. It
> eliminates the backreference to the filp from cifsFileInfo, allowing a
> cifsFileInfo to outlive the filp that it was generated against.
>
> With that change, most of the closing of a filehandle on the server is
> moved to cifsFileInfo_put. cifs_close is then changed to just put the
> filehandle instead of trying to wait around for existing users to
> finish with it.
>
> With this change, there is some further cleanup that can be done as
> well. For instance, there's no real need to continually search for new
> filehandles in cifs_writepages now that we hold a reference to one. I'll
> hold back on that until I know how this set has been received.
>
> This set is an ordered set and should not be committed out of order. It
> should be bisectable. I've tested the resulting set and it seems to work
> fine.
These all look good to me. Cleans up the code rather nicely.
Shaggy
> Jeff Layton (14):
> cifs: keep dentry reference in cifsFileInfo instead of inode
> reference
> cifs: don't use vfsmount to pin superblock for oplock breaks
> cifs: eliminate cifs_posix_open_inode_helper
> cifs: eliminate oflags option from cifs_new_fileinfo
> cifs: eliminate the inode argument from cifs_new_fileinfo
> cifs: clean up cifs_reopen_file
> cifs: cifs_write argument change and cleanup
> cifs: eliminate pfile pointer from cifsFileInfo
> cifs: move cifs_new_fileinfo to file.c
> cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock
> cifs: move cifsFileInfo_put to file.c
> cifs: move close processing from cifs_close to cifsFileInfo_put
> cifs: wait for writeback to complete in cifs_flush
> cifs: eliminate cifsInodeInfo->write_behind_rc
>
> fs/cifs/cifs_fs_sb.h | 1 +
> fs/cifs/cifsfs.c | 21 ++-
> fs/cifs/cifsfs.h | 6 +-
> fs/cifs/cifsglob.h | 19 +--
> fs/cifs/cifsproto.h | 6 +-
> fs/cifs/cifssmb.c | 4 +-
> fs/cifs/dir.c | 60 +-----
> fs/cifs/file.c | 574 +++++++++++++++++++-------------------------------
> fs/cifs/inode.c | 15 +-
> fs/cifs/misc.c | 18 +--
> fs/cifs/readdir.c | 6 +-
> 11 files changed, 260 insertions(+), 470 deletions(-)
>
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 00/14] cifs: clean up management of open filehandles
[not found] ` <1286214781-626-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (14 preceding siblings ...)
2010-10-04 20:31 ` [PATCH 00/14] cifs: clean up management of open filehandles Dave Kleikamp
@ 2010-10-05 10:30 ` Suresh Jayaraman
15 siblings, 0 replies; 19+ messages in thread
From: Suresh Jayaraman @ 2010-10-05 10:30 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On 10/04/2010 11:22 PM, Jeff Layton wrote:
> This patchset fixes a number of problems with the existing CIFS code. It
> eliminates the backreference to the filp from cifsFileInfo, allowing a
> cifsFileInfo to outlive the filp that it was generated against.
>
> With that change, most of the closing of a filehandle on the server is
> moved to cifsFileInfo_put. cifs_close is then changed to just put the
> filehandle instead of trying to wait around for existing users to
> finish with it.
>
> With this change, there is some further cleanup that can be done as
> well. For instance, there's no real need to continually search for new
> filehandles in cifs_writepages now that we hold a reference to one. I'll
> hold back on that until I know how this set has been received.
>
Nice cleanups. Patches 1 to 9 looks correct to me.
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
^ permalink raw reply [flat|nested] 19+ messages in thread