All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode
@ 2010-12-14 11:10 Pavel Shilovsky
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:10 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On strict cache mode when we close the last file handle of the inode we
should set invalid_mapping flag on this inode 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>
Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifs_fs_sb.h |    1 +
 fs/cifs/file.c       |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 7852cd6..ac51cd2 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 5a28660..60c3f84 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -264,6 +264,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 +280,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.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] CIFS: Implement cifs_strict_fsync
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 11:10   ` Pavel Shilovsky
       [not found]     ` <1292325060-8828-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:10   ` [PATCH 3/6] CIFS: Implement cifs_file_strict_mmap Pavel Shilovsky
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:10 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

Invalidate inode mapping if we don't have at least Level II oplock in
cifs_strict_fsync. Also remove filemap_write_and_wait call from cifs_fsync
because it is previously called from vfs_fsync_range. Add file operations'
structures for strict cache mode.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c |   38 ++++++++++++++++++++++++++++++++++++++
 fs/cifs/cifsfs.h |    8 ++++++--
 fs/cifs/file.c   |   36 ++++++++++++++++++++++++++++--------
 fs/cifs/inode.c  |    8 ++++++--
 4 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 3936aa7..7660133 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -710,6 +710,25 @@ const struct file_operations cifs_file_ops = {
 	.setlease = cifs_setlease,
 };
 
+const struct file_operations cifs_file_strict_ops = {
+	.read = do_sync_read,
+	.write = do_sync_write,
+	.aio_read = generic_file_aio_read,
+	.aio_write = cifs_file_aio_write,
+	.open = cifs_open,
+	.release = cifs_close,
+	.lock = cifs_lock,
+	.fsync = cifs_strict_fsync,
+	.flush = cifs_flush,
+	.mmap = cifs_file_mmap,
+	.splice_read = generic_file_splice_read,
+	.llseek = cifs_llseek,
+#ifdef CONFIG_CIFS_POSIX
+	.unlocked_ioctl	= cifs_ioctl,
+#endif /* CONFIG_CIFS_POSIX */
+	.setlease = cifs_setlease,
+};
+
 const struct file_operations cifs_file_direct_ops = {
 	/* no aio, no readv -
 	   BB reevaluate whether they can be done with directio, no cache */
@@ -728,6 +747,7 @@ const struct file_operations cifs_file_direct_ops = {
 	.llseek = cifs_llseek,
 	.setlease = cifs_setlease,
 };
+
 const struct file_operations cifs_file_nobrl_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
@@ -746,6 +766,24 @@ const struct file_operations cifs_file_nobrl_ops = {
 	.setlease = cifs_setlease,
 };
 
+const struct file_operations cifs_file_strict_nobrl_ops = {
+	.read = do_sync_read,
+	.write = do_sync_write,
+	.aio_read = generic_file_aio_read,
+	.aio_write = cifs_file_aio_write,
+	.open = cifs_open,
+	.release = cifs_close,
+	.fsync = cifs_strict_fsync,
+	.flush = cifs_flush,
+	.mmap = cifs_file_mmap,
+	.splice_read = generic_file_splice_read,
+	.llseek = cifs_llseek,
+#ifdef CONFIG_CIFS_POSIX
+	.unlocked_ioctl	= cifs_ioctl,
+#endif /* CONFIG_CIFS_POSIX */
+	.setlease = cifs_setlease,
+};
+
 const struct file_operations cifs_file_direct_nobrl_ops = {
 	/* no mmap, no aio, no readv -
 	   BB reevaluate whether they can be done with directio, no cache */
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 897b2b2..8584829 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -61,6 +61,7 @@ extern int cifs_rename(struct inode *, struct dentry *, struct inode *,
 		       struct dentry *);
 extern int cifs_revalidate_file(struct file *filp);
 extern int cifs_revalidate_dentry(struct dentry *);
+extern void cifs_invalidate_mapping(struct inode *inode);
 extern int cifs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
 extern int cifs_setattr(struct dentry *, struct iattr *);
 
@@ -72,8 +73,10 @@ extern const struct inode_operations cifs_dfs_referral_inode_operations;
 /* Functions related to files and directories */
 extern const struct file_operations cifs_file_ops;
 extern const struct file_operations cifs_file_direct_ops; /* if directio mnt */
-extern const struct file_operations cifs_file_nobrl_ops;
-extern const struct file_operations cifs_file_direct_nobrl_ops; /* no brlocks */
+extern const struct file_operations cifs_file_strict_ops; /* if strictio mnt */
+extern const struct file_operations cifs_file_nobrl_ops; /* no brlocks */
+extern const struct file_operations cifs_file_direct_nobrl_ops;
+extern const struct file_operations cifs_file_strict_nobrl_ops;
 extern int cifs_open(struct inode *inode, struct file *file);
 extern int cifs_close(struct inode *inode, struct file *file);
 extern int cifs_closedir(struct inode *inode, struct file *file);
@@ -83,6 +86,7 @@ extern ssize_t cifs_user_write(struct file *file, const char __user *write_data,
 			 size_t write_size, loff_t *poffset);
 extern int cifs_lock(struct file *, int, struct file_lock *);
 extern int cifs_fsync(struct file *, int);
+extern int cifs_strict_fsync(struct file *, int);
 extern int cifs_flush(struct file *, fl_owner_t id);
 extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
 extern const struct file_operations cifs_dir_ops;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 60c3f84..4e37cb8 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1586,27 +1586,47 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
 	return rc;
 }
 
-int cifs_fsync(struct file *file, int datasync)
+int cifs_strict_fsync(struct file *file, int datasync)
 {
 	int xid;
 	int rc = 0;
 	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_invalidate_mapping(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;
+}
+
+int cifs_fsync(struct file *file, int datasync)
+{
+	int xid;
+	int rc = 0;
+	struct cifsTconInfo *tcon;
+	struct cifsFileInfo *smbfile = file->private_data;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
+
+	xid = GetXid();
+
+	cFYI(1, "Sync file - name: %s datasync: 0x%x",
+		file->f_path.dentry->d_name.name, datasync);
+
+	tcon = tlink_tcon(smbfile->tlink);
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
+		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 
 	FreeXid(xid);
 	return rc;
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 589f3e3..a8f7cf3 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -44,13 +44,17 @@ static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
 				inode->i_fop = &cifs_file_direct_nobrl_ops;
 			else
 				inode->i_fop = &cifs_file_direct_ops;
+		} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) {
+			if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
+				inode->i_fop = &cifs_file_strict_nobrl_ops;
+			else
+				inode->i_fop = &cifs_file_strict_ops;
 		} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
 			inode->i_fop = &cifs_file_nobrl_ops;
 		else { /* not direct, send byte range locks */
 			inode->i_fop = &cifs_file_ops;
 		}
 
-
 		/* check if server can support readpages */
 		if (cifs_sb_master_tcon(cifs_sb)->ses->server->maxBuf <
 				PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE)
@@ -1679,7 +1683,7 @@ cifs_inode_needs_reval(struct inode *inode)
 /*
  * Zap the cache. Called when invalid_mapping flag is set.
  */
-static void
+void
 cifs_invalidate_mapping(struct inode *inode)
 {
 	int rc;
-- 
1.7.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] CIFS: Implement cifs_file_strict_mmap
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:10   ` [PATCH 2/6] CIFS: Implement cifs_strict_fsync Pavel Shilovsky
@ 2010-12-14 11:10   ` Pavel Shilovsky
       [not found]     ` <1292325060-8828-3-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:10   ` [PATCH 4/6] CIFS: Implement cifs_strict_write Pavel Shilovsky
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:10 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

Invalidate inode mapping if we don't have at least Level II oplock.

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

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 7660133..a218afe 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -720,7 +720,7 @@ const struct file_operations cifs_file_strict_ops = {
 	.lock = cifs_lock,
 	.fsync = cifs_strict_fsync,
 	.flush = cifs_flush,
-	.mmap = cifs_file_mmap,
+	.mmap = cifs_file_strict_mmap,
 	.splice_read = generic_file_splice_read,
 	.llseek = cifs_llseek,
 #ifdef CONFIG_CIFS_POSIX
@@ -775,7 +775,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = {
 	.release = cifs_close,
 	.fsync = cifs_strict_fsync,
 	.flush = cifs_flush,
-	.mmap = cifs_file_mmap,
+	.mmap = cifs_file_strict_mmap,
 	.splice_read = generic_file_splice_read,
 	.llseek = cifs_llseek,
 #ifdef CONFIG_CIFS_POSIX
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 8584829..657738f 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -89,6 +89,7 @@ extern int cifs_fsync(struct file *, int);
 extern int cifs_strict_fsync(struct file *, int);
 extern int cifs_flush(struct file *, fl_owner_t id);
 extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
+extern int cifs_file_strict_mmap(struct file * , struct vm_area_struct *);
 extern const struct file_operations cifs_dir_ops;
 extern int cifs_dir_open(struct inode *inode, struct file *file);
 extern int cifs_readdir(struct file *file, void *direntry, filldir_t filldir);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 4e37cb8..dd1e59a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1827,17 +1827,34 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
 	return total_read;
 }
 
+int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	int rc, xid;
+	struct inode *inode = file->f_path.dentry->d_inode;
+
+	xid = GetXid();
+
+	if (!CIFS_I(inode)->clientCanCacheRead)
+		cifs_invalidate_mapping(inode);
+
+	rc = generic_file_mmap(file, vma);
+	FreeXid(xid);
+	return rc;
+}
+
 int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	int rc, xid;
 
 	xid = GetXid();
+
 	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.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] CIFS: Implement cifs_strict_write
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:10   ` [PATCH 2/6] CIFS: Implement cifs_strict_fsync Pavel Shilovsky
  2010-12-14 11:10   ` [PATCH 3/6] CIFS: Implement cifs_file_strict_mmap Pavel Shilovsky
@ 2010-12-14 11:10   ` Pavel Shilovsky
       [not found]     ` <1292325060-8828-4-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:10   ` [PATCH 5/6] CIFS: Implement cifs_strict_read Pavel Shilovsky
  2010-12-14 11:11   ` [PATCH 6/6] CIFS: Add strictcache mount option Pavel Shilovsky
  4 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:10 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

If we don't have Exclusive oplock we write a data to the server through
cifs_write. Also set invalidate_mapping flag on the inode if we wrote
something to the server.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c    |   74 ++++++++++++++++++++++++++++++++++++++++++++++++---
 fs/cifs/cifsproto.h |    3 ++
 fs/cifs/file.c      |    5 +--
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index a218afe..5df0503 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -572,15 +572,81 @@ cifs_do_mount(struct file_system_type *fs_type,
 	return dget(sb->s_root);
 }
 
+static ssize_t cifs_strict_write(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 written = 0;
+	unsigned long i, len = 0;
+	char *buf, *offset;
+
+	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);
+
+	/*
+	 * 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.
+	 */
+
+	for (i = 0; i < nr_segs; i++)
+		len += iov[i].iov_len;
+
+	/*
+	 * BB - optimize the way when signing is disabled. We can drop this
+	 * extra memory-to-memory copying and use iovec buffers for constructing
+	 * write request.
+	 */
+
+	buf = kmalloc(len, GFP_KERNEL);
+
+	for (i = 0, offset = buf; i < nr_segs; i++) {
+		if (copy_from_user(offset, iov[i].iov_base, iov[i].iov_len)) {
+			kfree(buf);
+			return -EFAULT;
+		}
+
+		offset += iov[i].iov_len;
+	}
+
+	written = cifs_write((struct cifsFileInfo *)iocb->ki_filp->private_data,
+			     buf, len, &pos);
+	if (written < 0) {
+		kfree(buf);
+		return written;
+	}
+
+	iocb->ki_pos = pos;
+
+	if (written > 0)
+		CIFS_I(inode)->invalid_mapping = true;
+
+	kfree(buf);
+	return written;
+}
+
 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;
 	ssize_t written;
+	int rc;
 
 	written = generic_file_aio_write(iocb, iov, nr_segs, pos);
-	if (!CIFS_I(inode)->clientCanCacheAll)
-		filemap_fdatawrite(inode->i_mapping);
+
+	if (CIFS_I(inode)->clientCanCacheAll)
+		return written;
+
+	rc = filemap_fdatawrite(inode->i_mapping);
+	if (rc)
+		cFYI(1, "cifs_file_aio_write: %d rc on %p inode", rc, inode);
+
 	return written;
 }
 
@@ -714,7 +780,7 @@ const struct file_operations cifs_file_strict_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
 	.aio_read = generic_file_aio_read,
-	.aio_write = cifs_file_aio_write,
+	.aio_write = cifs_strict_write,
 	.open = cifs_open,
 	.release = cifs_close,
 	.lock = cifs_lock,
@@ -770,7 +836,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
 	.aio_read = generic_file_aio_read,
-	.aio_write = cifs_file_aio_write,
+	.aio_write = cifs_strict_write,
 	.open = cifs_open,
 	.release = cifs_close,
 	.fsync = cifs_strict_fsync,
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index e6d1481..3a337a9 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -79,6 +79,9 @@ extern int checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length);
 extern bool is_valid_oplock_break(struct smb_hdr *smb,
 				  struct TCP_Server_Info *);
 extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof);
+extern ssize_t cifs_write(struct cifsFileInfo *open_file,
+			  const char *write_data, size_t write_size,
+			  loff_t *poffset);
 extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool);
 extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
 extern unsigned int smbCalcSize(struct smb_hdr *ptr);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index dd1e59a..2c848ef 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1026,9 +1026,8 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
 	return total_written;
 }
 
-static ssize_t cifs_write(struct cifsFileInfo *open_file,
-			  const char *write_data, size_t write_size,
-			  loff_t *poffset)
+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;
-- 
1.7.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] CIFS: Implement cifs_strict_read
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2010-12-14 11:10   ` [PATCH 4/6] CIFS: Implement cifs_strict_write Pavel Shilovsky
@ 2010-12-14 11:10   ` Pavel Shilovsky
       [not found]     ` <1292325060-8828-5-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-12-14 11:11   ` [PATCH 6/6] CIFS: Add strictcache mount option Pavel Shilovsky
  4 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:10 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

Read from the cache if we have at least Level II oplock - otherwise
read from the server. Add cifs_user_readv to let the client read into
several buffers.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsfs.c |   43 +++++++++++++++++++++++++++++-
 fs/cifs/cifsfs.h |    5 +++-
 fs/cifs/file.c   |   76 ++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5df0503..a47b8af 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -572,6 +572,45 @@ cifs_do_mount(struct file_system_type *fs_type,
 	return dget(sb->s_root);
 }
 
+static ssize_t cifs_strict_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 = 0;
+	unsigned long i, len = 0;
+
+	if (!nr_segs)
+		return read;
+
+	inode = iocb->ki_filp->f_path.dentry->d_inode;
+	cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
+
+	if (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.
+	 */
+
+	for (i = 0; i < nr_segs; i++)
+		len += iov[i].iov_len;
+
+
+	read = cifs_user_readv(iocb->ki_filp, iov, nr_segs, len, &pos);
+	if (read < 0)
+		return read;
+
+	iocb->ki_pos = pos;
+
+	return read;
+}
+
 static ssize_t cifs_strict_write(struct kiocb *iocb, const struct iovec *iov,
 				 unsigned long nr_segs, loff_t pos)
 {
@@ -779,7 +818,7 @@ const struct file_operations cifs_file_ops = {
 const struct file_operations cifs_file_strict_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
-	.aio_read = generic_file_aio_read,
+	.aio_read = cifs_strict_read,
 	.aio_write = cifs_strict_write,
 	.open = cifs_open,
 	.release = cifs_close,
@@ -835,7 +874,7 @@ const struct file_operations cifs_file_nobrl_ops = {
 const struct file_operations cifs_file_strict_nobrl_ops = {
 	.read = do_sync_read,
 	.write = do_sync_write,
-	.aio_read = generic_file_aio_read,
+	.aio_read = cifs_strict_read,
 	.aio_write = cifs_strict_write,
 	.open = cifs_open,
 	.release = cifs_close,
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 657738f..a6bea06 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -81,7 +81,10 @@ extern int cifs_open(struct inode *inode, struct file *file);
 extern int cifs_close(struct inode *inode, struct file *file);
 extern int cifs_closedir(struct inode *inode, struct file *file);
 extern ssize_t cifs_user_read(struct file *file, char __user *read_data,
-			 size_t read_size, loff_t *poffset);
+			      size_t read_size, loff_t *poffset);
+extern ssize_t cifs_user_readv(struct file *file, const struct iovec *iov,
+			       unsigned long nr_segs, size_t len,
+			       loff_t *poffset);
 extern ssize_t cifs_user_write(struct file *file, const char __user *write_data,
 			 size_t write_size, loff_t *poffset);
 extern int cifs_lock(struct file *, int, struct file_lock *);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 2c848ef..9238a2f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1676,8 +1676,40 @@ int cifs_flush(struct file *file, fl_owner_t id)
 	return rc;
 }
 
-ssize_t cifs_user_read(struct file *file, char __user *read_data,
-	size_t read_size, loff_t *poffset)
+static int
+cifs_fill_iov(const struct iovec *iov, unsigned long nr_segs,
+	      unsigned long *last_len, unsigned long *last_ind, char *data,
+	      size_t datalen)
+{
+	const struct iovec *ciov;
+	unsigned long cur_len;
+
+	while (datalen) {
+		cur_len = min_t(unsigned long, datalen,
+					      *last_len);
+		ciov = &iov[*last_ind];
+
+		if (copy_to_user(ciov->iov_base + (ciov->iov_len - *last_len),
+				 data, cur_len))
+			return -EFAULT;
+
+		datalen -= cur_len;
+		data += cur_len;
+
+		*last_len -= cur_len;
+		if (*last_len == 0) {
+			(*last_ind)++;
+			*last_len = iov[*last_ind].iov_len;
+		}
+	}
+
+	return 0;
+}
+
+static ssize_t
+cifs_generic_user_read(struct file *file, char __user *read_data,
+		       size_t read_size, loff_t *poffset,
+		       const struct iovec *iov, unsigned long nr_segs)
 {
 	int rc = -EACCES;
 	unsigned int bytes_read = 0;
@@ -1690,6 +1722,11 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
 	char *smb_read_data;
 	char __user *current_offset;
 	struct smb_com_read_rsp *pSMBr;
+	unsigned long last_len = 0, last_ind = 0;
+
+	/* if vectored - set last_len value */
+	if (!read_data && nr_segs > 0)
+		last_len = iov->iov_len;
 
 	xid = GetXid();
 	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
@@ -1707,7 +1744,8 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
 
 	for (total_read = 0, current_offset = read_data;
 	     read_size > total_read;
-	     total_read += bytes_read, current_offset += bytes_read) {
+	     total_read += bytes_read,
+	     current_offset += read_data ? bytes_read : 0) {
 		current_read_size = min_t(const int, read_size - total_read,
 					  cifs_sb->rsize);
 		rc = -EAGAIN;
@@ -1726,12 +1764,18 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
 					 &buf_type);
 			pSMBr = (struct smb_com_read_rsp *)smb_read_data;
 			if (smb_read_data) {
-				if (copy_to_user(current_offset,
-						smb_read_data +
-						4 /* RFC1001 length field */ +
-						le16_to_cpu(pSMBr->DataOffset),
-						bytes_read))
-					rc = -EFAULT;
+				char *data_offset = smb_read_data + 4 +
+						le16_to_cpu(pSMBr->DataOffset);
+				if (read_data) {
+					if (copy_to_user(current_offset,
+						data_offset, bytes_read))
+						rc = -EFAULT;
+				} else {
+					if (cifs_fill_iov(iov, nr_segs,
+						&last_len, &last_ind,
+						data_offset, bytes_read))
+						rc = -EFAULT;
+				}
 
 				if (buf_type == CIFS_SMALL_BUFFER)
 					cifs_small_buf_release(smb_read_data);
@@ -1756,9 +1800,21 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
 	return total_read;
 }
 
+ssize_t cifs_user_read(struct file *file, char __user *read_data,
+		       size_t read_size, loff_t *poffset)
+{
+	return cifs_generic_user_read(file, read_data, read_size, poffset, NULL,
+				      0);
+}
+
+ssize_t cifs_user_readv(struct file *file, const struct iovec *iov,
+			unsigned long nr_segs, size_t len, loff_t *poffset)
+{
+	return cifs_generic_user_read(file, NULL, len, poffset, iov, nr_segs);
+}
 
 static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
-	loff_t *poffset)
+			 loff_t *poffset)
 {
 	int rc = -EACCES;
 	unsigned int bytes_read = 0;
-- 
1.7.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] CIFS: Add strictcache mount option
       [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2010-12-14 11:10   ` [PATCH 5/6] CIFS: Implement cifs_strict_read Pavel Shilovsky
