[PATCH 0/6] Introducing new strict cache mode (try #5)

[PATCH 0/6] Introducing new strict cache mode (try #5)

[Pavel Shilovsky]

Here are new set of patches. I made changes according to Jeff's notices and fix bugs in fsync and mmap.

Pavel Shilovsky (6):
  CIFS: Make cifsFileInfo_put work with strict cache mode
  CIFS: Make read call work with strict cache mode
  CIFS: Make write call work with strict cache mode
  CIFS: Make cifs_fsync work with strict cache mode
  CIFS: Make cifs_file_map work with strict cache mode
  CIFS: Add strictcache mount option

 fs/cifs/README       |    5 +++
 fs/cifs/cifs_fs_sb.h |    1 +
 fs/cifs/cifsfs.c     |   71 +++++++++++++++++++++++++++++++++++++++++++++----
 fs/cifs/connect.c    |    5 +++
 fs/cifs/file.c       |   62 +++++++++++++++++++++++++++++++++----------
 5 files changed, 124 insertions(+), 20 deletions(-)

-- 
1.7.2.3

[PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode

[Pavel Shilovsky]

On strict cache mode when we close the last file handle of the inode we
should set invalidate flag on the inode and check it during open to prevent
data coherency problem when we open it again but it has been modified by
other clients.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifs_fs_sb.h |    1 +
 fs/cifs/file.c       |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index e9a393c..be7b159 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -40,6 +40,7 @@
 #define CIFS_MOUNT_FSCACHE	0x8000 /* local caching enabled */
 #define CIFS_MOUNT_MF_SYMLINKS	0x10000 /* Minshall+French Symlinks enabled */
 #define CIFS_MOUNT_MULTIUSER	0x20000 /* multiuser mount */
+#define CIFS_MOUNT_STRICT_IO	0x40000 /* strict cache mode */
 
 struct cifs_sb_info {
 	struct rb_root tlink_tree;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 06c3e83..88bb366 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -148,6 +148,9 @@ client_can_cache:
 
 	cifs_set_oplock_level(pCifsInode, oplock);
 
+	if (pCifsInode->invalid_mapping)
+		invalidate_remote_inode(inode);
+
 	return rc;
 }
 
@@ -250,6 +253,9 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
 
 	cifs_set_oplock_level(pCifsInode, oplock);
 
+	if (pCifsInode->invalid_mapping)
+		invalidate_remote_inode(inode);
+
 	file->private_data = pCifsFile;
 	return pCifsFile;
 }
@@ -264,6 +270,7 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 	struct inode *inode = cifs_file->dentry->d_inode;
 	struct cifsTconInfo *tcon = tlink_tcon(cifs_file->tlink);
 	struct cifsInodeInfo *cifsi = CIFS_I(inode);
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifsLockInfo *li, *tmp;
 
 	spin_lock(&cifs_file_list_lock);
@@ -279,6 +286,13 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 	if (list_empty(&cifsi->openFileList)) {
 		cFYI(1, "closing last open instance for inode %p",
 			cifs_file->dentry->d_inode);
+
+		/* in strict cache mode we need invalidate mapping on the last
+		   close  because it may cause a error when we open this file
+		   again and get at least level II oplock */
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
+			CIFS_I(inode)->invalid_mapping = true;
+
 		cifs_set_oplock_level(cifsi, 0);
 	}
 	spin_unlock(&cifs_file_list_lock);
-- 
1.7.2.3

[PATCH 2/6] CIFS: Make read call work with strict cache mode

[Pavel Shilovsky]

Add cifs_file_aio_read where we read from the cache if we have at least
Level II oplock - otherwise read from the server.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 9c37897..bbb5294 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -568,6 +568,33 @@ cifs_do_mount(struct file_system_type *fs_type,
 	return dget(sb->s_root);
 }
 
+static ssize_t cifs_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
+				  unsigned long nr_segs, loff_t pos)
+{
+	struct inode *inode;
+	struct cifs_sb_info *cifs_sb;
+	ssize_t read;
+
+	inode = iocb->ki_filp->f_path.dentry->d_inode;
+	cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
+
+	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0 ||
+	    CIFS_I(inode)->clientCanCacheRead)
+		return generic_file_aio_read(iocb, iov, nr_segs, pos);
+
+	/* in strict cache mode we need to read from the server all the time
+	   if we don't have level II oplock because the server can delay mtime
+	   change - so we can't make a decision about inode invalidating.
+	   And we can also fail with pagereading if there are mandatory locks
+	   on pages affected by this read but not on the region from pos to
+	   pos+len-1 */
+	read = cifs_user_read(iocb->ki_filp, iov->iov_base,
+			      iov->iov_len, &pos);
+	iocb->ki_pos = pos;
+
+	return read;
+}
+
 static ssize_t cifs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 				   unsigned long nr_segs, loff_t pos)
 {
@@ -690,7 +717,7 @@ const struct inode_operations cifs_symlink_inode_ops = {
 const struct file_operations cifs_file_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
-	.aio_read = generic_file_aio_read,
+	.aio_read = cifs_file_aio_read,
 	.aio_write = cifs_file_aio_write,
 	.open = cifs_open,
 	.release = cifs_close,
@@ -727,7 +754,7 @@ const struct file_operations cifs_file_direct_ops = {
 const struct file_operations cifs_file_nobrl_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
-	.aio_read = generic_file_aio_read,
+	.aio_read = cifs_file_aio_read,
 	.aio_write = cifs_file_aio_write,
 	.open = cifs_open,
 	.release = cifs_close,
-- 
1.7.2.3

[PATCH 3/6] CIFS: Make write call work with strict cache mode

[Pavel Shilovsky]

On strict cache mode if we don't have Exclusive oplock we write a data to
the server through cifs_user_write. Then if we Level II oplock store it in
the cache, otherwise - invalidate inode pages affected by this writing.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c |   40 ++++++++++++++++++++++++++++++++++++----
 fs/cifs/file.c   |   14 ++++++++++++--
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index bbb5294..901c82b 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -598,12 +598,44 @@ static ssize_t cifs_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
 static ssize_t cifs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 				   unsigned long nr_segs, loff_t pos)
 {
-	struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;
+	struct inode *inode;
+	struct cifs_sb_info *cifs_sb;
 	ssize_t written;
 
-	written = generic_file_aio_write(iocb, iov, nr_segs, pos);
-	if (!CIFS_I(inode)->clientCanCacheAll)
-		filemap_fdatawrite(inode->i_mapping);
+	inode = iocb->ki_filp->f_path.dentry->d_inode;
+
+	if (CIFS_I(inode)->clientCanCacheAll)
+		return generic_file_aio_write(iocb, iov, nr_segs, pos);
+
+	cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
+
+	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0) {
+		int rc;
+
+		written = generic_file_aio_write(iocb, iov, nr_segs, pos);
+
+		rc = filemap_fdatawrite(inode->i_mapping);
+		if (rc)
+			cFYI(1, "cifs_file_aio_write: %d rc on %p inode",
+			     rc, inode);
+		return written;
+	}
+
+	/* in strict cache mode we need to write the data to the server exactly
+	   from the pos to pos+len-1 rather than flush all affected pages
+	   because it may cause a error with mandatory locks on these pages but
+	   not on the region from pos to ppos+len-1 */
+	written = cifs_user_write(iocb->ki_filp, iov->iov_base,
+				  iov->iov_len, &pos);
+
+	iocb->ki_pos = pos;
+
+	/* if we were successful - invalidate inode pages the write affected */
+	if (written > 0)
+		invalidate_mapping_pages(inode->i_mapping,
+					      (pos-written) >> PAGE_CACHE_SHIFT,
+					      (pos-1) >> PAGE_CACHE_SHIFT);
+
 	return written;
 }
 
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 88bb366..568e93f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1545,7 +1545,11 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
 			struct page *page, void *fsdata)
 {
 	int rc;
-	struct inode *inode = mapping->host;
+	struct inode *inode;
+	struct cifs_sb_info *cifs_sb;
+
+	inode = mapping->host;
+	cifs_sb = CIFS_SB(inode->i_sb);
 
 	cFYI(1, "write_end for page %p from pos %lld with %d bytes",
 		 page, pos, copied);
@@ -1578,7 +1582,13 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
 	} else {
 		rc = copied;
 		pos += copied;
-		set_page_dirty(page);
+		/* if we have strict cache switched on and don't have Exclusive
+		   oplock for the inode, we don't have to set_page_dirty
+		   because we flushed the data to the server in
+		   cifs_file_aio_write before */
+		if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0 ||
+		    CIFS_I(inode)->clientCanCacheAll)
+			set_page_dirty(page);
 	}
 
 	if (rc > 0) {
-- 
1.7.2.3

[PATCH 4/6] CIFS: Make cifs_fsync work with strict cache mode

[Pavel Shilovsky]

Remove filemap_write_and_wait call because it is previously called from
vfs_sync_range and invalidate inode if we don't have at least Level II
oplock and strcit cache mode switched on.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 568e93f..63fb551 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1611,20 +1611,20 @@ int cifs_fsync(struct file *file, int datasync)
 	struct cifsTconInfo *tcon;
 	struct cifsFileInfo *smbfile = file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
 	xid = GetXid();
 
 	cFYI(1, "Sync file - name: %s datasync: 0x%x",
 		file->f_path.dentry->d_name.name, datasync);
 
-	rc = filemap_write_and_wait(inode->i_mapping);
-	if (rc == 0) {
-		struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	if (!CIFS_I(inode)->clientCanCacheRead &&
+	    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO))
+		invalidate_remote_inode(inode);
 
-		tcon = tlink_tcon(smbfile->tlink);
-		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
-			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
-	}
+	tcon = tlink_tcon(smbfile->tlink);
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
+		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 
 	FreeXid(xid);
 	return rc;
