[PATCH 0/4] cifs: CONFIG_CIFS_EXPERIMENTAL removal

[PATCH 0/4] cifs: CONFIG_CIFS_EXPERIMENTAL removal

[Jeff Layton]

The CONFIG_CIFS_EXPERIMENTAL KConfig option is the sort of thing that
gives distro packagers nightmares. The things that live under it are
impossible to predict for someone who isn't following development
upstream.

We usually want bleeding-edge distros like Fedora to turn on these sorts
of bleeding edge features, but it's difficult for them to do so with any
confidence because so much of the code under this option is just plain
broken. If we have code that needs to be conditionally compiled in,
then it generally ought to be given its own KConfig option.

This patchset eliminates the CONFIG_CIFS_EXPERIMENTAL Kconfig option.
Code that currently resides under this option is either moved to being
built in by default or is removed from the kernel altogether.

The last patch in the series also removes /proc/fs/cifs/Experimental --
a knob that's purpose has been unclear since I've been working on CIFS.

I've tested this by building cifs.ko with a variety of different Kconfig
combinations and it seems to be OK.

I think this patchset is appropriate for 2.6.38, though I won't object
if you want to merge it sooner.

Jeff Layton (4):
  cifs: remove export_ops code
  cifs: move "ntlmssp" and "local_leases" options out of experimental
    code
  cifs: remove CIFSSMBQueryReparseLinkInfo and CONFIG_CIFS_EXPERIMENTAL
  cifs: remove /proc/fs/cifs/Experimental

 fs/cifs/Kconfig      |   13 ------
 fs/cifs/Makefile     |    2 +-
 fs/cifs/README       |   12 -----
 fs/cifs/cifs_debug.c |   42 -------------------
 fs/cifs/cifsfs.c     |    8 ----
 fs/cifs/cifsfs.h     |    4 --
 fs/cifs/cifsglob.h   |    1 -
 fs/cifs/cifsproto.h  |    6 ---
 fs/cifs/cifssmb.c    |  109 +-------------------------------------------------
 fs/cifs/connect.c    |    4 --
 fs/cifs/export.c     |   67 ------------------------------
 fs/cifs/file.c       |    6 +-
 fs/cifs/sess.c       |  103 +++++++++++++++++++++-------------------------
 13 files changed, 52 insertions(+), 325 deletions(-)
 delete mode 100644 fs/cifs/export.c

-- 
1.7.3.2

[PATCH 1/4] cifs: remove export_ops code

[Jeff Layton]

Stubs for this code were added under the CONFIG_CIFS_EXPERIMENTAL tag in
2007, but were never completed. The upshot -- anyone who turns on
CONFIG_CIFS_EXPERIMENTAL gets a filesystem that pretends to allow exporting
via nfsd. Trying to actually use it will just get you an error once
nfsd tries to walk into the directory.

Remove this code since it's unfinished. We can reintroduce it once someone
spends time on it and makes it usable.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/Makefile |    2 +-
 fs/cifs/cifsfs.c |    7 -----
 fs/cifs/cifsfs.h |    4 ---
 fs/cifs/export.c |   67 ------------------------------------------------------
 4 files changed, 1 insertions(+), 79 deletions(-)
 delete mode 100644 fs/cifs/export.c

diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile
index 43b19dd..ee8a33c 100644
--- a/fs/cifs/Makefile
+++ b/fs/cifs/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_CIFS) += cifs.o
 cifs-y := cifsfs.o cifssmb.o cifs_debug.o connect.o dir.o file.o inode.o \
 	  link.o misc.o netmisc.o smbdes.o smbencrypt.o transport.o asn1.o \
 	  md4.o md5.o cifs_unicode.o nterr.o xattr.o cifsencrypt.o \
-	  readdir.o ioctl.o sess.o export.o
+	  readdir.o ioctl.o sess.o
 
 cifs-$(CONFIG_CIFS_ACL) += cifsacl.o
 
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index db28c28..48582b7 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -174,13 +174,6 @@ cifs_read_super(struct super_block *sb, void *data,
 		goto out_no_root;
 	}
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
-		cFYI(1, "export ops supported");
-		sb->s_export_op = &cifs_export_ops;
-	}
-#endif /* EXPERIMENTAL */
-
 	return 0;
 
 out_no_root:
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 897b2b2..1d6d021 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -108,9 +108,5 @@ extern ssize_t	cifs_getxattr(struct dentry *, const char *, void *, size_t);
 extern ssize_t	cifs_listxattr(struct dentry *, char *, size_t);
 extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-extern const struct export_operations cifs_export_ops;
-#endif /* EXPERIMENTAL */
-
 #define CIFS_VERSION   "1.68"
 #endif				/* _CIFSFS_H */
diff --git a/fs/cifs/export.c b/fs/cifs/export.c
deleted file mode 100644
index 993f820..0000000
--- a/fs/cifs/export.c
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- *   fs/cifs/export.c
- *
- *   Copyright (C) International Business Machines  Corp., 2007
- *   Author(s): Steve French (sfrench-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org)
- *
- *   Common Internet FileSystem (CIFS) client
- *
- *   Operations related to support for exporting files via NFSD
- *
- *   This library is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU Lesser General Public License as published
- *   by the Free Software Foundation; either version 2.1 of the License, or
- *   (at your option) any later version.
- *
- *   This library is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU Lesser General Public License for more details.
- *
- *   You should have received a copy of the GNU Lesser General Public License
- *   along with this library; if not, write to the Free Software
- *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
-
- /*
-  * See Documentation/filesystems/nfs/Exporting
-  * and examples in fs/exportfs
-  *
-  * Since cifs is a network file system, an "fsid" must be included for
-  * any nfs exports file entries which refer to cifs paths.  In addition
-  * the cifs mount must be mounted with the "serverino" option (ie use stable
-  * server inode numbers instead of locally generated temporary ones).
-  * Although cifs inodes do not use generation numbers (have generation number
-  * of zero) - the inode number alone should be good enough for simple cases
-  * in which users want to export cifs shares with NFS. The decode and encode
-  * could be improved by using a new routine which expects 64 bit inode numbers
-  * instead of the default 32 bit routines in fs/exportfs
-  *
-  */
-
-#include <linux/fs.h>
-#include <linux/exportfs.h>
-#include "cifsglob.h"
-#include "cifs_debug.h"
-#include "cifsfs.h"
-
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-static struct dentry *cifs_get_parent(struct dentry *dentry)
-{
-	/* BB need to add code here eventually to enable export via NFSD */
-	cFYI(1, "get parent for %p", dentry);
-	return ERR_PTR(-EACCES);
-}
-
-const struct export_operations cifs_export_ops = {
-	.get_parent = cifs_get_parent,
-/*	Following five export operations are unneeded so far and can default:
-	.get_dentry =
-	.get_name =
-	.find_exported_dentry =
-	.decode_fh =
-	.encode_fs =  */
-};
-
-#endif /* EXPERIMENTAL */
-
-- 
1.7.3.2