@ 2010-12-14 11:11   ` Pavel Shilovsky
       [not found]     ` <1292325060-8828-6-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  4 siblings, 1 reply; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 11:11 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

Use for switching on strict cache mode. In this mode the
client reads from the cache all the time it has Oplock Level II,
otherwise - read from the server. As for write - the client stores
a data in the cache in Exclusive Oplock case, otherwise - write
directly 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 46af99a..fe16835 100644
--- a/fs/cifs/README
+++ b/fs/cifs/README
@@ -452,6 +452,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 cc1a860..877cb9e 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;
@@ -1357,6 +1358,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 "
@@ -2614,6 +2617,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.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] CIFS: Implement cifs_strict_write
       [not found]     ` <1292325060-8828-4-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 13:40       ` Jeff Layton
       [not found]         ` <20101214084053.0da711a2-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
       [not found]         ` <AANLkTinHKWHff8TzWapRLtX-sA4FK98Ntt+a_Fv8Z5f_@mail.gmail.com>
  0 siblings, 2 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 13:40 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 14:10:58 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> If we don't have Exclusive oplock we write a data to the server through
> cifs_write. Also set invalidate_mapping flag on the inode if we wrote
> something to the server.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c    |   74 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  fs/cifs/cifsproto.h |    3 ++
>  fs/cifs/file.c      |    5 +--
>  3 files changed, 75 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index a218afe..5df0503 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -572,15 +572,81 @@ cifs_do_mount(struct file_system_type *fs_type,
>  	return dget(sb->s_root);
>  }
>  
> +static ssize_t cifs_strict_write(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 written = 0;
> +	unsigned long i, len = 0;
> +	char *buf, *offset;
> +
> +	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);
> +
> +	/*
> +	 * 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.
> +	 */
> +
> +	for (i = 0; i < nr_segs; i++)
> +		len += iov[i].iov_len;
> +
> +	/*
> +	 * BB - optimize the way when signing is disabled. We can drop this
> +	 * extra memory-to-memory copying and use iovec buffers for constructing
> +	 * write request.
> +	 */
> +
> +	buf = kmalloc(len, GFP_KERNEL);
> +

