Re: Compatibility issue with 2.2.19pre7

[Trond Myklebust <trond.myklebust@fys.uio.no>]

>>>>> " " == Andrea Arcangeli <andrea@suse.de> writes:

     > On Thu, Jan 11, 2001 at 07:30:49PM +0100, Trond Myklebust
     > wrote:
    >> OK. In that case my patch, would just be amended to eliminate
    >> the redundant comparison as is the case below.

     > This patch looks fine w.r.t. alignment but given the below
     > seems called at runtime (not just at mount time) for
     > performance and to save a dozen of bytes of kernel stack it
     > would probably better to use the nfs_fh structure in 2.2.19pre7
     > for the in-kernel representation and to define a new structure
     > for userspace message passing (defined as the nfs_fh in
     > 2.2.19pre6). But at least now we see _why_ it broke ;)

I'm not at all convinced that saving us a copy of 66 bytes in NLM is
really worth the effort. It certainly isn't worth the ugliness of
living with arrays of (void *) in order to match the alignment of a
fake pointer. I'd rather we make them integers.
If we really are to change struct nfs_fh, I therefore propose that we
also make struct nfs_fhbase reflect the fact that fb_dentry is a
32-bit cookie. That means that both nfs_fh and knfs_fh would be
integer-aligned.

The following is untested, but should be correct.

Cheers,
  Trond

diff -u --recursive --new-file linux-2.2.18/fs/lockd/svcsubs.c linux-2.2.18-fix_ppc/fs/lockd/svcsubs.c
--- linux-2.2.18/fs/lockd/svcsubs.c	Mon Dec 11 01:49:44 2000
+++ linux-2.2.18-fix_ppc/fs/lockd/svcsubs.c	Thu Jan 11 20:37:37 2001
@@ -44,6 +44,10 @@
  * Note that we open the file O_RDONLY even when creating write locks.
  * This is not quite right, but for now, we assume the client performs
  * the proper R/W checking.
+ *
+ * BEWARE:
+ * The cast to struct knfs_fh in this routine, imposes an alignment
+ * requirement on (struct nfs_fh)->data for some platforms.
  */
 u32
 nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
@@ -63,8 +67,7 @@
 	down(&nlm_file_sema);
 
 	for (file = nlm_files[hash]; file; file = file->f_next) {
-		if (file->f_handle.fh_dcookie == fh->fh_dcookie &&
-		    !memcmp(&file->f_handle, fh, sizeof(*fh)))
+		if (!memcmp(&file->f_handle, fh, sizeof(*fh)))
 			goto found;
 	}
 
diff -u --recursive --new-file linux-2.2.18/fs/nfs/inode.c linux-2.2.18-fix_ppc/fs/nfs/inode.c
--- linux-2.2.18/fs/nfs/inode.c	Mon Dec 11 01:49:44 2000
+++ linux-2.2.18-fix_ppc/fs/nfs/inode.c	Thu Jan 11 21:13:04 2001
@@ -273,9 +273,8 @@
 	struct nfs_server	*server;
 	struct rpc_xprt		*xprt = 0;
 	struct rpc_clnt		*clnt = 0;
-	struct nfs_fh		*root_fh = NULL,
-				*root = &data->root,
-				fh;
+	struct nfs_fh		fh,
+				*root_fh = NULL;
 	struct inode		*root_inode = NULL;
 	unsigned int		authflavor;
 	struct sockaddr_in	srvaddr;
@@ -290,7 +289,6 @@
 		goto failure;
 	}
 