[PATCH 2/4] cifs: move "ntlmssp" and "local_leases" options out of experimental code

[Jeff Layton]

I see no real need to leave these sorts of options under an
EXPERIMENTAL ifdef. Since you need a mount option to turn this code
on, that only blows out the testing matrix.

local_leases has been under the EXPERIMENTAL tag for some time, but
it's only the mount option that's under this label. Move it out
from under this tag.

The NTLMSSP code is also under EXPERIMENTAL, but it needs a mount
option to turn it on, and in the future any distro will reasonably
want this enabled. Go ahead and move it out from under the
EXPERIMENTAL tag.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifssmb.c |    5 +--
 fs/cifs/connect.c |    4 --
 fs/cifs/sess.c    |  103 ++++++++++++++++++++++++-----------------------------
 3 files changed, 48 insertions(+), 64 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index d7957a3..4df1d10 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -401,15 +401,12 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
 	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_KRB5) {
 		cFYI(1, "Kerberos only mechanism, enable extended security");
 		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
-	}
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-	else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
+	} else if ((secFlags & CIFSSEC_MUST_NTLMSSP) == CIFSSEC_MUST_NTLMSSP)
 		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
 	else if ((secFlags & CIFSSEC_AUTH_MASK) == CIFSSEC_MAY_NTLMSSP) {
 		cFYI(1, "NTLMSSP only mechanism, enable extended security");
 		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
 	}