Actually...I thought about another problem here. Suppose an application
does a 100M write. This may fail in that case. It varies by the slab
allocator in use, but there are limits to how large you can kmalloc.
This is especially going to be a problem on 32-bit arches where
GFP_KERNEL memory is more limited.

There are a couple of different solutions, but I think the best one
would be to stitch together individual pages. Walk the iov array first
and figure out how many pages you're going to need, and kmalloc an iov
array that size. Then start allocating pages (preferably using
GFP_HIGHMEM and kmapping them) and then copy the data to them. Stop
when you get to the wsize limit, and issue a CIFSSMBWrite2 call. When
it returns, pick up where you left off and do it again.

> +	for (i = 0, offset = buf; i < nr_segs; i++) {
> +		if (copy_from_user(offset, iov[i].iov_base, iov[i].iov_len)) {
> +			kfree(buf);
> +			return -EFAULT;
> +		}
> +
> +		offset += iov[i].iov_len;
> +	}
> +
> +	written = cifs_write((struct cifsFileInfo *)iocb->ki_filp->private_data,
> +			     buf, len, &pos);
> +	if (written < 0) {
> +		kfree(buf);
> +		return written;
> +	}
> +
> +	iocb->ki_pos = pos;
> +
> +	if (written > 0)
> +		CIFS_I(inode)->invalid_mapping = true;
> +
> +	kfree(buf);
> +	return written;
> +}
> +
>  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;
>  	ssize_t written;
> +	int rc;
>  
>  	written = generic_file_aio_write(iocb, iov, nr_segs, pos);
> -	if (!CIFS_I(inode)->clientCanCacheAll)
> -		filemap_fdatawrite(inode->i_mapping);
> +
> +	if (CIFS_I(inode)->clientCanCacheAll)
> +		return written;
> +
> +	rc = filemap_fdatawrite(inode->i_mapping);
> +	if (rc)
> +		cFYI(1, "cifs_file_aio_write: %d rc on %p inode", rc, inode);
> +
>  	return written;
>  }
>  
> @@ -714,7 +780,7 @@ const struct file_operations cifs_file_strict_ops = {
>  	.read = do_sync_read,
>  	.write = do_sync_write,
>  	.aio_read = generic_file_aio_read,
> -	.aio_write = cifs_file_aio_write,
> +	.aio_write = cifs_strict_write,
>  	.open = cifs_open,
>  	.release = cifs_close,
>  	.lock = cifs_lock,
> @@ -770,7 +836,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = {
>  	.read = do_sync_read,
>  	.write = do_sync_write,
>  	.aio_read = generic_file_aio_read,
> -	.aio_write = cifs_file_aio_write,
> +	.aio_write = cifs_strict_write,
>  	.open = cifs_open,
>  	.release = cifs_close,
>  	.fsync = cifs_strict_fsync,
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index e6d1481..3a337a9 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -79,6 +79,9 @@ extern int checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length);
>  extern bool is_valid_oplock_break(struct smb_hdr *smb,
>  				  struct TCP_Server_Info *);
>  extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof);
> +extern ssize_t cifs_write(struct cifsFileInfo *open_file,
> +			  const char *write_data, size_t write_size,
> +			  loff_t *poffset);
>  extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool);
>  extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
>  extern unsigned int smbCalcSize(struct smb_hdr *ptr);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index dd1e59a..2c848ef 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1026,9 +1026,8 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
>  	return total_written;
>  }
>  
> -static ssize_t cifs_write(struct cifsFileInfo *open_file,
> -			  const char *write_data, size_t write_size,
> -			  loff_t *poffset)
> +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;


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] CIFS: Implement cifs_strict_write
       [not found]         ` <20101214084053.0da711a2-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-12-14 14:01           ` Pavel Shilovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Pavel Shilovsky @ 2010-12-14 14:01 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

