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

From: Jeff Layton
Date: Mon Feb 23 2009 - 11:34:37 EST


On Sun, 22 Feb 2009 22:07:05 -0600
Steve French <smfrench@xxxxxxxxx> 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@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/