-#endif
 
 	count = 0;
 	for (i = 0; i < CIFS_NUM_PROT; i++) {
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 23098c2..9fbe7c5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -985,13 +985,11 @@ cifs_parse_mount_options(char *options, const char *devname,
 				return 1;
 			} else if (strnicmp(value, "krb5", 4) == 0) {
 				vol->secFlg |= CIFSSEC_MAY_KRB5;
-#ifdef CONFIG_CIFS_EXPERIMENTAL
 			} else if (strnicmp(value, "ntlmsspi", 8) == 0) {
 				vol->secFlg |= CIFSSEC_MAY_NTLMSSP |
 					CIFSSEC_MUST_SIGN;
 			} else if (strnicmp(value, "ntlmssp", 7) == 0) {
 				vol->secFlg |= CIFSSEC_MAY_NTLMSSP;
-#endif
 			} else if (strnicmp(value, "ntlmv2i", 7) == 0) {
 				vol->secFlg |= CIFSSEC_MAY_NTLMV2 |
 					CIFSSEC_MUST_SIGN;
@@ -1342,10 +1340,8 @@ cifs_parse_mount_options(char *options, const char *devname,
 			vol->no_psx_acl = 0;
 		} else if (strnicmp(data, "noacl", 5) == 0) {
 			vol->no_psx_acl = 1;
-#ifdef CONFIG_CIFS_EXPERIMENTAL
 		} else if (strnicmp(data, "locallease", 6) == 0) {
 			vol->local_lease = 1;
-#endif
 		} else if (strnicmp(data, "sign", 4) == 0) {
 			vol->secFlg |= CIFSSEC_MUST_SIGN;
 		} else if (strnicmp(data, "seal", 4) == 0) {
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 7b01d3f..2997533 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -420,7 +420,6 @@ static int decode_ntlmssp_challenge(char *bcc_ptr, int blob_len,
 	return 0;
 }
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
 /* BB Move to ntlmssp.c eventually */
 
 /* We do not malloc the blob, it is passed in pbuffer, because
@@ -572,7 +571,6 @@ static void setup_ntlmssp_neg_req(SESSION_SETUP_ANDX *pSMB,
 
 	return;
 }
-#endif
 
 int
 CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses,
@@ -814,71 +812,64 @@ ssetup_ntlmssp_authenticate:
 		rc = -ENOSYS;
 		goto ssetup_exit;
 #endif /* CONFIG_CIFS_UPCALL */
-	} else {
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-		if (type == RawNTLMSSP) {
-			if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
-				cERROR(1, "NTLMSSP requires Unicode support");
-				rc = -ENOSYS;
+	} else if (type == RawNTLMSSP) {
+		if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
+			cERROR(1, "NTLMSSP requires Unicode support");
+			rc = -ENOSYS;
+			goto ssetup_exit;
+		}
+
+		cFYI(1, "ntlmssp session setup phase %d", phase);
+		pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
+		capabilities |= CAP_EXTENDED_SECURITY;
+		pSMB->req.Capabilities |= cpu_to_le32(capabilities);
+		if (phase == NtLmNegotiate) {
+			setup_ntlmssp_neg_req(pSMB, ses);
+			iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
+			iov[1].iov_base = &pSMB->req.SecurityBlob[0];
+		} else if (phase == NtLmAuthenticate) {
+			/* 5 is an empirical value, large enought to
+			 * hold authenticate message, max 10 of
+			 * av paris, doamin,user,workstation mames,
+			 * flags etc..
+			 */
+			ntlmsspblob = kmalloc(
+				5*sizeof(struct _AUTHENTICATE_MESSAGE),
+				GFP_KERNEL);
+			if (!ntlmsspblob) {
+				cERROR(1, "Can't allocate NTLMSSP");
+				rc = -ENOMEM;
 				goto ssetup_exit;
 			}
 
-			cFYI(1, "ntlmssp session setup phase %d", phase);
-			pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
-			capabilities |= CAP_EXTENDED_SECURITY;
-			pSMB->req.Capabilities |= cpu_to_le32(capabilities);
-			if (phase == NtLmNegotiate) {
-				setup_ntlmssp_neg_req(pSMB, ses);
-				iov[1].iov_len = sizeof(NEGOTIATE_MESSAGE);
-				iov[1].iov_base = &pSMB->req.SecurityBlob[0];
-			} else if (phase == NtLmAuthenticate) {
-				/* 5 is an empirical value, large enought to
-				 * hold authenticate message, max 10 of
-				 * av paris, doamin,user,workstation mames,
-				 * flags etc..
-				 */
-				ntlmsspblob = kmalloc(
-					5*sizeof(struct _AUTHENTICATE_MESSAGE),
-					GFP_KERNEL);
-				if (!ntlmsspblob) {
-					cERROR(1, "Can't allocate NTLMSSP");
-					rc = -ENOMEM;
-					goto ssetup_exit;
-				}
-
-				rc = build_ntlmssp_auth_blob(ntlmsspblob,
-							&blob_len, ses, nls_cp);
-				if (rc)
-					goto ssetup_exit;
-				iov[1].iov_len = blob_len;
-				iov[1].iov_base = ntlmsspblob;
-				pSMB->req.SecurityBlobLength =
-					cpu_to_le16(blob_len);
-				/* Make sure that we tell the server that we
-				   are using the uid that it just gave us back
-				   on the response (challenge) */
-				smb_buf->Uid = ses->Suid;
-			} else {
-				cERROR(1, "invalid phase %d", phase);
-				rc = -ENOSYS;
+			rc = build_ntlmssp_auth_blob(ntlmsspblob,
+						&blob_len, ses, nls_cp);
+			if (rc)
 				goto ssetup_exit;
-			}
-			/* unicode strings must be word aligned */
-			if ((iov[0].iov_len + iov[1].iov_len) % 2) {
-				*bcc_ptr = 0;
-				bcc_ptr++;
-			}
-			unicode_oslm_strings(&bcc_ptr, nls_cp);
+			iov[1].iov_len = blob_len;
+			iov[1].iov_base = ntlmsspblob;
+			pSMB->req.SecurityBlobLength = cpu_to_le16(blob_len);
+			/* Make sure that we tell the server that we
+			   are using the uid that it just gave us back
+			   on the response (challenge) */
+			smb_buf->Uid = ses->Suid;
 		} else {
-			cERROR(1, "secType %d not supported!", type);
+			cERROR(1, "invalid phase %d", phase);
 			rc = -ENOSYS;
 			goto ssetup_exit;
 		}
-#else
+
+		/* unicode strings must be word aligned */
+		if ((iov[0].iov_len + iov[1].iov_len) % 2) {
+			*bcc_ptr = 0;
+			bcc_ptr++;
+		}
+
+		unicode_oslm_strings(&bcc_ptr, nls_cp);
+	} else {
 		cERROR(1, "secType %d not supported!", type);
 		rc = -ENOSYS;
 		goto ssetup_exit;
-#endif
 	}
 
 	iov[2].iov_base = str_area;
-- 
1.7.3.2

[PATCH 3/4] cifs: remove CIFSSMBQueryReparseLinkInfo and CONFIG_CIFS_EXPERIMENTAL

[Jeff Layton]

Nothing calls this function. Remove it.

With this, the last user of CONFIG_CIFS_EXPERIMENTAL has been
removed. Also, update the README.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/Kconfig     |   13 ------
 fs/cifs/README      |   12 ------
 fs/cifs/cifsproto.h |    6 ---
 fs/cifs/cifssmb.c   |  104 ---------------------------------------------------
 4 files changed, 0 insertions(+), 135 deletions(-)

diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
index ee45648..27917a5 100644
--- a/fs/cifs/Kconfig
+++ b/fs/cifs/Kconfig
@@ -151,16 +151,3 @@ config CIFS_ACL
 	    Allows to fetch CIFS/NTFS ACL from the server.  The DACL blob
 	    is handed over to the application/caller.
 
-config CIFS_EXPERIMENTAL
-	  bool "CIFS Experimental Features (EXPERIMENTAL)"
-	  depends on CIFS && EXPERIMENTAL
-	  help
-	    Enables cifs features under testing. These features are
-	    experimental and currently include DFS support and directory
-	    change notification ie fcntl(F_DNOTIFY), as well as the upcall
-	    mechanism which will be used for Kerberos session negotiation
-	    and uid remapping.  Some of these features also may depend on
-	    setting a value of 1 to the pseudo-file /proc/fs/cifs/Experimental
-	    (which is disabled by default). See the file fs/cifs/README
-	    for more details.  If unsure, say N.
-
diff --git a/fs/cifs/README b/fs/cifs/README
index 46af99a..60a8ba5 100644
--- a/fs/cifs/README
+++ b/fs/cifs/README
@@ -715,18 +715,6 @@ the start of smb requests and responses can be enabled via:
 
 	echo 1 > /proc/fs/cifs/traceSMB
 
-Two other experimental features are under development. To test these
-requires enabling CONFIG_CIFS_EXPERIMENTAL
-
-	cifsacl support needed to retrieve approximated mode bits based on
-		the contents on the CIFS ACL.
-
-	lease support: cifs will check the oplock state before calling into
-	the vfs to see if we can grant a lease on a file.
-
-	DNOTIFY fcntl: needed for support of directory change 
-			    notification and perhaps later for file leases)
-
 Per share (per client mount) statistics are available in /proc/fs/cifs/Stats
 if the kernel was configured with cifs statistics enabled.  The statistics
 represent the number of successful (ie non-zero return code from the server) 
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index cb65499..fe77e69 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -296,12 +296,6 @@ extern int CIFSSMBUnixQuerySymLink(const int xid,
 			struct cifsTconInfo *tcon,
 			const unsigned char *searchName, char **syminfo,
 			const struct nls_table *nls_codepage);
-extern int CIFSSMBQueryReparseLinkInfo(const int xid,
-			struct cifsTconInfo *tcon,
-			const unsigned char *searchName,
-			char *symlinkinfo, const int buflen, __u16 fid,
-			const struct nls_table *nls_codepage);
-
 extern int CIFSSMBOpen(const int xid, struct cifsTconInfo *tcon,
 			const char *fileName, const int disposition,
 			const int access_flags, const int omode,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 4df1d10..9af98f6 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -2474,110 +2474,6 @@ querySymLinkRetry:
 	return rc;
 }
 
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-int
-CIFSSMBQueryReparseLinkInfo(const int xid, struct cifsTconInfo *tcon,
-			const unsigned char *searchName,
-			char *symlinkinfo, const int buflen, __u16 fid,
-			const struct nls_table *nls_codepage)
-{
-	int rc = 0;
-	int bytes_returned;
-	struct smb_com_transaction_ioctl_req *pSMB;
-	struct smb_com_transaction_ioctl_rsp *pSMBr;
-
-	cFYI(1, "In Windows reparse style QueryLink for path %s", searchName);
-	rc = smb_init(SMB_COM_NT_TRANSACT, 23, tcon, (void **) &pSMB,
-		      (void **) &pSMBr);
-	if (rc)
-		return rc;
-
-	pSMB->TotalParameterCount = 0 ;
-	pSMB->TotalDataCount = 0;
-	pSMB->MaxParameterCount = cpu_to_le32(2);
-	/* BB find exact data count max from sess structure BB */
-	pSMB->MaxDataCount = cpu_to_le32((tcon->ses->server->maxBuf -
-					  MAX_CIFS_HDR_SIZE) & 0xFFFFFF00);
-	pSMB->MaxSetupCount = 4;
-	pSMB->Reserved = 0;
-	pSMB->ParameterOffset = 0;
-	pSMB->DataCount = 0;
-	pSMB->DataOffset = 0;
-	pSMB->SetupCount = 4;
-	pSMB->SubCommand = cpu_to_le16(NT_TRANSACT_IOCTL);
-	pSMB->ParameterCount = pSMB->TotalParameterCount;
-	pSMB->FunctionCode = cpu_to_le32(FSCTL_GET_REPARSE_POINT);
-	pSMB->IsFsctl = 1; /* FSCTL */
-	pSMB->IsRootFlag = 0;
-	pSMB->Fid = fid; /* file handle always le */
-	pSMB->ByteCount = 0;
-
-	rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
-			 (struct smb_hdr *) pSMBr, &bytes_returned, 0);
-	if (rc) {
-		cFYI(1, "Send error in QueryReparseLinkInfo = %d", rc);
-	} else {		/* decode response */
-		__u32 data_offset = le32_to_cpu(pSMBr->DataOffset);
-		__u32 data_count = le32_to_cpu(pSMBr->DataCount);
-		if ((pSMBr->ByteCount < 2) || (data_offset > 512)) {
-		/* BB also check enough total bytes returned */
-			rc = -EIO;	/* bad smb */
-			goto qreparse_out;
-		}
-		if (data_count && (data_count < 2048)) {
-			char *end_of_smb = 2 /* sizeof byte count */ +
-				pSMBr->ByteCount + (char *)&pSMBr->ByteCount;
-
-			struct reparse_data *reparse_buf =
-						(struct reparse_data *)
-						((char *)&pSMBr->hdr.Protocol
-								 + data_offset);
-			if ((char *)reparse_buf >= end_of_smb) {
-				rc = -EIO;
-				goto qreparse_out;
-			}
-			if ((reparse_buf->LinkNamesBuf +
-				reparse_buf->TargetNameOffset +
-				reparse_buf->TargetNameLen) > end_of_smb) {
-				cFYI(1, "reparse buf beyond SMB");
-				rc = -EIO;
-				goto qreparse_out;
-			}
-
-			if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) {
-				cifs_from_ucs2(symlinkinfo, (__le16 *)
-						(reparse_buf->LinkNamesBuf +
-						reparse_buf->TargetNameOffset),
-						buflen,
-						reparse_buf->TargetNameLen,
-						nls_codepage, 0);
-			} else { /* ASCII names */
-				strncpy(symlinkinfo,
-					reparse_buf->LinkNamesBuf +
-					reparse_buf->TargetNameOffset,
-					min_t(const int, buflen,
-					   reparse_buf->TargetNameLen));
-			}
-		} else {
-			rc = -EIO;
-			cFYI(1, "Invalid return data count on "
-				 "get reparse info ioctl");
-		}
-		symlinkinfo[buflen] = 0; /* just in case so the caller
-					does not go off the end of the buffer */
-		cFYI(1, "readlink result - %s", symlinkinfo);
-	}
-
-qreparse_out:
-	cifs_buf_release(pSMB);
-
-	/* Note: On -EAGAIN error only caller can retry on handle based calls
-		since file handle passed in no longer valid */
-
-	return rc;
-}
-#endif /* CIFS_EXPERIMENTAL */
-
 #ifdef CONFIG_CIFS_POSIX
 
 /*Convert an Access Control Entry from wire format to local POSIX xattr format*/