2010/12/14 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> On Tue, 14 Dec 2010 14:10:58 +0300
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> If we don't have Exclusive oplock we write a data to the server through
>> cifs_write. Also set invalidate_mapping flag on the inode if we wrote
>> something to the server.
>>
>> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  fs/cifs/cifsfs.c    |   74 ++++++++++++++++++++++++++++++++++++++++++++++++---
>>  fs/cifs/cifsproto.h |    3 ++
>>  fs/cifs/file.c      |    5 +--
>>  3 files changed, 75 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index a218afe..5df0503 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -572,15 +572,81 @@ cifs_do_mount(struct file_system_type *fs_type,
>>       return dget(sb->s_root);
>>  }
>>
>> +static ssize_t cifs_strict_write(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 written = 0;
>> +     unsigned long i, len = 0;
>> +     char *buf, *offset;
>> +
>> +     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);
>> +
>> +     /*
>> +      * 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.
>> +      */
>> +
>> +     for (i = 0; i < nr_segs; i++)
>> +             len += iov[i].iov_len;
>> +
>> +     /*
>> +      * BB - optimize the way when signing is disabled. We can drop this
>> +      * extra memory-to-memory copying and use iovec buffers for constructing
>> +      * write request.
>> +      */
>> +
>> +     buf = kmalloc(len, GFP_KERNEL);
>> +
>
> Actually...I thought about another problem here. Suppose an application
> does a 100M write. This may fail in that case. It varies by the slab
> allocator in use, but there are limits to how large you can kmalloc.
> This is especially going to be a problem on 32-bit arches where
> GFP_KERNEL memory is more limited.
>
> There are a couple of different solutions, but I think the best one
> would be to stitch together individual pages. Walk the iov array first
> and figure out how many pages you're going to need, and kmalloc an iov
> array that size. Then start allocating pages (preferably using
> GFP_HIGHMEM and kmapping them) and then copy the data to them. Stop
> when you get to the wsize limit, and issue a CIFSSMBWrite2 call. When
> it returns, pick up where you left off and do it again.