-	memset(&fh, 0, sizeof(fh));
 	if (data->version != NFS_MOUNT_VERSION) {
 		printk(KERN_WARNING "nfs warning: mount version %s than kernel\n",
 			data->version < NFS_MOUNT_VERSION ? "older" : "newer");
@@ -298,12 +296,21 @@
 			data->namlen = 0;
 		if (data->version < 3)
 			data->bsize  = 0;
-		if (data->version < 4) {
+		if (data->version < 4)
 			data->flags &= ~NFS_MOUNT_VER3;
-			root = &fh;
-			root->size = NFS2_FHSIZE;
-			memcpy(root->data, data->old_root.data, NFS2_FHSIZE);
+	}
+
+	memset(&fh, 0, sizeof(fh));
+	if (data->version < 4) {
+		fh.size = NFS2_FHSIZE;
+		memcpy(fh.data, data->old_root.data, NFS2_FHSIZE);
+	} else {
+		fh.size = (data->flags & NFS_MOUNT_VER3) ? data->root.size : NFS2_FHSIZE;
+		if (fh.size > sizeof(fh.data)) {
+			printk(KERN_WARNING "NFS: mount program passes invalid filehandle!\n");
+			goto failure;
 		}
+		memcpy(fh.data, data->root.data, fh.size);
 	}
 
 	/* We now require that the mount process passes the remote address */
@@ -351,19 +358,15 @@
 	if (data->flags & NFS_MOUNT_VER3) {
 #ifdef CONFIG_NFS_V3
 		server->rpc_ops = &nfs_v3_clientops;
-		NFS_SB_FHSIZE(sb) = sizeof(unsigned short) + NFS3_FHSIZE;
+		NFS_SB_FHSIZE(sb) = NFS3_FHSIZE;
 		version = 3;
-		if (data->version < 4) {
-			printk(KERN_NOTICE "NFS: NFSv3 not supported by mount program.\n");
-			goto failure_unlock;
-		}
 #else
 		printk(KERN_NOTICE "NFS: NFSv3 not supported.\n");
 		goto failure_unlock;
 #endif
 	} else {
 		server->rpc_ops = &nfs_v2_clientops;
-		NFS_SB_FHSIZE(sb) = sizeof(unsigned short) + NFS2_FHSIZE;
+		NFS_SB_FHSIZE(sb) = NFS2_FHSIZE;
 		version = 2;
 	}
 
@@ -422,13 +425,14 @@
 	root_fh = nfs_fh_alloc();
 	if (!root_fh)
 		goto out_no_fh;
-	memcpy((u8*)root_fh, (u8*)root, sizeof(*root_fh));
+	memcpy((u8*)root_fh, (u8*)&fh, sizeof(*root_fh));
 
 	/* Did getting the root inode fail? */
-	if ((root->size > NFS_SB_FHSIZE(sb)
-	     || ! (root_inode = nfs_get_root(sb, root)))
+	if ((fh.size > NFS_SB_FHSIZE(sb)
+	     || ! (root_inode = nfs_get_root(sb, &fh)))
 	    && (data->flags & NFS_MOUNT_VER3)) {
 		data->flags &= ~NFS_MOUNT_VER3;
+		fh.size = NFS2_FHSIZE;
 		nfs_fh_free(root_fh);
 		rpciod_down();
 		rpc_shutdown_client(server->client);
@@ -445,7 +449,7 @@
 	sb->u.nfs_sb.s_root = root_fh;
 
 	/* Get some general file system info */
-	if (server->rpc_ops->statfs(server, root, &fsinfo) >= 0) {
+	if (server->rpc_ops->statfs(server, &fh, &fsinfo) >= 0) {
 		if (server->namelen == 0)
 			server->namelen = fsinfo.namelen;
 	} else {
diff -u --recursive --new-file linux-2.2.18/fs/nfsd/nfsfh.c linux-2.2.18-fix_ppc/fs/nfsd/nfsfh.c
--- linux-2.2.18/fs/nfsd/nfsfh.c	Mon Dec 11 01:49:44 2000
+++ linux-2.2.18-fix_ppc/fs/nfsd/nfsfh.c	Thu Jan 11 20:10:09 2001
@@ -690,7 +690,7 @@
 	fhp->fh_handle.fh_dev    = kdev_t_to_u32(parent->d_inode->i_dev);
 	fhp->fh_handle.fh_xdev   = kdev_t_to_u32(exp->ex_dev);
 	fhp->fh_handle.fh_xino   = ino_t_to_u32(exp->ex_ino);
-	fhp->fh_handle.fh_dcookie = (struct dentry *)0xfeebbaca;
+	fhp->fh_handle.fh_dcookie = 0xfeebbaca;
 	if (inode) {
 		fhp->fh_handle.fh_ino = ino_t_to_u32(inode->i_ino);
 		fhp->fh_handle.fh_generation = inode->i_generation;
diff -u --recursive --new-file linux-2.2.18/include/linux/nfs.h linux-2.2.18-fix_ppc/include/linux/nfs.h
--- linux-2.2.18/include/linux/nfs.h	Mon Dec 11 01:49:44 2000
+++ linux-2.2.18-fix_ppc/include/linux/nfs.h	Thu Jan 11 20:55:26 2001
@@ -94,8 +94,8 @@
  */
 #define NFS_MAXFHSIZE		64
 struct nfs_fh {
-	unsigned short		size;
-	unsigned char		data[NFS_MAXFHSIZE];
+	unsigned int		size;
+	unsigned int		data[NFS_MAXFHSIZE / sizeof(unsigned int)];
 };
 
 enum nfs3_stable_how {
diff -u --recursive --new-file linux-2.2.18/include/linux/nfs3.h linux-2.2.18-fix_ppc/include/linux/nfs3.h
--- linux-2.2.18/include/linux/nfs3.h	Mon Dec 11 01:49:44 2000
+++ linux-2.2.18-fix_ppc/include/linux/nfs3.h	Thu Jan 11 20:06:50 2001
@@ -58,6 +58,11 @@
 	NF3BAD  = 8
 };
 
+struct nfs3_fh {
+	unsigned short		size;
+	unsigned char		data[NFS3_FHSIZE];
+};
+
 #define NFS3_VERSION		3
 #define NFS3PROC_NULL		0
 #define NFS3PROC_GETATTR	1
diff -u --recursive --new-file linux-2.2.18/include/linux/nfs_mount.h linux-2.2.18-fix_ppc/include/linux/nfs_mount.h
--- linux-2.2.18/include/linux/nfs_mount.h	Thu Dec 14 12:30:13 2000
+++ linux-2.2.18-fix_ppc/include/linux/nfs_mount.h	Thu Jan 11 21:21:09 2001
@@ -8,9 +8,9 @@
  *
  *  structure passed from user-space to kernel-space during an nfs mount
  */
-#include <linux/nfs.h>
+#include <linux/in.h>
 #include <linux/nfs2.h>
-#include <linux/nfs_fs.h>
+#include <linux/nfs3.h>
 
 /*
  * WARNING!  Do not delete or change the order of these fields.  If
@@ -42,7 +42,7 @@
 	char		hostname[256];		/* 1 */
 	int		namlen;			/* 2 */
 	unsigned int	bsize;			/* 3 */
-	struct nfs_fh	root;			/* 4 */
+	struct nfs3_fh	root;			/* 4 */
 };
 
 /* bits in the flags field */
diff -u --recursive --new-file linux-2.2.18/include/linux/nfsd/nfsfh.h linux-2.2.18-fix_ppc/include/linux/nfsd/nfsfh.h
--- linux-2.2.18/include/linux/nfsd/nfsfh.h	Thu Dec 14 12:30:28 2000
+++ linux-2.2.18-fix_ppc/include/linux/nfsd/nfsfh.h	Thu Jan 11 20:09:31 2001
@@ -30,7 +30,7 @@
  * ino/dev of the exported inode.
  */
 struct nfs_fhbase {
-	struct dentry *	fb_dentry;	/* dentry cookie */
+	__u32		fb_dcookie;	/* dentry cookie */
 	__u32		fb_ino;		/* our inode number */
 	__u32		fb_dirino;	/* dir inode number */
 	__u32		fb_dev;		/* our device */
@@ -45,7 +45,7 @@
 	__u8			fh_cookie[NFS_FH_PADDING];
 };
 
-#define fh_dcookie		fh_base.fb_dentry
+#define fh_dcookie		fh_base.fb_dcookie
 #define fh_ino			fh_base.fb_ino
 #define fh_dirino		fh_base.fb_dirino
 #define fh_dev			fh_base.fb_dev
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/