[PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Horst Reiterer]

Hi,

In contrast to the now-obsolete smbfs, cifs does not send SMB_COM_FLUSH
in response to an explicit fsync(2) to guarantee that all volatile data
is written to stable storage on the server side, provided the server
honors the request (which, to my knowledge, is true for Windows and
Samba with 'strict sync' enabled).
This patch modifies the cifs_fsync implementation to restore the
fsync-behavior of smbfs by triggering SMB_COM_FLUSH after sending
outstanding data on the client side to the server.

I inquired about this issue in the linux-cifs-client@lists.samba.org
mailing list:

On Tue, Feb 10, 2009 at 12:35 AM, Jeff Layton <jlayton@redhat.com> wrote:
> Horst Reiterer wrote:
> > Why was the explicit SMB_COM_FLUSH dropped in the new implementation?
>
> It's not that it was "removed" per-se, just never implemented. Patches
> to add that capability would certainly be welcome.

I tested the patch with 2.6.28.6 and 2.6.18 (backported) on x86_64.
Please review and apply.

Horst.


Signed-off-by: Horst Reiterer <horst.reiterer@gmail.com>

diff -uprN linux-2.6.28.6/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
--- linux-2.6.28.6/fs/cifs/cifs_debug.c	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/cifs_debug.c	2009-02-20 00:42:18.000000000 +0100
@@ -340,6 +340,8 @@ static int cifs_stats_proc_show(struct s
 				seq_printf(m, "\nWrites: %d Bytes: %lld",
 					atomic_read(&tcon->num_writes),
 					(long long)(tcon->bytes_written));
+				seq_printf(m, "\nFlushes: %d",
+					atomic_read(&tcon->num_flushes));
 				seq_printf(m, "\nLocks: %d HardLinks: %d "
 					      "Symlinks: %d",
 					atomic_read(&tcon->num_locks),
diff -uprN linux-2.6.28.6/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
--- linux-2.6.28.6/fs/cifs/cifsglob.h	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/cifsglob.h	2009-02-20 00:42:18.000000000 +0100
@@ -246,6 +246,7 @@ struct cifsTconInfo {
 	atomic_t num_smbs_sent;
 	atomic_t num_writes;
 	atomic_t num_reads;
+	atomic_t num_flushes;
 	atomic_t num_oplock_brks;
 	atomic_t num_opens;
 	atomic_t num_closes;
diff -uprN linux-2.6.28.6/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
--- linux-2.6.28.6/fs/cifs/cifspdu.h	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/cifspdu.h	2009-02-20 00:42:18.000000000 +0100
@@ -43,6 +43,7 @@
 #define SMB_COM_CREATE_DIRECTORY      0x00 /* trivial response */
 #define SMB_COM_DELETE_DIRECTORY      0x01 /* trivial response */
 #define SMB_COM_CLOSE                 0x04 /* triv req/rsp, timestamp ignored */
+#define SMB_COM_FLUSH                 0x05 /* triv req/rsp */
 #define SMB_COM_DELETE                0x06 /* trivial response */
 #define SMB_COM_RENAME                0x07 /* trivial response */
 #define SMB_COM_QUERY_INFORMATION     0x08 /* aka getattr */
@@ -790,6 +791,12 @@ typedef struct smb_com_close_rsp {
 	__u16 ByteCount;	/* bct = 0 */
 } __attribute__((packed)) CLOSE_RSP;

+typedef struct smb_com_flush_req {
+	struct smb_hdr hdr;	/* wct = 1 */
+	__u16 FileID;
+	__u16 ByteCount;	/* 0 */
+} __attribute__((packed)) FLUSH_REQ;
+
 typedef struct smb_com_findclose_req {
 	struct smb_hdr hdr; /* wct = 1 */
 	__u16 FileID;
diff -uprN linux-2.6.28.6/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
--- linux-2.6.28.6/fs/cifs/cifsproto.h	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/cifsproto.h	2009-02-20 00:42:18.000000000 +0100
@@ -277,6 +277,9 @@ extern int CIFSPOSIXCreate(const int xid
 extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon,
 			const int smb_file_id);

+extern int CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon,
+			const int smb_file_id);
+
 extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
 			const int netfid, unsigned int count,
 			const __u64 lseek, unsigned int *nbytes, char **buf,
diff -uprN linux-2.6.28.6/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
--- linux-2.6.28.6/fs/cifs/cifssmb.c	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/cifssmb.c	2009-02-20 00:42:18.000000000 +0100
@@ -1928,6 +1928,27 @@ CIFSSMBClose(const int xid, struct cifsT
 }

 int
+CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
+{
+	int rc = 0;
+	FLUSH_REQ *pSMB = NULL;
+	cFYI(1, ("In CIFSSMBFlush"));
+
+	rc = small_smb_init(SMB_COM_FLUSH, 1, tcon, (void **) &pSMB);
+	if (rc)
+		return rc;
+
+	pSMB->FileID = (__u16) smb_file_id;
+	pSMB->ByteCount = 0;
+	rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
+	cifs_stats_inc(&tcon->num_flushes);
+	if (rc)
+		cERROR(1, ("Send error in Flush = %d", rc));
+
+	return rc;
+}
+
+int
 CIFSSMBRename(const int xid, struct cifsTconInfo *tcon,
 	      const char *fromName, const char *toName,
 	      const struct nls_table *nls_codepage, int remap)
diff -uprN linux-2.6.28.6/fs/cifs/file.c b/fs/cifs/file.c
--- linux-2.6.28.6/fs/cifs/file.c	2009-02-17 18:29:27.000000000 +0100
+++ b/fs/cifs/file.c	2009-02-20 00:42:18.000000000 +0100
@@ -1522,18 +1522,31 @@ int cifs_fsync(struct file *file, struct
 {
 	int xid;
 	int rc = 0;
+	struct cifs_sb_info *cifs_sb;
+	struct cifsTconInfo *pTcon;
+	struct cifsFileInfo *pSMBFile =
+		(struct cifsFileInfo *)file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;

 	xid = GetXid();

+	cifs_sb = CIFS_SB(inode->i_sb);
+	pTcon = cifs_sb->tcon;
+
 	cFYI(1, ("Sync file - name: %s datasync: 0x%x",
 		dentry->d_name.name, datasync));

-	rc = filemap_write_and_wait(inode->i_mapping);
-	if (rc == 0) {
-		rc = CIFS_I(inode)->write_behind_rc;
-		CIFS_I(inode)->write_behind_rc = 0;
-	}
+	if (pSMBFile) {
+		rc = filemap_write_and_wait(inode->i_mapping);
+		if (rc == 0) {
+			rc = CIFS_I(inode)->write_behind_rc;
+			CIFS_I(inode)->write_behind_rc = 0;
+			if (pTcon)
+				rc = CIFSSMBFlush(xid, pTcon, pSMBFile->netfid);
+		}
+	} else
+		rc = -EBADF;
+
 	FreeXid(xid);
 	return rc;
 }

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

On Fri, Feb 20, 2009 at 2:51 PM, Horst Reiterer
<horst.reiterer@gmail.com> wrote:
> Hi,
>
> In contrast to the now-obsolete smbfs, cifs does not send SMB_COM_FLUSH
> in response to an explicit fsync(2) to guarantee that all volatile data
> is written to stable storage on the server side, provided the server
> honors the request (which, to my knowledge, is true for Windows and
> Samba with 'strict sync' enabled).
> This patch modifies the cifs_fsync implementation to restore the

Patch seems reasonable at first glance.  Will take a look in more
detail this weekend.



-- 
Thanks,

Steve

Re: [linux-cifs-client] [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Jeff Layton]

On Fri, 20 Feb 2009 21:51:46 +0100 (CET)
Horst Reiterer <horst.reiterer@gmail.com> wrote:

> Hi,
> 
> In contrast to the now-obsolete smbfs, cifs does not send SMB_COM_FLUSH
> in response to an explicit fsync(2) to guarantee that all volatile data
> is written to stable storage on the server side, provided the server
> honors the request (which, to my knowledge, is true for Windows and
> Samba with 'strict sync' enabled).
> This patch modifies the cifs_fsync implementation to restore the
> fsync-behavior of smbfs by triggering SMB_COM_FLUSH after sending
> outstanding data on the client side to the server.
> 
> I inquired about this issue in the linux-cifs-client@lists.samba.org
> mailing list:
> 
> On Tue, Feb 10, 2009 at 12:35 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > Horst Reiterer wrote:
> > > Why was the explicit SMB_COM_FLUSH dropped in the new implementation?
> >
> > It's not that it was "removed" per-se, just never implemented. Patches
> > to add that capability would certainly be welcome.
> 
> I tested the patch with 2.6.28.6 and 2.6.18 (backported) on x86_64.
> Please review and apply.
> 
> Horst.
> 
> 
> Signed-off-by: Horst Reiterer <horst.reiterer@gmail.com>
> 


Thanks for doing this. Comments below...

> diff -uprN linux-2.6.28.6/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> --- linux-2.6.28.6/fs/cifs/cifs_debug.c	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/cifs_debug.c	2009-02-20 00:42:18.000000000 +0100
> @@ -340,6 +340,8 @@ static int cifs_stats_proc_show(struct s
>  				seq_printf(m, "\nWrites: %d Bytes: %lld",
>  					atomic_read(&tcon->num_writes),
>  					(long long)(tcon->bytes_written));
> +				seq_printf(m, "\nFlushes: %d",
> +					atomic_read(&tcon->num_flushes));
>  				seq_printf(m, "\nLocks: %d HardLinks: %d "
>  					      "Symlinks: %d",
>  					atomic_read(&tcon->num_locks),
> diff -uprN linux-2.6.28.6/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> --- linux-2.6.28.6/fs/cifs/cifsglob.h	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/cifsglob.h	2009-02-20 00:42:18.000000000 +0100
> @@ -246,6 +246,7 @@ struct cifsTconInfo {
>  	atomic_t num_smbs_sent;
>  	atomic_t num_writes;
>  	atomic_t num_reads;
> +	atomic_t num_flushes;
>  	atomic_t num_oplock_brks;
>  	atomic_t num_opens;
>  	atomic_t num_closes;
> diff -uprN linux-2.6.28.6/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> --- linux-2.6.28.6/fs/cifs/cifspdu.h	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/cifspdu.h	2009-02-20 00:42:18.000000000 +0100
> @@ -43,6 +43,7 @@
>  #define SMB_COM_CREATE_DIRECTORY      0x00 /* trivial response */
>  #define SMB_COM_DELETE_DIRECTORY      0x01 /* trivial response */
>  #define SMB_COM_CLOSE                 0x04 /* triv req/rsp, timestamp ignored */
> +#define SMB_COM_FLUSH                 0x05 /* triv req/rsp */
>  #define SMB_COM_DELETE                0x06 /* trivial response */
>  #define SMB_COM_RENAME                0x07 /* trivial response */
>  #define SMB_COM_QUERY_INFORMATION     0x08 /* aka getattr */
> @@ -790,6 +791,12 @@ typedef struct smb_com_close_rsp {
>  	__u16 ByteCount;	/* bct = 0 */
>  } __attribute__((packed)) CLOSE_RSP;
> 
> +typedef struct smb_com_flush_req {
> +	struct smb_hdr hdr;	/* wct = 1 */
> +	__u16 FileID;
> +	__u16 ByteCount;	/* 0 */
> +} __attribute__((packed)) FLUSH_REQ;
> +
>  typedef struct smb_com_findclose_req {
>  	struct smb_hdr hdr; /* wct = 1 */
>  	__u16 FileID;
> diff -uprN linux-2.6.28.6/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> --- linux-2.6.28.6/fs/cifs/cifsproto.h	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/cifsproto.h	2009-02-20 00:42:18.000000000 +0100
> @@ -277,6 +277,9 @@ extern int CIFSPOSIXCreate(const int xid
>  extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon,
>  			const int smb_file_id);
> 
> +extern int CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon,
> +			const int smb_file_id);
> +
>  extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
>  			const int netfid, unsigned int count,
>  			const __u64 lseek, unsigned int *nbytes, char **buf,
> diff -uprN linux-2.6.28.6/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> --- linux-2.6.28.6/fs/cifs/cifssmb.c	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/cifssmb.c	2009-02-20 00:42:18.000000000 +0100
> @@ -1928,6 +1928,27 @@ CIFSSMBClose(const int xid, struct cifsT
>  }
> 
>  int
> +CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
> +{
> +	int rc = 0;
> +	FLUSH_REQ *pSMB = NULL;
> +	cFYI(1, ("In CIFSSMBFlush"));
> +
> +	rc = small_smb_init(SMB_COM_FLUSH, 1, tcon, (void **) &pSMB);
> +	if (rc)
> +		return rc;
> +
> +	pSMB->FileID = (__u16) smb_file_id;
> +	pSMB->ByteCount = 0;
> +	rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);

^^^
Seems to me like we should wait for the response here. If there's an error from the flush, shouldn't we report that to the caller of fsync()?

> +	cifs_stats_inc(&tcon->num_flushes);
> +	if (rc)
> +		cERROR(1, ("Send error in Flush = %d", rc));
> +
> +	return rc;
> +}
> +
> +int
>  CIFSSMBRename(const int xid, struct cifsTconInfo *tcon,
>  	      const char *fromName, const char *toName,
>  	      const struct nls_table *nls_codepage, int remap)
> diff -uprN linux-2.6.28.6/fs/cifs/file.c b/fs/cifs/file.c
> --- linux-2.6.28.6/fs/cifs/file.c	2009-02-17 18:29:27.000000000 +0100
> +++ b/fs/cifs/file.c	2009-02-20 00:42:18.000000000 +0100
> @@ -1522,18 +1522,31 @@ int cifs_fsync(struct file *file, struct
>  {
>  	int xid;
>  	int rc = 0;
> +	struct cifs_sb_info *cifs_sb;
> +	struct cifsTconInfo *pTcon;
> +	struct cifsFileInfo *pSMBFile =
> +		(struct cifsFileInfo *)file->private_data;
>  	struct inode *inode = file->f_path.dentry->d_inode;
> 
>  	xid = GetXid();
> 
> +	cifs_sb = CIFS_SB(inode->i_sb);
> +	pTcon = cifs_sb->tcon;
> +
>  	cFYI(1, ("Sync file - name: %s datasync: 0x%x",
>  		dentry->d_name.name, datasync));
> 
> -	rc = filemap_write_and_wait(inode->i_mapping);
> -	if (rc == 0) {
> -		rc = CIFS_I(inode)->write_behind_rc;
> -		CIFS_I(inode)->write_behind_rc = 0;
> -	}
> +	if (pSMBFile) {
> +		rc = filemap_write_and_wait(inode->i_mapping);
> +		if (rc == 0) {
> +			rc = CIFS_I(inode)->write_behind_rc;
> +			CIFS_I(inode)->write_behind_rc = 0;
> +			if (pTcon)
> +				rc = CIFSSMBFlush(xid, pTcon, pSMBFile->netfid);
> +		}
> +	} else
> +		rc = -EBADF;
> +
>  	FreeXid(xid);
>  	return rc;
>  }

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Fri, Feb 20, 2009 at 2:51 PM, Horst Reiterer
<horst.reiterer@gmail.com> wrote:
> Hi,
>
> In contrast to the now-obsolete smbfs, cifs does not send SMB_COM_FLUSH
> in response to an explicit fsync(2) to guarantee that all volatile data
> is written to stable storage on the server side, provided the server
> honors the request (which, to my knowledge, is true for Windows and
> Samba with 'strict sync' enabled).
> This patch modifies the cifs_fsync implementation

I modified your patch slightly to not lose the writeback rc in one
case, and to change camel case pTcon to tcon and remove one
unnecessary local variable.

See attached.   Thanks for the submission - looks fine otherwise.   If
you have any performance numbers before and after (with e.g. dbench,
iozone, bonnie etc. or perhaps something which calls fsync more often
- that would be helpful in determining whether we need a mount option
to optionally disable it - as the samba server does)


-- 
Thanks,

Steve

[-- Attachment #2: fsync.patch --]
[-- Type: text/x-diff, Size: 4794 bytes --]

diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index 851388f..d43e0fe 100644
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -6,7 +6,9 @@ the server to treat subsequent connections, especially those that
 are authenticated as guest, as reconnections, invalidating the earlier
 user's smb session.  This fix allows cifs to mount multiple times to the
 same server with different userids without risking invalidating earlier
-established security contexts.
+established security contexts.  fsync now sends SMB Flush operation
+to better ensure that we wait for server to write all of the data to
+server disk (not just write it over the network).
 
 Version 1.56
 ------------
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 490e34b..877e4d9 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -340,6 +340,8 @@ static int cifs_stats_proc_show(struct seq_file *m, void *v)
 				seq_printf(m, "\nWrites: %d Bytes: %lld",
 					atomic_read(&tcon->num_writes),
 					(long long)(tcon->bytes_written));
+				seq_printf(m, "\nFlushes: %d",
+					atomic_read(&tcon->num_flushes));
 				seq_printf(m, "\nLocks: %d HardLinks: %d "
 					      "Symlinks: %d",
 					atomic_read(&tcon->num_locks),
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index e004f6d..44ff94d 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -254,6 +254,7 @@ struct cifsTconInfo {
 	atomic_t num_smbs_sent;
 	atomic_t num_writes;
 	atomic_t num_reads;
+	atomic_t num_flushes;
 	atomic_t num_oplock_brks;
 	atomic_t num_opens;
 	atomic_t num_closes;
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index b4e2e9f..eda6e51 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -43,6 +43,7 @@
 #define SMB_COM_CREATE_DIRECTORY      0x00 /* trivial response */
 #define SMB_COM_DELETE_DIRECTORY      0x01 /* trivial response */
 #define SMB_COM_CLOSE                 0x04 /* triv req/rsp, timestamp ignored */
+#define SMB_COM_FLUSH                 0x05 /* triv req/rsp */
 #define SMB_COM_DELETE                0x06 /* trivial response */
 #define SMB_COM_RENAME                0x07 /* trivial response */
 #define SMB_COM_QUERY_INFORMATION     0x08 /* aka getattr */
@@ -790,6 +791,12 @@ typedef struct smb_com_close_rsp {
 	__u16 ByteCount;	/* bct = 0 */
 } __attribute__((packed)) CLOSE_RSP;
 
+typedef struct smb_com_flush_req {
+	struct smb_hdr hdr;	/* wct = 1 */
+	__u16 FileID;
+	__u16 ByteCount;	/* 0 */
+} __attribute__((packed)) FLUSH_REQ;
+
 typedef struct smb_com_findclose_req {
 	struct smb_hdr hdr; /* wct = 1 */
 	__u16 FileID;
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 083dfc5..596fc86 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -281,6 +281,9 @@ extern int CIFSPOSIXCreate(const int xid, struct cifsTconInfo *tcon,
 extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon,
 			const int smb_file_id);
 
+extern int CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon,
+			const int smb_file_id);
+
 extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
 			const int netfid, unsigned int count,
 			const __u64 lseek, unsigned int *nbytes, char **buf,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 939e2f7..4c344fe 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1934,6 +1934,27 @@ CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
 }
 
 int
+CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
+{
+	int rc = 0;
+	FLUSH_REQ *pSMB = NULL;
+	cFYI(1, ("In CIFSSMBFlush"));
+
+	rc = small_smb_init(SMB_COM_FLUSH, 1, tcon, (void **) &pSMB);
+	if (rc)
+		return rc;
+
+	pSMB->FileID = (__u16) smb_file_id;
+	pSMB->ByteCount = 0;
+	rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
+	cifs_stats_inc(&tcon->num_flushes);
+	if (rc)
+		cERROR(1, ("Send error in Flush = %d", rc));
+
+	return rc;
+}
+
+int
 CIFSSMBRename(const int xid, struct cifsTconInfo *tcon,
 	      const char *fromName, const char *toName,
 	      const struct nls_table *nls_codepage, int remap)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 12bb656..83b4741 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1523,6 +1523,9 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
 {
 	int xid;
 	int rc = 0;
+	struct cifsTconInfo *tcon;
+	struct cifsFileInfo *smbfile =
+		(struct cifsFileInfo *)file->private_data;
 	struct inode *inode = file->f_path.dentry->d_inode;
 
 	xid = GetXid();
@@ -1534,7 +1537,11 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
 	if (rc == 0) {
 		rc = CIFS_I(inode)->write_behind_rc;
 		CIFS_I(inode)->write_behind_rc = 0;
+		tcon = CIFS_SB(inode->i_sb)->tcon;
+		if (!rc && tcon && smbfile)
+			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 	}
+
 	FreeXid(xid);
 	return rc;
 }

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Horst Reiterer]

Hi,

On Fri, 20 Feb 2009, Steve French wrote:
> I modified your patch slightly to not lose the writeback rc in one
> case, and to change camel case pTcon to tcon and remove one
> unnecessary local variable.

Thanks.

> If you have any performance numbers before and after (with e.g. dbench,
> iozone, bonnie etc. or perhaps something which calls fsync more often
> - that would be helpful in determining whether we need a mount option
> to optionally disable it - as the samba server does)

I tested the patch using a simple test driver that transfers data in 1MB
chunks and calls fsync after each chunk. The results are as follows:

  Test run            Throughput (MB/s)
                      1 client (MB/s) 10 clients 20 clients 40 clients
  cifs - vanilla                   64        116        117        111
  cifs - fsync.patch               54         90         95         95

As you can see, fsync does make a noticeable difference here. The problem
with benchmarks in this respect is that the difference solely depends on
the underlying device. In this scenario, the storage device featured a
battery-backed disk cache. Without it, the overhead would have been much
more significant.

In any case, it's probably a good idea to introduce a mount option. The
new behavior should be enabled by default though, to provide maximum data
consistency. While I can't think of a legitimate reason to disable fsync
if data consistency is a concern, an option might be useful in the
exceptional case where 1) data consistency is not a strict requirement and
2) SMB_COM_FLUSH cannot be ignored on the server side.

Thanks,

Horst.

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

On Sun, Feb 22, 2009 at 2:50 PM, Horst Reiterer
<horst.reiterer@gmail.com> wrote:
> In any case, it's probably a good idea to introduce a mount option. The
> new behavior should be enabled by default though, to provide maximum data
> consistency.

Suggestions on what to call such a new mount option?  How about
"nostrictfsync"  ?

-- 
Thanks,

Steve

Fwd: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

Does the Linux nfs client (or other similar clients) have a mount
option to control fsync behavior?


---------- Forwarded message ----------
From: Steve French <smfrench@gmail.com>
Date: Sun, Feb 22, 2009 at 3:03 PM
Subject: Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync
To: Horst Reiterer <horst.reiterer@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-cifs-client@lists.samba.org


On Sun, Feb 22, 2009 at 2:50 PM, Horst Reiterer
<horst.reiterer@gmail.com> wrote:
> In any case, it's probably a good idea to introduce a mount option. The
> new behavior should be enabled by default though, to provide maximum data
> consistency.

Suggestions on what to call such a new mount option?  How about
"nostrictfsync"  ?

--
Thanks,

Steve



-- 
Thanks,

Steve

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Horst Reiterer]

On Sun, 22 Feb 2009, Steve French wrote:
> Suggestions on what to call such a new mount option?  How about
> "nostrictfsync"  ?

Sound good, should be self-explanatory for Samba users and those familiar
with the fsync concept.

Horst.

Re: Fwd: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Trond Myklebust]

On Sun, 2009-02-22 at 15:04 -0600, Steve French wrote:
> Does the Linux nfs client (or other similar clients) have a mount
> option to control fsync behavior?

Err??? What's the context here? I suppose that we do have '-onolock'
which turns off NFSv2/v3 remote locking.

Cheers
  Trond

> ---------- Forwarded message ----------
> From: Steve French <smfrench@gmail.com>
> Date: Sun, Feb 22, 2009 at 3:03 PM
> Subject: Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync
> To: Horst Reiterer <horst.reiterer@gmail.com>
> Cc: linux-kernel@vger.kernel.org, linux-cifs-client@lists.samba.org
> 
> 
> On Sun, Feb 22, 2009 at 2:50 PM, Horst Reiterer
> <horst.reiterer@gmail.com> wrote:
> > In any case, it's probably a good idea to introduce a mount option. The
> > new behavior should be enabled by default though, to provide maximum data
> > consistency.
> 
> Suggestions on what to call such a new mount option?  How about
> "nostrictfsync"  ?
> 
> --
> Thanks,
> 
> Steve
> 
> 
> 

Re: Fwd: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

On Sun, Feb 22, 2009 at 4:04 PM, Trond Myklebust
<trond.myklebust@fys.uio.no> wrote:
> On Sun, 2009-02-22 at 15:04 -0600, Steve French wrote:
>> Does the Linux nfs client (or other similar clients) have a mount
>> option to control fsync behavior?
>
> Err??? What's the context here? I suppose that we do have '-onolock'
> which turns off NFSv2/v3 remote locking.
>
> Cheers
>  Trond

CIFS implementation of fsync flushed the cache on the client
(filemap_fdatawait etc.), but did not send the SMB FLUSH operation to
the server requesting that the server write all data for this inode to
the metal on the server side (until now).   Some servers have a range
of configuration options to handle dumb applications that sync too
often (which is common on windows) or strange workloads e.g. "strict
sync = no" to "sync always" (in which the server issues fsync locally
after after SMB write is processed).

The suggestion was to add a mount option on the cifs client
("nostrictsync") to allow it to optionally just flush all of the
writes (and wait for write responses) but not force the server to sync
them to the metal (as a strict interpretation of sync would require).


>> ---------- Forwarded message ----------
>> From: Steve French <smfrench@gmail.com>
>> Date: Sun, Feb 22, 2009 at 3:03 PM
>> Subject: Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync
>> To: Horst Reiterer <horst.reiterer@gmail.com>
>> Cc: linux-kernel@vger.kernel.org, linux-cifs-client@lists.samba.org
>>
>>
>> On Sun, Feb 22, 2009 at 2:50 PM, Horst Reiterer
>> <horst.reiterer@gmail.com> wrote:
>> > In any case, it's probably a good idea to introduce a mount option. The
>> > new behavior should be enabled by default though, to provide maximum data
>> > consistency.
>>
>> Suggestions on what to call such a new mount option?  How about
>> "nostrictfsync"  ?



-- 
Thanks,

Steve

Re: [linux-cifs-client] Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Jeff Layton]

On Sun, 22 Feb 2009 22:12:11 +0100 (CET)
Horst Reiterer <horst.reiterer@gmail.com> wrote:

> On Sun, 22 Feb 2009, Steve French wrote:
> > Suggestions on what to call such a new mount option?  How about
> > "nostrictfsync"  ?
> 
> Sound good, should be self-explanatory for Samba users and those familiar
> with the fsync concept.
> 
> Horst.

My suggestion would be to not add a new option until someone requests
it/complains about it. We already have a lot of unneeded/unused mount
options, and I think this will just be adding one to the pile.

My $.02...

-- 
Jeff Layton <jlayton@redhat.com>

Re: Fwd: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Trond Myklebust]

On Sun, 2009-02-22 at 19:23 -0600, Steve French wrote:
> CIFS implementation of fsync flushed the cache on the client
> (filemap_fdatawait etc.), but did not send the SMB FLUSH operation to
> the server requesting that the server write all data for this inode to
> the metal on the server side (until now).   Some servers have a range
> of configuration options to handle dumb applications that sync too
> often (which is common on windows) or strange workloads e.g. "strict
> sync = no" to "sync always" (in which the server issues fsync locally
> after after SMB write is processed).
> 
> The suggestion was to add a mount option on the cifs client
> ("nostrictsync") to allow it to optionally just flush all of the
> writes (and wait for write responses) but not force the server to sync
> them to the metal (as a strict interpretation of sync would require).

