* [PATCH 01/15] cifs: keep dentry reference in cifsFileInfo instead of inode reference
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-08 17:30 ` Jeff Layton
2010-10-08 17:30 ` [PATCH 02/15] cifs: don't use vfsmount to pin superblock for oplock breaks Jeff Layton
` (13 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: 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>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@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 eef5df1..8fa4ccf 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] 25+ messages in thread* [PATCH 02/15] cifs: don't use vfsmount to pin superblock for oplock breaks
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-08 17:30 ` [PATCH 01/15] cifs: keep dentry reference in cifsFileInfo instead of inode reference Jeff Layton
@ 2010-10-08 17:30 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 03/15] cifs: eliminate cifs_posix_open_inode_helper Jeff Layton
` (12 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: 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>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@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 586ee3d..525ba59 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 1aca2d7..3facf25 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 786bdf3..85544bc 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 8fa4ccf..1e24ddc 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..fd78a35 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,14 +2380,14 @@ void cifs_oplock_break(struct work_struct *work)
void cifs_oplock_break_get(struct cifsFileInfo *cfile)
{
- mntget(cfile->mnt);
+ cifs_sb_active(cfile->dentry->d_sb);
cifsFileInfo_get(cfile);
}
void cifs_oplock_break_put(struct cifsFileInfo *cfile)
{
- mntput(cfile->mnt);
cifsFileInfo_put(cfile);
+ cifs_sb_deactive(cfile->dentry->d_sb);
}
const struct address_space_operations cifs_addr_ops = {
--
1.7.2.3
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 03/15] cifs: eliminate cifs_posix_open_inode_helper
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-08 17:30 ` [PATCH 01/15] cifs: keep dentry reference in cifsFileInfo instead of inode reference Jeff Layton
2010-10-08 17:30 ` [PATCH 02/15] cifs: don't use vfsmount to pin superblock for oplock breaks Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 04/15] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
` (11 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: 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>
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Acked-by: Dave Kleikamp <shaggy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@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 fd78a35..c10f085 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] 25+ messages in thread* [PATCH 04/15] cifs: eliminate oflags option from cifs_new_fileinfo
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 03/15] cifs: eliminate cifs_posix_open_inode_helper Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
[not found] ` <1286559072-29032-5-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-08 17:31 ` [PATCH 05/15] cifs: eliminate the inode argument " Jeff Layton
` (10 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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, 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 it really wants here is the file->f_mode, but this check
really makes no sense whatsoever. FMODE_READ will be set for O_RDWR or
O_RDONLY opens. So this in effect just moves those to the front of the
list and leaves O_WRONLY at the end.
That might make some sense if anything actually paid attention to this
list order, but nothing does. find_readable_file and find_writable_file
both walk through the list in the same direction.
Let's just keep this simple and just put things on the list with
list_add.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifsproto.h | 3 +--
fs/cifs/dir.c | 15 ++++-----------
fs/cifs/file.c | 5 ++---
3 files changed, 7 insertions(+), 16 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..452c9b5 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;
@@ -160,13 +160,7 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
pCifsInode = CIFS_I(newinode);
if (pCifsInode) {
- /* if readable file instance put first in list*/
- if (oflags & FMODE_READ)
- list_add(&pCifsFile->flist, &pCifsInode->openFileList);
- else
- list_add_tail(&pCifsFile->flist,
- &pCifsInode->openFileList);
-
+ list_add(&pCifsFile->flist, &pCifsInode->openFileList);
if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
pCifsInode->clientCanCacheAll = true;
pCifsInode->clientCanCacheRead = true;
@@ -483,7 +477,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 +751,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 c10f085..e863d80 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] 25+ messages in thread* [PATCH 05/15] cifs: eliminate the inode argument from cifs_new_fileinfo
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 04/15] cifs: eliminate oflags option from cifs_new_fileinfo Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 06/15] cifs: clean up cifs_reopen_file Jeff Layton
` (9 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 | 30 ++++++++++++++----------------
fs/cifs/file.c | 6 +++---
3 files changed, 18 insertions(+), 21 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 452c9b5..6c38a41 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,18 +159,16 @@ 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) {
- list_add(&pCifsFile->flist, &pCifsInode->openFileList);
- if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
- pCifsInode->clientCanCacheAll = true;
- pCifsInode->clientCanCacheRead = true;
- cFYI(1, "Exclusive Oplock inode %p", newinode);
- } else if ((oplock & 0xF) == OPLOCK_READ)
- pCifsInode->clientCanCacheRead = true;
- }
+ list_add(&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;
@@ -476,8 +475,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);
@@ -750,8 +748,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 e863d80..1edec19 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] 25+ messages in thread* [PATCH 06/15] cifs: clean up cifs_reopen_file
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 05/15] cifs: eliminate the inode argument " Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 07/15] cifs: cifs_write argument change and cleanup Jeff Layton
` (8 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 | 128 ++++++++++++++++++++++------------------------------
3 files changed, 56 insertions(+), 74 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 1e24ddc..3c8904e 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 6c38a41..5fe1d89 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 1edec19..db38547 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,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;
@@ -935,7 +915,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 +1013,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 +1161,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 +1709,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 +1795,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 +1960,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] 25+ messages in thread* [PATCH 07/15] cifs: cifs_write argument change and cleanup
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 06/15] cifs: clean up cifs_reopen_file Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 08/15] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
` (7 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 db38547..57fbfad 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -963,8 +963,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;
@@ -972,17 +973,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();
@@ -992,15 +990,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)
@@ -1060,20 +1049,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;
}
@@ -1244,8 +1226,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);
@@ -1560,7 +1542,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] 25+ messages in thread* [PATCH 08/15] cifs: eliminate pfile pointer from cifsFileInfo
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (6 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 07/15] cifs: cifs_write argument change and cleanup Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 09/15] cifs: move cifs_new_fileinfo to file.c Jeff Layton
` (6 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 3c8904e..6dcd911 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 5fe1d89..0ad8eef 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 57fbfad..83e16d6 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1080,8 +1080,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 */
@@ -1130,9 +1129,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) {
@@ -2096,9 +2093,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] 25+ messages in thread* [PATCH 09/15] cifs: move cifs_new_fileinfo to file.c
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (7 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 08/15] cifs: eliminate pfile pointer from cifsFileInfo Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 10/15] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
` (5 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 | 44 --------------------------------------------
fs/cifs/file.c | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 0ad8eef..983c483 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -130,50 +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));
- list_add(&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_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 83e16d6..86a1597 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -159,6 +159,49 @@ 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));
+ list_add(&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] 25+ messages in thread* [PATCH 10/15] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (8 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 09/15] cifs: move cifs_new_fileinfo to file.c Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
[not found] ` <1286559072-29032-11-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-08 17:31 ` [PATCH 11/15] cifs: move cifsFileInfo_put to file.c Jeff Layton
` (4 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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>
---
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 6dcd911..c29ef5f 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 54bd83a..d0aed27 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 86a1597..b6b88fe 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -186,10 +186,10 @@ 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));
list_add(&pCifsFile->flist, &pCifsInode->openFileList);
- write_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
if ((oplock & 0xF) == OPLOCK_EXCLUSIVE) {
pCifsInode->clientCanCacheAll = true;
@@ -539,13 +539,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)) {
@@ -565,9 +565,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. */
@@ -578,16 +578,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
@@ -595,7 +595,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);
@@ -617,18 +617,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");
@@ -1114,7 +1114,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 */
@@ -1128,7 +1128,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
@@ -1136,7 +1136,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
@@ -1163,7 +1163,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)
@@ -1177,11 +1177,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) {
@@ -1189,7 +1189,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;
}
@@ -1203,7 +1203,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);
@@ -1224,7 +1224,7 @@ refind_writable:
any_available = true;
goto refind_writable;
}
- read_unlock(&GlobalSMBSeslock);
+ spin_unlock(&cifs_file_list_lock);
return NULL;
}
@@ -2132,16 +2132,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;
}
@@ -2305,8 +2305,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] 25+ messages in thread* [PATCH 11/15] cifs: move cifsFileInfo_put to file.c
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (9 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 10/15] cifs: convert GlobalSMBSeslock from a rwlock to regular spinlock Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 12/15] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
` (3 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 c29ef5f..01e2f56 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 b6b88fe..2518711 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -202,6 +202,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] 25+ messages in thread* [PATCH 12/15] cifs: move close processing from cifs_close to cifsFileInfo_put
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (10 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 11/15] cifs: move cifsFileInfo_put to file.c Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 13/15] cifs: convert cifsFileInfo->count to non-atomic counter Jeff Layton
` (2 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 01e2f56..c928e39 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 2518711..8cedcd7 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);
@@ -202,14 +201,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)
@@ -537,79 +577,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)
@@ -956,13 +928,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
@@ -1043,13 +1008,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
@@ -1129,8 +1087,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) {
@@ -1176,8 +1132,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())
@@ -1192,34 +1146,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
@@ -1740,8 +1678,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;
@@ -1826,8 +1763,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;
@@ -1991,8 +1927,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;
@@ -2144,8 +2079,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;
@@ -2304,7 +2237,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] 25+ messages in thread* [PATCH 13/15] cifs: convert cifsFileInfo->count to non-atomic counter
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (11 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 12/15] cifs: move close processing from cifs_close to cifsFileInfo_put Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
[not found] ` <1286559072-29032-14-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-08 17:31 ` [PATCH 14/15] cifs: wait for writeback to complete in cifs_flush Jeff Layton
2010-10-08 17:31 ` [PATCH 15/15] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
14 siblings, 1 reply; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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>
---
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 c928e39..2327dcb 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -393,16 +393,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 8cedcd7..7b8122b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -172,6 +172,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();
@@ -182,7 +183,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);
@@ -203,7 +203,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)
{
@@ -212,7 +213,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;
}
@@ -2254,6 +2255,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] 25+ messages in thread* [PATCH 14/15] cifs: wait for writeback to complete in cifs_flush
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (12 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 13/15] cifs: convert cifsFileInfo->count to non-atomic counter Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
2010-10-08 17:31 ` [PATCH 15/15] cifs: eliminate cifsInodeInfo->write_behind_rc Jeff Layton
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 7b8122b..ce57953 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1620,20 +1620,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] 25+ messages in thread* [PATCH 15/15] cifs: eliminate cifsInodeInfo->write_behind_rc
[not found] ` <1286559072-29032-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (13 preceding siblings ...)
2010-10-08 17:31 ` [PATCH 14/15] cifs: wait for writeback to complete in cifs_flush Jeff Layton
@ 2010-10-08 17:31 ` Jeff Layton
14 siblings, 0 replies; 25+ messages in thread
From: Jeff Layton @ 2010-10-08 17:31 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 2327dcb..78db6f8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -418,7 +418,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 ce57953..8d13299 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");
@@ -538,8 +537,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;
@@ -1421,12 +1419,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);
}
@@ -1571,10 +1564,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);
}
@@ -1620,14 +1611,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);
@@ -2206,7 +2191,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)
@@ -2215,13 +2200,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] 25+ messages in thread