-- 
1.7.2.3

[PATCH 5/6] CIFS: Make cifs_file_map work with strict cache mode

[Pavel Shilovsky]

Invalidate inode if we don't have at least Level II oplock and strict
cache mode switched on.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 63fb551..ef8d647 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1828,14 +1828,24 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
 int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	int rc, xid;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
 	xid = GetXid();
-	rc = cifs_revalidate_file(file);
-	if (rc) {
-		cFYI(1, "Validation prior to mmap failed, error=%d", rc);
-		FreeXid(xid);
-		return rc;
+
+	if (!CIFS_I(inode)->clientCanCacheRead &&
+	    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO))
+		invalidate_remote_inode(inode);
+	else {
+		rc = cifs_revalidate_file(file);
+		if (rc) {
+			cFYI(1, "Validation prior to mmap failed, error=%d",
+			     rc);
+			FreeXid(xid);
+			return rc;
+		}
 	}
+
 	rc = generic_file_mmap(file, vma);
 	FreeXid(xid);
 	return rc;
-- 
1.7.2.3

[PATCH 6/6] CIFS: Add strictcache mount option

[Pavel Shilovsky]

Use for switching on strict cache mode. In this mode the
client read from the cache all the time it has Oplock Level II,
otherwise - read from the server. All written data are stored
in the cache, but if the client doesn't have Exclusive Oplock,
it writes the data to the server.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/README    |    5 +++++
 fs/cifs/connect.c |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/README b/fs/cifs/README