In NFS we always enforce the strict rule that we COMMIT on close(), but
I do know that a lot of *BSD based implementations skip that step. As
long as the server doesn't reboot, and/or the file is not shared with
other clients, then that is a reasonable strategy.
That said, I do also agree that you might want to enforce stricter rules
by default, and then allow admins to relax them using a mount option.

Speaking of cache consistency, what about the ability to flush caches?
Is there any interest within the CIFS community to allow user
applications to invalidate the page cache and/or attribute caches when
no delegation/oplock is held and the app knows that a file or directory
may have changed on the server? It is an issue that keeps getting raised
on the NFS side, but that we haven't addressed yet.

Cheers
  Trond

Re: Fwd: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

On Sun, Feb 22, 2009 at 7:36 PM, Trond Myklebust
<trond.myklebust@fys.uio.no> wrote:
> On Sun, 2009-02-22 at 19:23 -0600, Steve French wrote:
>> CIFS implementation of fsync flushed the cache on the client
>> (filemap_fdatawait etc.), but did not send the SMB FLUSH operation to
>> the server requesting that the server write all data for this inode to
>> the metal on the server side (until now).   Some servers have a range
>> of configuration options to handle dumb applications that sync too
>> often (which is common on windows) or strange workloads e.g. "strict
>> sync = no" to "sync always" (in which the server issues fsync locally
>> after after SMB write is processed).
>>
>> The suggestion was to add a mount option on the cifs client
>> ("nostrictsync") to allow it to optionally just flush all of the
>> writes (and wait for write responses) but not force the server to sync
>> them to the metal (as a strict interpretation of sync would require).
>
> In NFS we always enforce the strict rule that we COMMIT on close(), but
> I do know that a lot of *BSD based implementations skip that step. As
> long as the server doesn't reboot, and/or the file is not shared with
> other clients, then that is a reasonable strategy.
> That said, I do also agree that you might want to enforce stricter rules
> by default, and then allow admins to relax them using a mount option.
>
> Speaking of cache consistency, what about the ability to flush caches?
> Is there any interest within the CIFS community to allow user
> applications to invalidate the page cache and/or attribute caches when
> no delegation/oplock is held and the app knows that a file or directory
> may have changed on the server? It is an issue that keeps getting raised
> on the NFS side, but that we haven't addressed yet.
That is an interesting question.   In cifs we already read through to
the server (rather than out of the page cache) if no read oplock is
held so I doubt that it would make much difference if we knew that the
server copy had changed (we would still read through to the server).
For the attribute cache it might make a difference though since we
cache file attributes (time stamps, mode etc.) for 1 second.