May be I should allocate buffer of min(wsize, write_len) size, fill it
with a data to write on cycle and issue a cifs_write with this buffer.

1) get write data len into 'len'
2) buf = kmalloc(min(wsize, len), GFP_KERNEL)
3) last_ind = 0, last_len = iov[0].iov_len
4) while (len > 0){
      cur_len = min(len, last_len)
      memcpy(buf, iov[last_ind].iov_base + iov[last_ind].iov_len -
last_len, cur_len)
      len -= cur_len
      last_len -= cur_len
      if (last_len == 0) {
        last_ind++
        last_len = iov[last_ind].iov_len
      }
      cifs_write(buf)
   }

So - it is like the way I do in read part (cifs_fill_iov). Thoughts?

>
>> +     for (i = 0, offset = buf; i < nr_segs; i++) {
>> +             if (copy_from_user(offset, iov[i].iov_base, iov[i].iov_len)) {
>> +                     kfree(buf);
>> +                     return -EFAULT;
>> +             }
>> +
>> +             offset += iov[i].iov_len;
>> +     }
>> +
>> +     written = cifs_write((struct cifsFileInfo *)iocb->ki_filp->private_data,
>> +                          buf, len, &pos);
>> +     if (written < 0) {
>> +             kfree(buf);
>> +             return written;
>> +     }
>> +
>> +     iocb->ki_pos = pos;
>> +
>> +     if (written > 0)
>> +             CIFS_I(inode)->invalid_mapping = true;
>> +
>> +     kfree(buf);
>> +     return written;
>> +}
>> +
>>  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;
>>       ssize_t written;
>> +     int rc;
>>
>>       written = generic_file_aio_write(iocb, iov, nr_segs, pos);
>> -     if (!CIFS_I(inode)->clientCanCacheAll)
>> -             filemap_fdatawrite(inode->i_mapping);
>> +
>> +     if (CIFS_I(inode)->clientCanCacheAll)
>> +             return written;
>> +
>> +     rc = filemap_fdatawrite(inode->i_mapping);
>> +     if (rc)
>> +             cFYI(1, "cifs_file_aio_write: %d rc on %p inode", rc, inode);
>> +
>>       return written;
>>  }
>>
>> @@ -714,7 +780,7 @@ const struct file_operations cifs_file_strict_ops = {
>>       .read = do_sync_read,
>>       .write = do_sync_write,
>>       .aio_read = generic_file_aio_read,
>> -     .aio_write = cifs_file_aio_write,
>> +     .aio_write = cifs_strict_write,
>>       .open = cifs_open,
>>       .release = cifs_close,
>>       .lock = cifs_lock,
>> @@ -770,7 +836,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = {
>>       .read = do_sync_read,
>>       .write = do_sync_write,
>>       .aio_read = generic_file_aio_read,
>> -     .aio_write = cifs_file_aio_write,
>> +     .aio_write = cifs_strict_write,
>>       .open = cifs_open,
>>       .release = cifs_close,
>>       .fsync = cifs_strict_fsync,
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index e6d1481..3a337a9 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -79,6 +79,9 @@ extern int checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length);
>>  extern bool is_valid_oplock_break(struct smb_hdr *smb,
>>                                 struct TCP_Server_Info *);
>>  extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof);
>> +extern ssize_t cifs_write(struct cifsFileInfo *open_file,
>> +                       const char *write_data, size_t write_size,
>> +                       loff_t *poffset);
>>  extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool);
>>  extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
>>  extern unsigned int smbCalcSize(struct smb_hdr *ptr);
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index dd1e59a..2c848ef 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -1026,9 +1026,8 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
>>       return total_written;
>>  }
>>
>> -static ssize_t cifs_write(struct cifsFileInfo *open_file,
>> -                       const char *write_data, size_t write_size,
>> -                       loff_t *poffset)
>> +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;
>
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>



-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] CIFS: Implement cifs_strict_write
       [not found]           ` <AANLkTinHKWHff8TzWapRLtX-sA4FK98Ntt+a_Fv8Z5f_-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-12-14 14:02             ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 14:02 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 16:56:03 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> 2010/12/14 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> > On Tue, 14 Dec 2010 14:10:58 +0300
> > Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > > If we don't have Exclusive oplock we write a data to the server through
> > > cifs_write. Also set invalidate_mapping flag on the inode if we wrote
> > > something to the server.
> > >
> > > Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > ---
> > >  fs/cifs/cifsfs.c    |   74
> > ++++++++++++++++++++++++++++++++++++++++++++++++---
> > >  fs/cifs/cifsproto.h |    3 ++
> > >  fs/cifs/file.c      |    5 +--
> > >  3 files changed, 75 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > > index a218afe..5df0503 100644
> > > --- a/fs/cifs/cifsfs.c
> > > +++ b/fs/cifs/cifsfs.c
> > > @@ -572,15 +572,81 @@ cifs_do_mount(struct file_system_type *fs_type,
> > >       return dget(sb->s_root);
> > >  }
> > >
> > > +static ssize_t cifs_strict_write(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 written = 0;
> > > +     unsigned long i, len = 0;
> > > +     char *buf, *offset;
> > > +
> > > +     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);
> > > +
> > > +     /*
> > > +      * 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.
> > > +      */
> > > +
> > > +     for (i = 0; i < nr_segs; i++)
> > > +             len += iov[i].iov_len;
> > > +
> > > +     /*
> > > +      * BB - optimize the way when signing is disabled. We can drop this
> > > +      * extra memory-to-memory copying and use iovec buffers for
> > constructing
> > > +      * write request.
> > > +      */
> > > +
> > > +     buf = kmalloc(len, GFP_KERNEL);
> > > +
> >
> > Actually...I thought about another problem here. Suppose an application
> > does a 100M write. This may fail in that case. It varies by the slab
> > allocator in use, but there are limits to how large you can kmalloc.
> > This is especially going to be a problem on 32-bit arches where
> > GFP_KERNEL memory is more limited.
> >
> > There are a couple of different solutions, but I think the best one
> > would be to stitch together individual pages. Walk the iov array first
> > and figure out how many pages you're going to need, and kmalloc an iov
> > array that size. Then start allocating pages (preferably using
> > GFP_HIGHMEM and kmapping them) and then copy the data to them. Stop
> > when you get to the wsize limit, and issue a CIFSSMBWrite2 call. When
> > it returns, pick up where you left off and do it again.
> >
> 
> May be I should allocate buffer of min(wsize, write_len) size, fill it with
> a data to write on cycle and issue a cifs_write with this buffer.
> 
> 1) get write data len into 'len'
> 2) buf = kmalloc(min(wsize, len), GFP_KERNEL)
> 3) last_ind = 0, last_len = iov[0].iov_len
> 4) while (len > 0){
>       cur_len = min(len, last_len)
>       memcpy(buf, iov[last_ind].iov_base + iov[last_ind].iov_len - last_len,
> cur_len)
>       len -= cur_len
>       last_len -= cur_len
>       if (last_len == 0) {
>         last_ind++
>         last_len = iov[last_ind].iov_len
>       }
>       cifs_write(buf)
>    }
> 
> So - it is like the way I do in read part (cifs_fill_iov). Thoughts?
> 

That will work, assuming that you can kmalloc a buffer that size. It's
less of a problem than it used to be, but when you kmalloc you're
asking for contiguous pages. In order to satisfy that, the VM may have
to flush other pages if there is memory pressure.

Large memory allocations in the write codepath are generally a bad
idea. Individual page allocations (particularly from GFP_HIGHMEM) are
almost always able to be satisfied without needing to do things like
flush other stuff out of pagecache.

Also by doing it the way I'm suggesting, you'll eliminate an unneeded
copy of the data too.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/6] CIFS: Implement cifs_strict_read
       [not found]     ` <1292325060-8828-5-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 14:46       ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 14:46 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 14:10:59 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Read from the cache if we have at least Level II oplock - otherwise