index ee68d10..8fb6192 100644
--- a/fs/cifs/README
+++ b/fs/cifs/README
@@ -443,6 +443,11 @@ A partial list of the supported mount options follows:
 		if oplock (caching token) is granted and held. Note that
 		direct allows write operations larger than page size
 		to be sent to the server.
+  strictcache   Use for switching on strict cache mode. In this mode the
+		client read from the cache all the time it has Oplock Level II,
+		otherwise - read from the server. All written data are stored
+		in the cache, but if the client doesn't have Exclusive Oplock,
+		it writes the data to the server.
   acl   	Allow setfacl and getfacl to manage posix ACLs if server
 		supports them.  (default)
   noacl 	Do not allow setfacl and getfacl calls on this mount
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 251a17c..97286c7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -84,6 +84,7 @@ struct smb_vol {
 	bool no_xattr:1;   /* set if xattr (EA) support should be disabled*/
 	bool server_ino:1; /* use inode numbers from server ie UniqueId */
 	bool direct_io:1;
+	bool strict_io:1; /* strict cache behavior */
 	bool remap:1;      /* set to remap seven reserved chars in filenames */
 	bool posix_paths:1; /* unset to not ask for posix pathnames. */
 	bool no_linux_ext:1;
@@ -1347,6 +1348,8 @@ cifs_parse_mount_options(char *options, const char *devname,
 			vol->direct_io = 1;
 		} else if (strnicmp(data, "forcedirectio", 13) == 0) {
 			vol->direct_io = 1;
+		} else if (strnicmp(data, "strictcache", 11) == 0) {
+			vol->strict_io = 1;
 		} else if (strnicmp(data, "noac", 4) == 0) {
 			printk(KERN_WARNING "CIFS: Mount option noac not "
 				"supported. Instead set "
@@ -2597,6 +2600,8 @@ static void setup_cifs_sb(struct smb_vol *pvolume_info,
 	if (pvolume_info->multiuser)
 		cifs_sb->mnt_cifs_flags |= (CIFS_MOUNT_MULTIUSER |
 					    CIFS_MOUNT_NO_PERM);
+	if (pvolume_info->strict_io)
+		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_STRICT_IO;
 	if (pvolume_info->direct_io) {
 		cFYI(1, "mounting share using direct i/o");
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_DIRECT_IO;
-- 
1.7.2.3

[PATCH 3/6] CIFS: Make write call work with strict cache mode

[Pavel Shilovsky]

On strict cache mode if we don't have Exclusive oplock we write a data to
the server through cifs_user_write. Then if we Level II oplock store it in
the cache, otherwise - invalidate inode pages affected by this writing.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c |   40 ++++++++++++++++++++++++++++++++++++----
 fs/cifs/file.c   |    2 +-
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index bbb5294..901c82b 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -598,12 +598,44 @@ static ssize_t cifs_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
 static ssize_t cifs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 				   unsigned long nr_segs, loff_t pos)
 {
-	struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;
+	struct inode *inode;
+	struct cifs_sb_info *cifs_sb;
 	ssize_t written;
 
-	written = generic_file_aio_write(iocb, iov, nr_segs, pos);
-	if (!CIFS_I(inode)->clientCanCacheAll)
-		filemap_fdatawrite(inode->i_mapping);
+	inode = iocb->ki_filp->f_path.dentry->d_inode;
+
+	if (CIFS_I(inode)->clientCanCacheAll)
+		return generic_file_aio_write(iocb, iov, nr_segs, pos);
+
+	cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
+
+	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0) {
+		int rc;
+
+		written = generic_file_aio_write(iocb, iov, nr_segs, pos);
+
+		rc = filemap_fdatawrite(inode->i_mapping);
+		if (rc)
+			cFYI(1, "cifs_file_aio_write: %d rc on %p inode",
+			     rc, inode);
+		return written;
+	}
+
+	/* in strict cache mode we need to write the data to the server exactly
+	   from the pos to pos+len-1 rather than flush all affected pages
+	   because it may cause a error with mandatory locks on these pages but
+	   not on the region from pos to ppos+len-1 */
+	written = cifs_user_write(iocb->ki_filp, iov->iov_base,
+				  iov->iov_len, &pos);
+
+	iocb->ki_pos = pos;
+
+	/* if we were successful - invalidate inode pages the write affected */
+	if (written > 0)
+		invalidate_mapping_pages(inode->i_mapping,
+					      (pos-written) >> PAGE_CACHE_SHIFT,
+					      (pos-1) >> PAGE_CACHE_SHIFT);
+
 	return written;
 }
 
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 88bb366..9a30557 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1545,7 +1545,7 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
 			struct page *page, void *fsdata)
 {
 	int rc;
-	struct inode *inode = mapping->host;
+	struct inode *inode = mapping->host
 
 	cFYI(1, "write_end for page %p from pos %lld with %d bytes",
 		 page, pos, copied);
-- 
1.7.2.3

Re: [PATCH 3/6] CIFS: Make write call work with strict cache mode

[Pavel Shilovsky]

2010/11/10 Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> On strict cache mode if we don't have Exclusive oplock we write a data to
> the server through cifs_user_write. Then if we Level II oplock store it in
> the cache, otherwise - invalidate inode pages affected by this writing.

I am very sorry about this spam. I am going to post right version of
the patch further.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode

[Jeff Layton]

On Wed, 10 Nov 2010 18:41:46 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On strict cache mode when we close the last file handle of the inode we
> should set invalidate flag on the inode and check it during open to prevent
> data coherency problem when we open it again but it has been modified by
> other clients.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifs_fs_sb.h |    1 +
>  fs/cifs/file.c       |   14 ++++++++++++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
> index e9a393c..be7b159 100644
> --- a/fs/cifs/cifs_fs_sb.h
> +++ b/fs/cifs/cifs_fs_sb.h
> @@ -40,6 +40,7 @@
>  #define CIFS_MOUNT_FSCACHE	0x8000 /* local caching enabled */
>  #define CIFS_MOUNT_MF_SYMLINKS	0x10000 /* Minshall+French Symlinks enabled */
>  #define CIFS_MOUNT_MULTIUSER	0x20000 /* multiuser mount */
> +#define CIFS_MOUNT_STRICT_IO	0x40000 /* strict cache mode */
>  
>  struct cifs_sb_info {
>  	struct rb_root tlink_tree;
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 06c3e83..88bb366 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -148,6 +148,9 @@ client_can_cache:
>  
>  	cifs_set_oplock_level(pCifsInode, oplock);
>  
> +	if (pCifsInode->invalid_mapping)
> +		invalidate_remote_inode(inode);
> +
>  	return rc;
>  }
>  
> @@ -250,6 +253,9 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
>  
>  	cifs_set_oplock_level(pCifsInode, oplock);
>  
> +	if (pCifsInode->invalid_mapping)
> +		invalidate_remote_inode(inode);
> +
>  	file->private_data = pCifsFile;
>  	return pCifsFile;
>  }

I don't see why the above is necessary. cifs_revalidate_dentry should
have taken care of invalidating the mapping when it revalidated the
dentry.

Also, you're leaving invalid_mapping set so you're almost certainly
going to end up invalidating the mapping unnecessarily again.

Finally, you're not considering the fscache case here. I think you need
to look closely at how dentry/inode revalidation is handled -- in
particular cifs_revalidate_dentry and cifs_invalidate_mapping.

> @@ -264,6 +270,7 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  	struct inode *inode = cifs_file->dentry->d_inode;
>  	struct cifsTconInfo *tcon = tlink_tcon(cifs_file->tlink);
>  	struct cifsInodeInfo *cifsi = CIFS_I(inode);
> +	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>  	struct cifsLockInfo *li, *tmp;
>  
>  	spin_lock(&cifs_file_list_lock);
> @@ -279,6 +286,13 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  	if (list_empty(&cifsi->openFileList)) {
>  		cFYI(1, "closing last open instance for inode %p",
>  			cifs_file->dentry->d_inode);
> +
> +		/* in strict cache mode we need invalidate mapping on the last
> +		   close  because it may cause a error when we open this file
> +		   again and get at least level II oplock */
> +		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
> +			CIFS_I(inode)->invalid_mapping = true;
> +
>  		cifs_set_oplock_level(cifsi, 0);
>  	}
>  	spin_unlock(&cifs_file_list_lock);


-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode

[Pavel Shilovsky]

2010/11/13 Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>
> I don't see why the above is necessary. cifs_revalidate_dentry should
> have taken care of invalidating the mapping when it revalidated the
> dentry.
>
> Also, you're leaving invalid_mapping set so you're almost certainly
> going to end up invalidating the mapping unnecessarily again.
>
> Finally, you're not considering the fscache case here. I think you need
> to look closely at how dentry/inode revalidation is handled -- in
> particular cifs_revalidate_dentry and cifs_invalidate_mapping.
>

Jeff, you are right about leaving invalid_mapping set - I noticed it
after some time I posted this patch. So - I reposted it in the list
again. Let's consider this version as invalid.

-- 
Best regards,
Pavel Shilovsky.

[PATCH 4/6] CIFS: Make cifs_fsync work with strict cache mode

[Pavel Shilovsky]

Remove filemap_write_and_wait call because it is previously called from
vfs_sync_range and invalidate inode if we don't have at least Level II
oplock and strict cache mode switched on.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 85824c0..e8ca2d6 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1603,20 +1603,20 @@ int cifs_fsync(struct file *file, int datasync)
 	struct cifsTconInfo *tcon;
 	struct cifsFileInfo *smbfile = file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
 	xid = GetXid();
 
 	cFYI(1, "Sync file - name: %s datasync: 0x%x",
 		file->f_path.dentry->d_name.name, datasync);
 
-	rc = filemap_write_and_wait(inode->i_mapping);
-	if (rc == 0) {
-		struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	if (!CIFS_I(inode)->clientCanCacheRead &&
+	    ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0))
+		invalidate_remote_inode(inode);
 
-		tcon = tlink_tcon(smbfile->tlink);
-		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
-			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
-	}
+	tcon = tlink_tcon(smbfile->tlink);
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
+		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 
 	FreeXid(xid);
 	return rc;
-- 
1.7.2.3

[PATCH 4/6] CIFS: Make cifs_fsync work with strict cache mode

[Pavel Shilovsky]

Remove filemap_write_and_wait call because it is previously called from
vfs_sync_range and invalidate inode if we don't have at least Level II
oplock and strict cache mode switched on.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 85824c0..e8ca2d6 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1603,20 +1603,20 @@ int cifs_fsync(struct file *file, int datasync)
 	struct cifsTconInfo *tcon;
 	struct cifsFileInfo *smbfile = file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
 	xid = GetXid();
 
 	cFYI(1, "Sync file - name: %s datasync: 0x%x",
 		file->f_path.dentry->d_name.name, datasync);
 
-	rc = filemap_write_and_wait(inode->i_mapping);
-	if (rc == 0) {
-		struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	if (!CIFS_I(inode)->clientCanCacheRead &&
+	    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO))
+		invalidate_remote_inode(inode);
 
-		tcon = tlink_tcon(smbfile->tlink);
-		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
-			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
-	}
+	tcon = tlink_tcon(smbfile->tlink);
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
+		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 
 	FreeXid(xid);
 	return rc;