-- 
1.7.3.2

[PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Jeff Layton]

This flag only affects a couple of places in the write codepath, and
it's not entirely clear to me what the purpose of it is. I've never
known anyone (even developers) to use this knob, so let's remove it.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifs_debug.c |   42 ------------------------------------------
 fs/cifs/cifsfs.c     |    1 -
 fs/cifs/cifsglob.h   |    1 -
 fs/cifs/file.c       |    6 +++---
 4 files changed, 3 insertions(+), 47 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 103ab8b..04eecb2 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -425,7 +425,6 @@ static const struct file_operations cifs_lookup_cache_proc_fops;
 static const struct file_operations traceSMB_proc_fops;
 static const struct file_operations cifs_multiuser_mount_proc_fops;
 static const struct file_operations cifs_security_flags_proc_fops;
-static const struct file_operations cifs_experimental_proc_fops;
 static const struct file_operations cifs_linux_ext_proc_fops;
 
 void
@@ -443,8 +442,6 @@ cifs_proc_init(void)
 	proc_create("cifsFYI", 0, proc_fs_cifs, &cifsFYI_proc_fops);
 	proc_create("traceSMB", 0, proc_fs_cifs, &traceSMB_proc_fops);
 	proc_create("OplockEnabled", 0, proc_fs_cifs, &cifs_oplock_proc_fops);
