Re: Compatibility issue with 2.2.19pre7

From: Trond Myklebust (trond.myklebust@fys.uio.no)
Date: Thu Jan 11 2001 - 15:39:01 EST


>>>>> " " == 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/



This archive was generated by hypermail 2b29 : Mon Jan 15 2001 - 21:00:32 EST