-- 
1.7.2.3

Re: [PATCH 6/6] CIFS: Add strictcache mount option

[Suresh Jayaraman]

On 11/10/2010 09:11 PM, Pavel Shilovsky wrote:
> Use for switching on strict cache mode. In this mode the
> client read from the cache all the time it has Oplock Level II,
> otherwise - read from the server. All written data are stored
> in the cache, but if the client doesn't have Exclusive Oplock,
> it writes the data to the server.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/README    |    5 +++++
>  fs/cifs/connect.c |    5 +++++
>  2 files changed, 10 insertions(+), 0 deletions(-)


Sorry for chiming in so late. IIUC, the name 'strictcache' was chosen to
mean "strictly use cache" as in "strict sync" option in smb.conf. But,
I'm afraid that the name also make users think of strict cache coherency
which is a different behavior. Is there a alternative name that you
could think of to avoid confusion?


-- 
Suresh Jayaraman

[PATCH 4/6] CIFS: Make cifs_fsync work with strict cache mode

[Pavel Shilovsky]

Remove filemap_write_and_wait call because it is previously called from
vfs_sync_range and invalidate inode if we don't have at least Level II
oplock and strcit cache mode switched on.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 85824c0..e8ca2d6 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1603,20 +1603,20 @@ int cifs_fsync(struct file *file, int datasync)
 	struct cifsTconInfo *tcon;
 	struct cifsFileInfo *smbfile = file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 
 	xid = GetXid();
 
 	cFYI(1, "Sync file - name: %s datasync: 0x%x",
 		file->f_path.dentry->d_name.name, datasync);
 
-	rc = filemap_write_and_wait(inode->i_mapping);
-	if (rc == 0) {
-		struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	if (!CIFS_I(inode)->clientCanCacheRead &&
+	    ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) == 0))
+		invalidate_remote_inode(inode);
 
-		tcon = tlink_tcon(smbfile->tlink);
-		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
-			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
-	}
+	tcon = tlink_tcon(smbfile->tlink);
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
+		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 
 	FreeXid(xid);
 	return rc;
-- 
1.7.2.3