SMB/CIFS (and SMB2) protocol has a fairly useful set of protocol
operations (that we don't use) for file change notification (and
directory change notification) that we could use to do "loose cache
consistency" if someone wanted that (we can get notified by the server
when a non-oplocked inode is modified, or a directory is modified, so
could catch server side updates fairly quickly even when a file is not
strictly cacheable).

-- 
Thanks,

Steve

Re: [linux-cifs-client] Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Jeff Layton]

On Sun, 22 Feb 2009 17:34:46 -0800
Jeff Layton <jlayton@redhat.com> wrote:

> On Sun, 22 Feb 2009 22:12:11 +0100 (CET)
> Horst Reiterer <horst.reiterer@gmail.com> wrote:
> 
> > On Sun, 22 Feb 2009, Steve French wrote:
> > > Suggestions on what to call such a new mount option?  How about
> > > "nostrictfsync"  ?
> > 
> > Sound good, should be self-explanatory for Samba users and those familiar
> > with the fsync concept.
> > 
> > Horst.
> 
> My suggestion would be to not add a new option until someone requests
> it/complains about it. We already have a lot of unneeded/unused mount
> options, and I think this will just be adding one to the pile.
> 
> My $.02...
> 

...and to lend further strength to this argument, we're not doing this
at close(), but rather at fsync(). posix is pretty clear about what's
supposed to happen at fsync:

"The fsync() function shall request that all data for the open file
descriptor named by fildes is to be transferred to the storage device
associated with the file described by fildes."

...if you don't want to take the performance penalty then don't use
fsync(). If you're using fsync, then you care about data integrity
and adding a mount option to disable it is rather pointless.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [linux-cifs-client] Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

On Sun, Feb 22, 2009 at 8:10 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Sun, 22 Feb 2009 17:34:46 -0800
> Jeff Layton <jlayton@redhat.com> wrote:
>
>> On Sun, 22 Feb 2009 22:12:11 +0100 (CET)
>> Horst Reiterer <horst.reiterer@gmail.com> wrote:
>>
>> > On Sun, 22 Feb 2009, Steve French wrote:
>> > > Suggestions on what to call such a new mount option?  How about
>> > > "nostrictfsync"  ?
>> >
>> > Sound good, should be self-explanatory for Samba users and those familiar
>> > with the fsync concept.
>> >
>> > Horst.
>>
>> My suggestion would be to not add a new option until someone requests
>> it/complains about it. We already have a lot of unneeded/unused mount
>> options, and I think this will just be adding one to the pile.
>>
>> My $.02...
>>
>
> ...and to lend further strength to this argument, we're not doing this
> at close(), but rather at fsync(). posix is pretty clear about what's
> supposed to happen at fsync:
>
> "The fsync() function shall request that all data for the open file
> descriptor named by fildes is to be transferred to the storage device
> associated with the file described by fildes."
>
> ...if you don't want to take the performance penalty then don't use
> fsync(). If you're using fsync, then you care about data integrity
> and adding a mount option to disable it is rather pointless.