> read from the server. Add cifs_user_readv to let the client read into
> several buffers.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c |   43 +++++++++++++++++++++++++++++-
>  fs/cifs/cifsfs.h |    5 +++-
>  fs/cifs/file.c   |   76 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 111 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 5df0503..a47b8af 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -572,6 +572,45 @@ cifs_do_mount(struct file_system_type *fs_type,
>  	return dget(sb->s_root);
>  }
>  
> +static ssize_t cifs_strict_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 = 0;
> +	unsigned long i, len = 0;
> +
> +	if (!nr_segs)
> +		return read;
> +
> +	inode = iocb->ki_filp->f_path.dentry->d_inode;
> +	cifs_sb = CIFS_SB(iocb->ki_filp->f_path.dentry->d_sb);
> +
> +	if (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.
> +	 */
> +
> +	for (i = 0; i < nr_segs; i++)
> +		len += iov[i].iov_len;
> +
> +
> +	read = cifs_user_readv(iocb->ki_filp, iov, nr_segs, len, &pos);
> +	if (read < 0)
> +		return read;
> +
> +	iocb->ki_pos = pos;
> +
> +	return read;
> +}
> +
>  static ssize_t cifs_strict_write(struct kiocb *iocb, const struct iovec *iov,
>  				 unsigned long nr_segs, loff_t pos)
>  {
> @@ -779,7 +818,7 @@ const struct file_operations cifs_file_ops = {
>  const struct file_operations cifs_file_strict_ops = {
>  	.read = do_sync_read,
>  	.write = do_sync_write,
> -	.aio_read = generic_file_aio_read,
> +	.aio_read = cifs_strict_read,
>  	.aio_write = cifs_strict_write,
>  	.open = cifs_open,
>  	.release = cifs_close,
> @@ -835,7 +874,7 @@ const struct file_operations cifs_file_nobrl_ops = {
>  const struct file_operations cifs_file_strict_nobrl_ops = {
>  	.read = do_sync_read,
>  	.write = do_sync_write,
> -	.aio_read = generic_file_aio_read,
> +	.aio_read = cifs_strict_read,
>  	.aio_write = cifs_strict_write,
>  	.open = cifs_open,
>  	.release = cifs_close,
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index 657738f..a6bea06 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -81,7 +81,10 @@ extern int cifs_open(struct inode *inode, struct file *file);
>  extern int cifs_close(struct inode *inode, struct file *file);
>  extern int cifs_closedir(struct inode *inode, struct file *file);
>  extern ssize_t cifs_user_read(struct file *file, char __user *read_data,
> -			 size_t read_size, loff_t *poffset);
> +			      size_t read_size, loff_t *poffset);
> +extern ssize_t cifs_user_readv(struct file *file, const struct iovec *iov,
> +			       unsigned long nr_segs, size_t len,
> +			       loff_t *poffset);
>  extern ssize_t cifs_user_write(struct file *file, const char __user *write_data,
>  			 size_t write_size, loff_t *poffset);
>  extern int cifs_lock(struct file *, int, struct file_lock *);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 2c848ef..9238a2f 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1676,8 +1676,40 @@ int cifs_flush(struct file *file, fl_owner_t id)
>  	return rc;
>  }
>  
> -ssize_t cifs_user_read(struct file *file, char __user *read_data,
> -	size_t read_size, loff_t *poffset)
> +static int
> +cifs_fill_iov(const struct iovec *iov, unsigned long nr_segs,
> +	      unsigned long *last_len, unsigned long *last_ind, char *data,
> +	      size_t datalen)
> +{
> +	const struct iovec *ciov;
> +	unsigned long cur_len;
> +
> +	while (datalen) {
> +		cur_len = min_t(unsigned long, datalen,
> +					      *last_len);
> +		ciov = &iov[*last_ind];
> +
> +		if (copy_to_user(ciov->iov_base + (ciov->iov_len - *last_len),
> +				 data, cur_len))
> +			return -EFAULT;
> +
> +		datalen -= cur_len;
> +		data += cur_len;
> +
> +		*last_len -= cur_len;
> +		if (*last_len == 0) {
> +			(*last_ind)++;
> +			*last_len = iov[*last_ind].iov_len;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t
> +cifs_generic_user_read(struct file *file, char __user *read_data,
> +		       size_t read_size, loff_t *poffset,
> +		       const struct iovec *iov, unsigned long nr_segs)
>  {
>  	int rc = -EACCES;
>  	unsigned int bytes_read = 0;
> @@ -1690,6 +1722,11 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
>  	char *smb_read_data;
>  	char __user *current_offset;
>  	struct smb_com_read_rsp *pSMBr;
> +	unsigned long last_len = 0, last_ind = 0;
> +
> +	/* if vectored - set last_len value */
> +	if (!read_data && nr_segs > 0)
> +		last_len = iov->iov_len;
>  
	^^^^
cifs_user_read can just be the trivial case of cifs_user_readv. It just
has a nr_segs of 1.

Rather than do this, it would probably be cleaner to just drop the
"read_data" and "read_size" parms from this function. Then make
cifs_user_read a wrapper around the cifs_user_readv that declares a
single iovec and passes in nr_segs of 1.

See smb_send and smb_sendv for an example of what I mean.


>  	xid = GetXid();
>  	cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> @@ -1707,7 +1744,8 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
>  
>  	for (total_read = 0, current_offset = read_data;
>  	     read_size > total_read;
> -	     total_read += bytes_read, current_offset += bytes_read) {
> +	     total_read += bytes_read,
> +	     current_offset += read_data ? bytes_read : 0) {
>  		current_read_size = min_t(const int, read_size - total_read,
>  					  cifs_sb->rsize);
>  		rc = -EAGAIN;
> @@ -1726,12 +1764,18 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
>  					 &buf_type);
>  			pSMBr = (struct smb_com_read_rsp *)smb_read_data;
>  			if (smb_read_data) {
> -				if (copy_to_user(current_offset,
> -						smb_read_data +
> -						4 /* RFC1001 length field */ +
> -						le16_to_cpu(pSMBr->DataOffset),
> -						bytes_read))
> -					rc = -EFAULT;
> +				char *data_offset = smb_read_data + 4 +
> +						le16_to_cpu(pSMBr->DataOffset);
> +				if (read_data) {
> +					if (copy_to_user(current_offset,
> +						data_offset, bytes_read))
> +						rc = -EFAULT;
> +				} else {
> +					if (cifs_fill_iov(iov, nr_segs,
> +						&last_len, &last_ind,
> +						data_offset, bytes_read))
> +						rc = -EFAULT;
> +				}
>  
>  				if (buf_type == CIFS_SMALL_BUFFER)
>  					cifs_small_buf_release(smb_read_data);
> @@ -1756,9 +1800,21 @@ ssize_t cifs_user_read(struct file *file, char __user *read_data,
>  	return total_read;
>  }
>  
> +ssize_t cifs_user_read(struct file *file, char __user *read_data,
> +		       size_t read_size, loff_t *poffset)
> +{
> +	return cifs_generic_user_read(file, read_data, read_size, poffset, NULL,
> +				      0);
> +}
> +
> +ssize_t cifs_user_readv(struct file *file, const struct iovec *iov,
> +			unsigned long nr_segs, size_t len, loff_t *poffset)
> +{
> +	return cifs_generic_user_read(file, NULL, len, poffset, iov, nr_segs);
> +}
>  
>  static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
> -	loff_t *poffset)
> +			 loff_t *poffset)
>  {
>  	int rc = -EACCES;
>  	unsigned int bytes_read = 0;


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/6] CIFS: Implement cifs_file_strict_mmap
       [not found]     ` <1292325060-8828-3-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 14:47       ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 14:47 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 14:10:57 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Invalidate inode mapping if we don't have at least Level II oplock.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c |    4 ++--
>  fs/cifs/cifsfs.h |    1 +
>  fs/cifs/file.c   |   17 +++++++++++++++++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 7660133..a218afe 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -720,7 +720,7 @@ const struct file_operations cifs_file_strict_ops = {
>  	.lock = cifs_lock,
>  	.fsync = cifs_strict_fsync,
>  	.flush = cifs_flush,
> -	.mmap = cifs_file_mmap,
> +	.mmap = cifs_file_strict_mmap,
>  	.splice_read = generic_file_splice_read,
>  	.llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_POSIX
> @@ -775,7 +775,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = {
>  	.release = cifs_close,
>  	.fsync = cifs_strict_fsync,
>  	.flush = cifs_flush,
> -	.mmap = cifs_file_mmap,
> +	.mmap = cifs_file_strict_mmap,
>  	.splice_read = generic_file_splice_read,
>  	.llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_POSIX
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index 8584829..657738f 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -89,6 +89,7 @@ extern int cifs_fsync(struct file *, int);
>  extern int cifs_strict_fsync(struct file *, int);
>  extern int cifs_flush(struct file *, fl_owner_t id);
>  extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
> +extern int cifs_file_strict_mmap(struct file * , struct vm_area_struct *);
>  extern const struct file_operations cifs_dir_ops;
>  extern int cifs_dir_open(struct inode *inode, struct file *file);
>  extern int cifs_readdir(struct file *file, void *direntry, filldir_t filldir);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 4e37cb8..dd1e59a 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1827,17 +1827,34 @@ static ssize_t cifs_read(struct file *file, char *read_data, size_t read_size,
>  	return total_read;
>  }
>  
> +int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	int rc, xid;
> +	struct inode *inode = file->f_path.dentry->d_inode;
> +
> +	xid = GetXid();
> +
> +	if (!CIFS_I(inode)->clientCanCacheRead)
> +		cifs_invalidate_mapping(inode);
> +
> +	rc = generic_file_mmap(file, vma);
> +	FreeXid(xid);
> +	return rc;
> +}
> +
>  int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	int rc, xid;
>  
>  	xid = GetXid();
> +
>  	rc = cifs_revalidate_file(file);
>  	if (rc) {
>  		cFYI(1, "Validation prior to mmap failed, error=%d", rc);
>  		FreeXid(xid);
>  		return rc;
>  	}
> +
	^^^^
Slip of the return key? I don't think we need these extra newlines.

>  	rc = generic_file_mmap(file, vma);
>  	FreeXid(xid);
>  	return rc;


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] CIFS: Implement cifs_strict_fsync
       [not found]     ` <1292325060-8828-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 14:49       ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 14:49 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 14:10:56 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Invalidate inode mapping if we don't have at least Level II oplock in
> cifs_strict_fsync. Also remove filemap_write_and_wait call from cifs_fsync
> because it is previously called from vfs_fsync_range. Add file operations'
> structures for strict cache mode.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c |   38 ++++++++++++++++++++++++++++++++++++++
>  fs/cifs/cifsfs.h |    8 ++++++--
>  fs/cifs/file.c   |   36 ++++++++++++++++++++++++++++--------
>  fs/cifs/inode.c  |    8 ++++++--
>  4 files changed, 78 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 3936aa7..7660133 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -710,6 +710,25 @@ const struct file_operations cifs_file_ops = {
>  	.setlease = cifs_setlease,
>  };
>  
> +const struct file_operations cifs_file_strict_ops = {
> +	.read = do_sync_read,
> +	.write = do_sync_write,
> +	.aio_read = generic_file_aio_read,
> +	.aio_write = cifs_file_aio_write,
> +	.open = cifs_open,
> +	.release = cifs_close,
> +	.lock = cifs_lock,
> +	.fsync = cifs_strict_fsync,
> +	.flush = cifs_flush,
> +	.mmap = cifs_file_mmap,
> +	.splice_read = generic_file_splice_read,
> +	.llseek = cifs_llseek,
> +#ifdef CONFIG_CIFS_POSIX
> +	.unlocked_ioctl	= cifs_ioctl,
> +#endif /* CONFIG_CIFS_POSIX */
> +	.setlease = cifs_setlease,
> +};
> +
>  const struct file_operations cifs_file_direct_ops = {
>  	/* no aio, no readv -
>  	   BB reevaluate whether they can be done with directio, no cache */
> @@ -728,6 +747,7 @@ const struct file_operations cifs_file_direct_ops = {
>  	.llseek = cifs_llseek,
>  	.setlease = cifs_setlease,
>  };
> +
>  const struct file_operations cifs_file_nobrl_ops = {
>  	.read = do_sync_read,
>  	.write = do_sync_write,
> @@ -746,6 +766,24 @@ const struct file_operations cifs_file_nobrl_ops = {
>  	.setlease = cifs_setlease,
>  };
>  
> +const struct file_operations cifs_file_strict_nobrl_ops = {
> +	.read = do_sync_read,
> +	.write = do_sync_write,
> +	.aio_read = generic_file_aio_read,
> +	.aio_write = cifs_file_aio_write,
> +	.open = cifs_open,
> +	.release = cifs_close,
> +	.fsync = cifs_strict_fsync,
> +	.flush = cifs_flush,
> +	.mmap = cifs_file_mmap,
> +	.splice_read = generic_file_splice_read,
> +	.llseek = cifs_llseek,
> +#ifdef CONFIG_CIFS_POSIX
> +	.unlocked_ioctl	= cifs_ioctl,
> +#endif /* CONFIG_CIFS_POSIX */
> +	.setlease = cifs_setlease,
> +};
> +
>  const struct file_operations cifs_file_direct_nobrl_ops = {
>  	/* no mmap, no aio, no readv -
>  	   BB reevaluate whether they can be done with directio, no cache */
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index 897b2b2..8584829 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -61,6 +61,7 @@ extern int cifs_rename(struct inode *, struct dentry *, struct inode *,
>  		       struct dentry *);
>  extern int cifs_revalidate_file(struct file *filp);
>  extern int cifs_revalidate_dentry(struct dentry *);
> +extern void cifs_invalidate_mapping(struct inode *inode);
>  extern int cifs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
>  extern int cifs_setattr(struct dentry *, struct iattr *);
>  
> @@ -72,8 +73,10 @@ extern const struct inode_operations cifs_dfs_referral_inode_operations;
>  /* Functions related to files and directories */
>  extern const struct file_operations cifs_file_ops;
>  extern const struct file_operations cifs_file_direct_ops; /* if directio mnt */
> -extern const struct file_operations cifs_file_nobrl_ops;
> -extern const struct file_operations cifs_file_direct_nobrl_ops; /* no brlocks */
> +extern const struct file_operations cifs_file_strict_ops; /* if strictio mnt */
> +extern const struct file_operations cifs_file_nobrl_ops; /* no brlocks */
> +extern const struct file_operations cifs_file_direct_nobrl_ops;
> +extern const struct file_operations cifs_file_strict_nobrl_ops;
>  extern int cifs_open(struct inode *inode, struct file *file);
>  extern int cifs_close(struct inode *inode, struct file *file);
>  extern int cifs_closedir(struct inode *inode, struct file *file);
> @@ -83,6 +86,7 @@ extern ssize_t cifs_user_write(struct file *file, const char __user *write_data,
>  			 size_t write_size, loff_t *poffset);
>  extern int cifs_lock(struct file *, int, struct file_lock *);
>  extern int cifs_fsync(struct file *, int);
> +extern int cifs_strict_fsync(struct file *, int);
>  extern int cifs_flush(struct file *, fl_owner_t id);
>  extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
>  extern const struct file_operations cifs_dir_ops;
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 60c3f84..4e37cb8 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1586,27 +1586,47 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
>  	return rc;
>  }
>  
> -int cifs_fsync(struct file *file, int datasync)
> +int cifs_strict_fsync(struct file *file, int datasync)
>  {
>  	int xid;
>  	int rc = 0;
>  	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_invalidate_mapping(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;
> +}
> +
> +int cifs_fsync(struct file *file, int datasync)
> +{
> +	int xid;
> +	int rc = 0;
> +	struct cifsTconInfo *tcon;
> +	struct cifsFileInfo *smbfile = file->private_data;
> +	struct cifs_sb_info *cifs_sb = CIFS_SB(file->f_path.dentry->d_sb);
> +
> +	xid = GetXid();
> +
> +	cFYI(1, "Sync file - name: %s datasync: 0x%x",
> +		file->f_path.dentry->d_name.name, datasync);
> +
> +	tcon = tlink_tcon(smbfile->tlink);
> +	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC))
> +		rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
>  
>  	FreeXid(xid);
>  	return rc;
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 589f3e3..a8f7cf3 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -44,13 +44,17 @@ static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
>  				inode->i_fop = &cifs_file_direct_nobrl_ops;
>  			else
>  				inode->i_fop = &cifs_file_direct_ops;
> +		} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO) {
> +			if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
> +				inode->i_fop = &cifs_file_strict_nobrl_ops;
> +			else
> +				inode->i_fop = &cifs_file_strict_ops;
>  		} else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
>  			inode->i_fop = &cifs_file_nobrl_ops;
>  		else { /* not direct, send byte range locks */
>  			inode->i_fop = &cifs_file_ops;
>  		}
>  
> -
>  		/* check if server can support readpages */
>  		if (cifs_sb_master_tcon(cifs_sb)->ses->server->maxBuf <
>  				PAGE_CACHE_SIZE + MAX_CIFS_HDR_SIZE)
> @@ -1679,7 +1683,7 @@ cifs_inode_needs_reval(struct inode *inode)
>  /*
>   * Zap the cache. Called when invalid_mapping flag is set.
>   */
> -static void
> +void
>  cifs_invalidate_mapping(struct inode *inode)
>  {
>  	int rc;

Looks good to me.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 6/6] CIFS: Add strictcache mount option
       [not found]     ` <1292325060-8828-6-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-12-14 14:50       ` Jeff Layton
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2010-12-14 14:50 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 14 Dec 2010 14:11:00 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Use for switching on strict cache mode. In this mode the
> client reads from the cache all the time it has Oplock Level II,
> otherwise - read from the server. As for write - the client stores
> a data in the cache in Exclusive Oplock case, otherwise - write
> directly 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 46af99a..fe16835 100644
> --- a/fs/cifs/README
> +++ b/fs/cifs/README
> @@ -452,6 +452,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 cc1a860..877cb9e 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;
> @@ -1357,6 +1358,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 "
> @@ -2614,6 +2617,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;

Looks reasonable, but shouldn't be merged until the previous patches
have been.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-12-14 14:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-14 11:10 [PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode Pavel Shilovsky
     [not found] ` <1292325060-8828-1-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 11:10   ` [PATCH 2/6] CIFS: Implement cifs_strict_fsync Pavel Shilovsky
     [not found]     ` <1292325060-8828-2-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 14:49       ` Jeff Layton
2010-12-14 11:10   ` [PATCH 3/6] CIFS: Implement cifs_file_strict_mmap Pavel Shilovsky
     [not found]     ` <1292325060-8828-3-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 14:47       ` Jeff Layton
2010-12-14 11:10   ` [PATCH 4/6] CIFS: Implement cifs_strict_write Pavel Shilovsky
     [not found]     ` <1292325060-8828-4-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 13:40       ` Jeff Layton
     [not found]         ` <20101214084053.0da711a2-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-12-14 14:01           ` Pavel Shilovsky
     [not found]         ` <AANLkTinHKWHff8TzWapRLtX-sA4FK98Ntt+a_Fv8Z5f_@mail.gmail.com>
     [not found]           ` <AANLkTinHKWHff8TzWapRLtX-sA4FK98Ntt+a_Fv8Z5f_-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-12-14 14:02             ` Jeff Layton
2010-12-14 11:10   ` [PATCH 5/6] CIFS: Implement cifs_strict_read Pavel Shilovsky
     [not found]     ` <1292325060-8828-5-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 14:46       ` Jeff Layton
2010-12-14 11:11   ` [PATCH 6/6] CIFS: Add strictcache mount option Pavel Shilovsky
     [not found]     ` <1292325060-8828-6-git-send-email-piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-12-14 14:50       ` Jeff Layton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.