-	proc_create("Experimental", 0, proc_fs_cifs,
-		    &cifs_experimental_proc_fops);
 	proc_create("LinuxExtensionsEnabled", 0, proc_fs_cifs,
 		    &cifs_linux_ext_proc_fops);
 	proc_create("MultiuserMount", 0, proc_fs_cifs,
@@ -552,45 +549,6 @@ static const struct file_operations cifs_oplock_proc_fops = {
 	.write		= cifs_oplock_proc_write,
 };
 
-static int cifs_experimental_proc_show(struct seq_file *m, void *v)
-{
-	seq_printf(m, "%d\n", experimEnabled);
-	return 0;
-}
-
-static int cifs_experimental_proc_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, cifs_experimental_proc_show, NULL);
-}
-
-static ssize_t cifs_experimental_proc_write(struct file *file,
-		const char __user *buffer, size_t count, loff_t *ppos)
-{
-	char c;
-	int rc;
-
-	rc = get_user(c, buffer);
-	if (rc)
-		return rc;
-	if (c == '0' || c == 'n' || c == 'N')
-		experimEnabled = 0;
-	else if (c == '1' || c == 'y' || c == 'Y')
-		experimEnabled = 1;
-	else if (c == '2')
-		experimEnabled = 2;
-
-	return count;
-}
-
-static const struct file_operations cifs_experimental_proc_fops = {
-	.owner		= THIS_MODULE,
-	.open		= cifs_experimental_proc_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-	.write		= cifs_experimental_proc_write,
-};
-
 static int cifs_linux_ext_proc_show(struct seq_file *m, void *v)
 {
 	seq_printf(m, "%d\n", linuxExtEnabled);
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 48582b7..932a347 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -53,7 +53,6 @@ int cifsFYI = 0;
 int cifsERROR = 1;
 int traceSMB = 0;
 unsigned int oplockEnabled = 1;
-unsigned int experimEnabled = 0;
 unsigned int linuxExtEnabled = 1;
 unsigned int lookupCacheEnabled = 1;
 unsigned int multiuser_mount = 0;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 37fb246..5ab6701 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -777,7 +777,6 @@ GLOBAL_EXTERN unsigned int multiuser_mount; /* if enabled allows new sessions
 				have the uid/password or Kerberos credential
 				or equivalent for current user */
 GLOBAL_EXTERN unsigned int oplockEnabled;
-GLOBAL_EXTERN unsigned int experimEnabled;
 GLOBAL_EXTERN unsigned int lookupCacheEnabled;
 GLOBAL_EXTERN unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 5a28660..5f6907e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1054,10 +1054,10 @@ static ssize_t cifs_write(struct cifsFileInfo *open_file,
 				if (rc != 0)
 					break;
 			}
-			if (experimEnabled || (pTcon->ses->server &&
+			if (pTcon->ses->server &&
 				((pTcon->ses->server->secMode &
 				(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
-				== 0))) {
+				== 0)) {
 				struct kvec iov[2];
 				unsigned int len;
 
@@ -1319,7 +1319,7 @@ static int cifs_writepages(struct address_space *mapping,
 	}
 
 	tcon = tlink_tcon(open_file->tlink);
-	if (!experimEnabled && tcon->ses->server->secMode &
+	if (tcon->ses->server->secMode &
 			(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
 		cifsFileInfo_put(open_file);
 		kfree(iov);
-- 
1.7.3.2

Re: [PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Jeff Layton]

On Tue,  7 Dec 2010 09:23:55 -0500
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> This flag only affects a couple of places in the write codepath, and
> it's not entirely clear to me what the purpose of it is. I've never
> known anyone (even developers) to use this knob, so let's remove it.
> 
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifs_debug.c |   42 ------------------------------------------
>  fs/cifs/cifsfs.c     |    1 -
>  fs/cifs/cifsglob.h   |    1 -
>  fs/cifs/file.c       |    6 +++---
>  4 files changed, 3 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 103ab8b..04eecb2 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -425,7 +425,6 @@ static const struct file_operations cifs_lookup_cache_proc_fops;
>  static const struct file_operations traceSMB_proc_fops;
>  static const struct file_operations cifs_multiuser_mount_proc_fops;
>  static const struct file_operations cifs_security_flags_proc_fops;
> -static const struct file_operations cifs_experimental_proc_fops;
>  static const struct file_operations cifs_linux_ext_proc_fops;
>  
>  void
> @@ -443,8 +442,6 @@ cifs_proc_init(void)
>  	proc_create("cifsFYI", 0, proc_fs_cifs, &cifsFYI_proc_fops);
>  	proc_create("traceSMB", 0, proc_fs_cifs, &traceSMB_proc_fops);
>  	proc_create("OplockEnabled", 0, proc_fs_cifs, &cifs_oplock_proc_fops);
> -	proc_create("Experimental", 0, proc_fs_cifs,
> -		    &cifs_experimental_proc_fops);
>  	proc_create("LinuxExtensionsEnabled", 0, proc_fs_cifs,
>  		    &cifs_linux_ext_proc_fops);
>  	proc_create("MultiuserMount", 0, proc_fs_cifs,
> @@ -552,45 +549,6 @@ static const struct file_operations cifs_oplock_proc_fops = {
>  	.write		= cifs_oplock_proc_write,
>  };
>  
> -static int cifs_experimental_proc_show(struct seq_file *m, void *v)
> -{
> -	seq_printf(m, "%d\n", experimEnabled);
> -	return 0;
> -}
> -
> -static int cifs_experimental_proc_open(struct inode *inode, struct file *file)
> -{
> -	return single_open(file, cifs_experimental_proc_show, NULL);
> -}
> -
> -static ssize_t cifs_experimental_proc_write(struct file *file,
> -		const char __user *buffer, size_t count, loff_t *ppos)
> -{
> -	char c;
> -	int rc;
> -
> -	rc = get_user(c, buffer);
> -	if (rc)
> -		return rc;
> -	if (c == '0' || c == 'n' || c == 'N')
> -		experimEnabled = 0;
> -	else if (c == '1' || c == 'y' || c == 'Y')
> -		experimEnabled = 1;
> -	else if (c == '2')
> -		experimEnabled = 2;
> -
> -	return count;
> -}
> -
> -static const struct file_operations cifs_experimental_proc_fops = {
> -	.owner		= THIS_MODULE,
> -	.open		= cifs_experimental_proc_open,
> -	.read		= seq_read,
> -	.llseek		= seq_lseek,
> -	.release	= single_release,
> -	.write		= cifs_experimental_proc_write,
> -};
> -
>  static int cifs_linux_ext_proc_show(struct seq_file *m, void *v)
>  {
>  	seq_printf(m, "%d\n", linuxExtEnabled);
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 48582b7..932a347 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -53,7 +53,6 @@ int cifsFYI = 0;
>  int cifsERROR = 1;
>  int traceSMB = 0;
>  unsigned int oplockEnabled = 1;
> -unsigned int experimEnabled = 0;
>  unsigned int linuxExtEnabled = 1;
>  unsigned int lookupCacheEnabled = 1;
>  unsigned int multiuser_mount = 0;
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 37fb246..5ab6701 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -777,7 +777,6 @@ GLOBAL_EXTERN unsigned int multiuser_mount; /* if enabled allows new sessions
>  				have the uid/password or Kerberos credential
>  				or equivalent for current user */
>  GLOBAL_EXTERN unsigned int oplockEnabled;
> -GLOBAL_EXTERN unsigned int experimEnabled;
>  GLOBAL_EXTERN unsigned int lookupCacheEnabled;
>  GLOBAL_EXTERN unsigned int global_secflags;	/* if on, session setup sent
>  				with more secure ntlmssp2 challenge/resp */
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 5a28660..5f6907e 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1054,10 +1054,10 @@ static ssize_t cifs_write(struct cifsFileInfo *open_file,
>  				if (rc != 0)
>  					break;
>  			}
> -			if (experimEnabled || (pTcon->ses->server &&
> +			if (pTcon->ses->server &&
>  				((pTcon->ses->server->secMode &
>  				(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> -				== 0))) {
> +				== 0)) {
>  				struct kvec iov[2];
>  				unsigned int len;
>  
> @@ -1319,7 +1319,7 @@ static int cifs_writepages(struct address_space *mapping,
>  	}
>  
>  	tcon = tlink_tcon(open_file->tlink);
> -	if (!experimEnabled && tcon->ses->server->secMode &
> +	if (tcon->ses->server->secMode &
>  			(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
>  		cifsFileInfo_put(open_file);
>  		kfree(iov);

Just noticed that this misses removing the de-registration of
"Experimental", so it'll need to be respun.

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

Re: [PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Steve French]

The issue of how to prevent a page from being modified as it is
written out (due to signing in our case) has been discussed on lkml a
few times (e.g. T10 block devices).   We should hold off on changing
this until we have a way of handling the case of:   we calculate the
signature, but just before the page in the cache is remodified, we
send it with the wrong signature ... obviously if we reissue the write
we are fine (the data is fine) but there may be better ways to lock
the page (and some suggestions have been made on lkml for similar
sounding problems).

On Tue, Dec 7, 2010 at 10:13 AM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Tue,  7 Dec 2010 09:23:55 -0500
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>> This flag only affects a couple of places in the write codepath, and
>> it's not entirely clear to me what the purpose of it is. I've never
>> known anyone (even developers) to use this knob, so let's remove it.
>>
>> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  fs/cifs/cifs_debug.c |   42 ------------------------------------------
>>  fs/cifs/cifsfs.c     |    1 -
>>  fs/cifs/cifsglob.h   |    1 -
>>  fs/cifs/file.c       |    6 +++---
>>  4 files changed, 3 insertions(+), 47 deletions(-)
>>
>> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
>> index 103ab8b..04eecb2 100644
>> --- a/fs/cifs/cifs_debug.c
>> +++ b/fs/cifs/cifs_debug.c
>> @@ -425,7 +425,6 @@ static const struct file_operations cifs_lookup_cache_proc_fops;
>>  static const struct file_operations traceSMB_proc_fops;
>>  static const struct file_operations cifs_multiuser_mount_proc_fops;
>>  static const struct file_operations cifs_security_flags_proc_fops;
>> -static const struct file_operations cifs_experimental_proc_fops;
>>  static const struct file_operations cifs_linux_ext_proc_fops;
>>
>>  void
>> @@ -443,8 +442,6 @@ cifs_proc_init(void)
>>       proc_create("cifsFYI", 0, proc_fs_cifs, &cifsFYI_proc_fops);
>>       proc_create("traceSMB", 0, proc_fs_cifs, &traceSMB_proc_fops);
>>       proc_create("OplockEnabled", 0, proc_fs_cifs, &cifs_oplock_proc_fops);
>> -     proc_create("Experimental", 0, proc_fs_cifs,
>> -                 &cifs_experimental_proc_fops);
>>       proc_create("LinuxExtensionsEnabled", 0, proc_fs_cifs,
>>                   &cifs_linux_ext_proc_fops);
>>       proc_create("MultiuserMount", 0, proc_fs_cifs,
>> @@ -552,45 +549,6 @@ static const struct file_operations cifs_oplock_proc_fops = {
>>       .write          = cifs_oplock_proc_write,
>>  };
>>
>> -static int cifs_experimental_proc_show(struct seq_file *m, void *v)
>> -{
>> -     seq_printf(m, "%d\n", experimEnabled);
>> -     return 0;
>> -}
>> -
>> -static int cifs_experimental_proc_open(struct inode *inode, struct file *file)
>> -{
>> -     return single_open(file, cifs_experimental_proc_show, NULL);
>> -}
>> -
>> -static ssize_t cifs_experimental_proc_write(struct file *file,
>> -             const char __user *buffer, size_t count, loff_t *ppos)
>> -{
>> -     char c;
>> -     int rc;
>> -
>> -     rc = get_user(c, buffer);
>> -     if (rc)
>> -             return rc;
>> -     if (c == '0' || c == 'n' || c == 'N')
>> -             experimEnabled = 0;
>> -     else if (c == '1' || c == 'y' || c == 'Y')
>> -             experimEnabled = 1;
>> -     else if (c == '2')
>> -             experimEnabled = 2;
>> -
>> -     return count;
>> -}
>> -
>> -static const struct file_operations cifs_experimental_proc_fops = {
>> -     .owner          = THIS_MODULE,
>> -     .open           = cifs_experimental_proc_open,
>> -     .read           = seq_read,
>> -     .llseek         = seq_lseek,
>> -     .release        = single_release,
>> -     .write          = cifs_experimental_proc_write,
>> -};
>> -
>>  static int cifs_linux_ext_proc_show(struct seq_file *m, void *v)
>>  {
>>       seq_printf(m, "%d\n", linuxExtEnabled);
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index 48582b7..932a347 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -53,7 +53,6 @@ int cifsFYI = 0;
>>  int cifsERROR = 1;
>>  int traceSMB = 0;
>>  unsigned int oplockEnabled = 1;
>> -unsigned int experimEnabled = 0;
>>  unsigned int linuxExtEnabled = 1;
>>  unsigned int lookupCacheEnabled = 1;
>>  unsigned int multiuser_mount = 0;
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 37fb246..5ab6701 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -777,7 +777,6 @@ GLOBAL_EXTERN unsigned int multiuser_mount; /* if enabled allows new sessions
>>                               have the uid/password or Kerberos credential
>>                               or equivalent for current user */
>>  GLOBAL_EXTERN unsigned int oplockEnabled;
>> -GLOBAL_EXTERN unsigned int experimEnabled;
>>  GLOBAL_EXTERN unsigned int lookupCacheEnabled;
>>  GLOBAL_EXTERN unsigned int global_secflags;  /* if on, session setup sent
>>                               with more secure ntlmssp2 challenge/resp */
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index 5a28660..5f6907e 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -1054,10 +1054,10 @@ static ssize_t cifs_write(struct cifsFileInfo *open_file,
>>                               if (rc != 0)
>>                                       break;
>>                       }
>> -                     if (experimEnabled || (pTcon->ses->server &&
>> +                     if (pTcon->ses->server &&
>>                               ((pTcon->ses->server->secMode &
>>                               (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
>> -                             == 0))) {
>> +                             == 0)) {
>>                               struct kvec iov[2];
>>                               unsigned int len;
>>
>> @@ -1319,7 +1319,7 @@ static int cifs_writepages(struct address_space *mapping,
>>       }
>>
>>       tcon = tlink_tcon(open_file->tlink);
>> -     if (!experimEnabled && tcon->ses->server->secMode &
>> +     if (tcon->ses->server->secMode &
>>                       (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
>>               cifsFileInfo_put(open_file);
>>               kfree(iov);
>
> Just noticed that this misses removing the de-registration of
> "Experimental", so it'll need to be respun.
>
> --
> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>



-- 
Thanks,

Steve

Re: [PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Jeff Layton]

On Tue, 7 Dec 2010 10:21:11 -0600
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> The issue of how to prevent a page from being modified as it is
> written out (due to signing in our case) has been discussed on lkml a
> few times (e.g. T10 block devices).   We should hold off on changing
> this until we have a way of handling the case of:   we calculate the
> signature, but just before the page in the cache is remodified, we
> send it with the wrong signature ... obviously if we reissue the write
> we are fine (the data is fine) but there may be better ways to lock
> the page (and some suggestions have been made on lkml for similar
> sounding problems).
> 

This behavior has been "Experimental" for years. At what point do we
remove this kludge? Who in their right mind is going to turn on a
switch called "Experimental" to enable this? As a user, I'd certainly
be reticent to do so. It's not clear what turning on "Experimental"
would give me.

If you think this behavior deserves to be switchable then let's put a
real usable switch on it. I don't think it ought to be hidden
as /proc/fs/cifs/Experimental. Perhaps a module parameter for
this would be more appropriate?

If so what would you name such a switch?

> On Tue, Dec 7, 2010 at 10:13 AM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> > On Tue,  7 Dec 2010 09:23:55 -0500
> > Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >> This flag only affects a couple of places in the write codepath, and
> >> it's not entirely clear to me what the purpose of it is. I've never
> >> known anyone (even developers) to use this knob, so let's remove it.
> >>
> >> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> ---
> >>  fs/cifs/cifs_debug.c |   42 ------------------------------------------
> >>  fs/cifs/cifsfs.c     |    1 -
> >>  fs/cifs/cifsglob.h   |    1 -
> >>  fs/cifs/file.c       |    6 +++---
> >>  4 files changed, 3 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> >> index 103ab8b..04eecb2 100644
> >> --- a/fs/cifs/cifs_debug.c
> >> +++ b/fs/cifs/cifs_debug.c
> >> @@ -425,7 +425,6 @@ static const struct file_operations cifs_lookup_cache_proc_fops;
> >>  static const struct file_operations traceSMB_proc_fops;
> >>  static const struct file_operations cifs_multiuser_mount_proc_fops;
> >>  static const struct file_operations cifs_security_flags_proc_fops;
> >> -static const struct file_operations cifs_experimental_proc_fops;
> >>  static const struct file_operations cifs_linux_ext_proc_fops;
> >>
> >>  void
> >> @@ -443,8 +442,6 @@ cifs_proc_init(void)
> >>       proc_create("cifsFYI", 0, proc_fs_cifs, &cifsFYI_proc_fops);
> >>       proc_create("traceSMB", 0, proc_fs_cifs, &traceSMB_proc_fops);
> >>       proc_create("OplockEnabled", 0, proc_fs_cifs, &cifs_oplock_proc_fops);
> >> -     proc_create("Experimental", 0, proc_fs_cifs,
> >> -                 &cifs_experimental_proc_fops);
> >>       proc_create("LinuxExtensionsEnabled", 0, proc_fs_cifs,
> >>                   &cifs_linux_ext_proc_fops);
> >>       proc_create("MultiuserMount", 0, proc_fs_cifs,
> >> @@ -552,45 +549,6 @@ static const struct file_operations cifs_oplock_proc_fops = {
> >>       .write          = cifs_oplock_proc_write,
> >>  };
> >>
> >> -static int cifs_experimental_proc_show(struct seq_file *m, void *v)
> >> -{
> >> -     seq_printf(m, "%d\n", experimEnabled);
> >> -     return 0;
> >> -}
> >> -
> >> -static int cifs_experimental_proc_open(struct inode *inode, struct file *file)
> >> -{
> >> -     return single_open(file, cifs_experimental_proc_show, NULL);
> >> -}
> >> -
> >> -static ssize_t cifs_experimental_proc_write(struct file *file,
> >> -             const char __user *buffer, size_t count, loff_t *ppos)
> >> -{
> >> -     char c;
> >> -     int rc;
> >> -
> >> -     rc = get_user(c, buffer);
> >> -     if (rc)
> >> -             return rc;
> >> -     if (c == '0' || c == 'n' || c == 'N')
> >> -             experimEnabled = 0;
> >> -     else if (c == '1' || c == 'y' || c == 'Y')
> >> -             experimEnabled = 1;
> >> -     else if (c == '2')
> >> -             experimEnabled = 2;
> >> -
> >> -     return count;
> >> -}
> >> -
> >> -static const struct file_operations cifs_experimental_proc_fops = {
> >> -     .owner          = THIS_MODULE,
> >> -     .open           = cifs_experimental_proc_open,
> >> -     .read           = seq_read,
> >> -     .llseek         = seq_lseek,
> >> -     .release        = single_release,
> >> -     .write          = cifs_experimental_proc_write,
> >> -};
> >> -
> >>  static int cifs_linux_ext_proc_show(struct seq_file *m, void *v)
> >>  {
> >>       seq_printf(m, "%d\n", linuxExtEnabled);
> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> >> index 48582b7..932a347 100644
> >> --- a/fs/cifs/cifsfs.c
> >> +++ b/fs/cifs/cifsfs.c
> >> @@ -53,7 +53,6 @@ int cifsFYI = 0;
> >>  int cifsERROR = 1;
> >>  int traceSMB = 0;
> >>  unsigned int oplockEnabled = 1;
> >> -unsigned int experimEnabled = 0;
> >>  unsigned int linuxExtEnabled = 1;
> >>  unsigned int lookupCacheEnabled = 1;
> >>  unsigned int multiuser_mount = 0;
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 37fb246..5ab6701 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -777,7 +777,6 @@ GLOBAL_EXTERN unsigned int multiuser_mount; /* if enabled allows new sessions
> >>                               have the uid/password or Kerberos credential
> >>                               or equivalent for current user */
> >>  GLOBAL_EXTERN unsigned int oplockEnabled;
> >> -GLOBAL_EXTERN unsigned int experimEnabled;
> >>  GLOBAL_EXTERN unsigned int lookupCacheEnabled;
> >>  GLOBAL_EXTERN unsigned int global_secflags;  /* if on, session setup sent
> >>                               with more secure ntlmssp2 challenge/resp */
> >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> >> index 5a28660..5f6907e 100644
> >> --- a/fs/cifs/file.c
> >> +++ b/fs/cifs/file.c
> >> @@ -1054,10 +1054,10 @@ static ssize_t cifs_write(struct cifsFileInfo *open_file,
> >>                               if (rc != 0)
> >>                                       break;
> >>                       }
> >> -                     if (experimEnabled || (pTcon->ses->server &&
> >> +                     if (pTcon->ses->server &&
> >>                               ((pTcon->ses->server->secMode &
> >>                               (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
> >> -                             == 0))) {
> >> +                             == 0)) {
> >>                               struct kvec iov[2];
> >>                               unsigned int len;
> >>
> >> @@ -1319,7 +1319,7 @@ static int cifs_writepages(struct address_space *mapping,
> >>       }
> >>
> >>       tcon = tlink_tcon(open_file->tlink);
> >> -     if (!experimEnabled && tcon->ses->server->secMode &
> >> +     if (tcon->ses->server->secMode &
> >>                       (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED)) {
> >>               cifsFileInfo_put(open_file);
> >>               kfree(iov);
> >
> > Just noticed that this misses removing the de-registration of
> > "Experimental", so it'll need to be respun.
> >
> > --
> > Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> >
> 
> 
> 


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

Re: [PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Steve French]

On Tue, Dec 7, 2010 at 1:10 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Tue, 7 Dec 2010 10:21:11 -0600
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> The issue of how to prevent a page from being modified as it is
>> written out (due to signing in our case) has been discussed on lkml a
>> few times (e.g. T10 block devices).   We should hold off on changing
>> this until we have a way of handling the case of:   we calculate the
>> signature, but just before the page in the cache is remodified, we
>> send it with the wrong signature ... obviously if we reissue the write
>> we are fine (the data is fine) but there may be better ways to lock
>> the page (and some suggestions have been made on lkml for similar
>> sounding problems).
>>
>
> This behavior has been "Experimental" for years. At what point do we
> remove this kludge? Who in their right mind is going to turn on a
> switch called "Experimental" to enable this? As a user, I'd certainly
> be reticent to do so. It's not clear what turning on "Experimental"
> would give me.

20%+ better write performance

> If you think this behavior deserves to be switchable then let's put a
> real usable switch on it. I don't think it ought to be hidden
> as /proc/fs/cifs/Experimental. Perhaps a module parameter for
> this would be more appropriate?

Probably best to work through how to fix the general issue on fsevel
(ie how to write a presumably unstable page from the cache out)



-- 
Thanks,

Steve

Re: [PATCH 4/4] cifs: remove /proc/fs/cifs/Experimental

[Jeff Layton]

On Tue, 7 Dec 2010 14:31:25 -0600
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Tue, Dec 7, 2010 at 1:10 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> > On Tue, 7 Dec 2010 10:21:11 -0600
> > Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> The issue of how to prevent a page from being modified as it is
> >> written out (due to signing in our case) has been discussed on lkml a
> >> few times (e.g. T10 block devices).   We should hold off on changing
> >> this until we have a way of handling the case of:   we calculate the
> >> signature, but just before the page in the cache is remodified, we
> >> send it with the wrong signature ... obviously if we reissue the write
> >> we are fine (the data is fine) but there may be better ways to lock
> >> the page (and some suggestions have been made on lkml for similar
> >> sounding problems).
> >>
> >
> > This behavior has been "Experimental" for years. At what point do we
> > remove this kludge? Who in their right mind is going to turn on a
> > switch called "Experimental" to enable this? As a user, I'd certainly
> > be reticent to do so. It's not clear what turning on "Experimental"
> > would give me.
> 
> 20%+ better write performance
> 
> > If you think this behavior deserves to be switchable then let's put a
> > real usable switch on it. I don't think it ought to be hidden
> > as /proc/fs/cifs/Experimental. Perhaps a module parameter for
> > this would be more appropriate?
> 
> Probably best to work through how to fix the general issue on fsevel
> (ie how to write a presumably unstable page from the cache out)
> 
> 
> 

Obviously there needs to be some sort of endgame for this code. It's
not healthy to keep this under "Experimental" for years on end. That
said, I don't really want to tackle that project at the moment. 

I am however interested in moving this code out from under this
"Experimental" procfile. Would you be amenable to a module option for
this? If so, how would you describe this knob? What name should it have?

The main reason I mention module option is that it's now very helpful
to have this sort of knob based on a file under /proc/fs/cifs. It can't
be set at boot time without a special script.

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