There is a lot of history here (especially in the man page of
smb.conf) on dumb things apps do in this area.  I don't object to
allowing users to turn strict sync behavior off.   There are some apps
who presumably consider getting data to the server (which we already
do in fsync) "stable enough" storage (clients are not battery backed
up, with redundant, highly available hardware as some servers are).
I agree that the default should be to write the data to the metal on
the server (even though this can be very, very expensive) for fsync

-- 
Thanks,

Steve

Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

On Sun, Feb 22, 2009 at 3:12 PM, Horst Reiterer
<horst.reiterer@gmail.com> wrote:
> On Sun, 22 Feb 2009, Steve French wrote:
>> Suggestions on what to call such a new mount option?  How about
>> "nostrictfsync"  ?
>
> Sound good, should be self-explanatory for Samba users and those familiar
> with the fsync concept.
>
> Horst.

Patch attached for nostrictfsync cifs mount option



-- 
Thanks,

Steve

[-- Attachment #2: new-mount-option.patch --]
[-- Type: text/x-diff, Size: 4378 bytes --]

diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index d43e0fe..b33c841 100644
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -8,7 +8,9 @@ user's smb session.  This fix allows cifs to mount multiple times to the
 same server with different userids without risking invalidating earlier
 established security contexts.  fsync now sends SMB Flush operation
 to better ensure that we wait for server to write all of the data to
-server disk (not just write it over the network).
+server disk (not just write it over the network).  Add new mount
+parameter to allow user to disable sending the (slow) SMB flush on
+fsync if desired (fsync still flushes all cached write data to the server).
 
 Version 1.56
 ------------
diff --git a/fs/cifs/README b/fs/cifs/README
index da4515e..6436966 100644
--- a/fs/cifs/README
+++ b/fs/cifs/README
@@ -472,6 +472,19 @@ A partial list of the supported mount options follows:
 		even if the cifs server would support posix advisory locks.
 		"forcemand" is accepted as a shorter form of this mount
 		option.
+ nostrictsync   If this mount option is set, when an application does an
+		fsync call then the cifs client does not send an SMB Flush
+		to the server (to force the server to write all dirty data
+		for this file immediately to disk), although cifs still sends
+		all dirty (cached) file data to the server and waits for the
+		server to respond to the write write.  Since SMB Flush can be
+		very slow, and some servers may be reliable enough (to risk
+		delaying slightly flushing the data to disk on the server),
+		turning on this option may be useful to improve performance for
+		applications that fsync too much, at a small risk of server
+		crash.  If this mount option is not set, by default cifs will
+		send an SMB flush request (and wait for a response) on every
+		fsync call.
  nodfs          Disable DFS (global name space support) even if the
 		server claims to support it.  This can help work around
 		a problem with parsing of DFS paths with Samba server
diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index c4c306f..e9f177b 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -32,6 +32,7 @@
 #define CIFS_MOUNT_OVERR_GID    0x800 /* override gid returned from server    */
 #define CIFS_MOUNT_DYNPERM      0x1000 /* allow in-memory only mode setting   */
 #define CIFS_MOUNT_NOPOSIXBRL   0x2000 /* mandatory not posix byte range lock */
+#define CIFS_MOUNT_NO_SSYNC     0x4000 /* don't do slow SMBflush on every sync*/
 
 struct cifs_sb_info {
 	struct cifsTconInfo *tcon;	/* primary mount */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index da0f4ff..18e84a4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -95,6 +95,7 @@ struct smb_vol {
 	bool local_lease:1; /* check leases only on local system, not remote */
 	bool noblocksnd:1;
 	bool noautotune:1;
+	bool nostrictsync:1; /* do not force expensive SMBflush on every sync */
 	unsigned int rsize;
 	unsigned int wsize;
 	unsigned int sockopt;
@@ -1274,6 +1275,10 @@ cifs_parse_mount_options(char *options, const char *devname,
 			vol->intr = 0;
 		} else if (strnicmp(data, "intr", 4) == 0) {
 			vol->intr = 1;
+		} else if (strnicmp(data, "nostrictsync", 12) == 0) {
+			vol->nostrictsync = 1;
+		} else if (strnicmp(data, "strictsync", 10) == 0) {
+			vol->nostrictsync = 0;
 		} else if (strnicmp(data, "serverino", 7) == 0) {
 			vol->server_ino = 1;
 		} else if (strnicmp(data, "noserverino", 9) == 0) {
@@ -2160,6 +2165,8 @@ static void setup_cifs_sb(struct smb_vol *pvolume_info,
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_UNX_EMUL;
 	if (pvolume_info->nobrl)
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NO_BRL;
+	if (pvolume_info->nostrictsync)
+		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NO_SSYNC;
 	if (pvolume_info->mand_lock)
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NOPOSIXBRL;
 	if (pvolume_info->cifs_acl)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 83b4741..6411f5f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1538,7 +1538,8 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
 		rc = CIFS_I(inode)->write_behind_rc;
 		CIFS_I(inode)->write_behind_rc = 0;
 		tcon = CIFS_SB(inode->i_sb)->tcon;
-		if (!rc && tcon && smbfile)
+		if (!rc && tcon && smbfile &&
+		   !(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_SSYNC))
 			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
 	}
 

Re: [linux-cifs-client] Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync

[Jeff Layton]

On Sun, 22 Feb 2009 22:07:05 -0600
Steve French <smfrench@gmail.com> wrote:

> diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
> index d43e0fe..b33c841 100644
> --- a/fs/cifs/CHANGES
> +++ b/fs/cifs/CHANGES
> @@ -8,7 +8,9 @@ user's smb session.  This fix allows cifs to mount multiple times to the
>  same server with different userids without risking invalidating earlier
>  established security contexts.  fsync now sends SMB Flush operation
>  to better ensure that we wait for server to write all of the data to
> -server disk (not just write it over the network).
> +server disk (not just write it over the network).  Add new mount
> +parameter to allow user to disable sending the (slow) SMB flush on
> +fsync if desired (fsync still flushes all cached write data to the server).
>  
>  Version 1.56
>  ------------
> diff --git a/fs/cifs/README b/fs/cifs/README
> index da4515e..6436966 100644
> --- a/fs/cifs/README
> +++ b/fs/cifs/README
> @@ -472,6 +472,19 @@ A partial list of the supported mount options follows:
>  		even if the cifs server would support posix advisory locks.
>  		"forcemand" is accepted as a shorter form of this mount
>  		option.
> + nostrictsync   If this mount option is set, when an application does an
> +		fsync call then the cifs client does not send an SMB Flush
> +		to the server (to force the server to write all dirty data
> +		for this file immediately to disk), although cifs still sends
> +		all dirty (cached) file data to the server and waits for the
> +		server to respond to the write write.  Since SMB Flush can be
> +		very slow, and some servers may be reliable enough (to risk
> +		delaying slightly flushing the data to disk on the server),
> +		turning on this option may be useful to improve performance for
> +		applications that fsync too much, at a small risk of server
> +		crash.  If this mount option is not set, by default cifs will
> +		send an SMB flush request (and wait for a response) on every
> +		fsync call.
>   nodfs          Disable DFS (global name space support) even if the
>  		server claims to support it.  This can help work around
>  		a problem with parsing of DFS paths with Samba server
> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
> index c4c306f..e9f177b 100644
> --- a/fs/cifs/cifs_fs_sb.h
> +++ b/fs/cifs/cifs_fs_sb.h
> @@ -32,6 +32,7 @@
>  #define CIFS_MOUNT_OVERR_GID    0x800 /* override gid returned from server    */
>  #define CIFS_MOUNT_DYNPERM      0x1000 /* allow in-memory only mode setting   */
>  #define CIFS_MOUNT_NOPOSIXBRL   0x2000 /* mandatory not posix byte range lock */
> +#define CIFS_MOUNT_NO_SSYNC     0x4000 /* don't do slow SMBflush on every sync*/
>  
>  struct cifs_sb_info {
>  	struct cifsTconInfo *tcon;	/* primary mount */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index da0f4ff..18e84a4 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -95,6 +95,7 @@ struct smb_vol {
>  	bool local_lease:1; /* check leases only on local system, not remote */
>  	bool noblocksnd:1;
>  	bool noautotune:1;
> +	bool nostrictsync:1; /* do not force expensive SMBflush on every sync */
>  	unsigned int rsize;
>  	unsigned int wsize;
>  	unsigned int sockopt;
> @@ -1274,6 +1275,10 @@ cifs_parse_mount_options(char *options, const char *devname,
>  			vol->intr = 0;
>  		} else if (strnicmp(data, "intr", 4) == 0) {
>  			vol->intr = 1;
> +		} else if (strnicmp(data, "nostrictsync", 12) == 0) {
> +			vol->nostrictsync = 1;
> +		} else if (strnicmp(data, "strictsync", 10) == 0) {
> +			vol->nostrictsync = 0;
>  		} else if (strnicmp(data, "serverino", 7) == 0) {
>  			vol->server_ino = 1;
>  		} else if (strnicmp(data, "noserverino", 9) == 0) {
> @@ -2160,6 +2165,8 @@ static void setup_cifs_sb(struct smb_vol *pvolume_info,
>  		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_UNX_EMUL;
>  	if (pvolume_info->nobrl)
>  		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NO_BRL;
> +	if (pvolume_info->nostrictsync)
> +		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NO_SSYNC;
>  	if (pvolume_info->mand_lock)
>  		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NOPOSIXBRL;
>  	if (pvolume_info->cifs_acl)
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 83b4741..6411f5f 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1538,7 +1538,8 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync)
>  		rc = CIFS_I(inode)->write_behind_rc;
>  		CIFS_I(inode)->write_behind_rc = 0;
>  		tcon = CIFS_SB(inode->i_sb)->tcon;
> -		if (!rc && tcon && smbfile)
> +		if (!rc && tcon && smbfile &&
> +		   !(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_SSYNC))
		     ^^^^^^^
This doesn't compile as there is no cifs_sb defined in this function...

>  			rc = CIFSSMBFlush(xid, tcon, smbfile->netfid);
>  	}
>  


-- 
Jeff Layton <jlayton@redhat.com>