[PATCH] 64 bit ino support for NFS server

[PATCH] 64 bit ino support for NFS server

[Peter Staubach]

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

Hi.

Attached is a patch to modify the NFS server code to support
64 bit ino's, as appropriate for the system and the NFS
protocol version.

The gist of the changes is to query the underlying file system
for attributes and not just to use the cached attributes in the
inode.  For this specific purpose, the inode only contains an
ino field which unsigned long, which is large enough on 64 bit
platforms, but is not large enough on 32 bit platforms.

    Thanx...

       ps

Signed-off-by: Peter Staubach

[-- Attachment #2: fc-6.ino64.server --]
[-- Type: text/plain, Size: 7065 bytes --]

--- linux-2.6.22.i686/fs/nfsd/nfs3xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs3xdr.c
@@ -191,7 +191,7 @@ encode_fattr3(struct svc_rqst *rqstp, __
 	*p++ = htonl((u32) MAJOR(stat->rdev));
 	*p++ = htonl((u32) MINOR(stat->rdev));
 	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) stat->ino);
+	p = xdr_encode_hyper(p, stat->ino);
 	p = encode_time3(p, &stat->atime);
 	lease_get_mtime(dentry->d_inode, &time); 
 	p = encode_time3(p, &time);
@@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
 static __be32 *
 encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
 {
-	struct inode	*inode = fhp->fh_dentry->d_inode;
+	if (!fhp->fh_post_saved) {
+		*p++ = xdr_zero;
+		return p;
+	}
 
 	/* Attributes to follow */
 	*p++ = xdr_one;
 
-	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
-	*p++ = htonl((u32) fhp->fh_post_mode);
-	*p++ = htonl((u32) fhp->fh_post_nlink);
-	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
-	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
-	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
-		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
-	} else {
-		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
-	}
-	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
-	*p++ = fhp->fh_post_rdev[0];
-	*p++ = fhp->fh_post_rdev[1];
-	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) inode->i_ino);
-	p = encode_time3(p, &fhp->fh_post_atime);
-	p = encode_time3(p, &fhp->fh_post_mtime);
-	p = encode_time3(p, &fhp->fh_post_ctime);
-
-	return p;
+	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
 }
 
 /*
@@ -284,6 +268,23 @@ encode_wcc_data(struct svc_rqst *rqstp, 
 	return encode_post_op_attr(rqstp, p, fhp);
 }
 
+/*
+ * Fill in the post_op attr for the wcc data
+ */
+void fill_post_wcc(struct svc_fh *fhp)
+{
+	int err;
+
+	if (fhp->fh_post_saved)
+		printk("nfsd: inode locked twice during operation.\n");
+
+	err = vfs_getattr(fhp->fh_export->ex_mnt, fhp->fh_dentry,
+			&fhp->fh_post_attr);
+	if (err)
+		fhp->fh_post_saved = 0;
+	else
+		fhp->fh_post_saved = 1;
+}
 
 /*
  * XDR decode functions
@@ -802,7 +803,7 @@ nfs3svc_encode_readdirres(struct svc_rqs
 
 static __be32 *
 encode_entry_baggage(struct nfsd3_readdirres *cd, __be32 *p, const char *name,
-	     int namlen, ino_t ino)
+	     int namlen, u64 ino)
 {
 	*p++ = xdr_one;				 /* mark entry present */
 	p    = xdr_encode_hyper(p, ino);	 /* file id */
@@ -873,7 +874,7 @@ compose_entry_fh(struct nfsd3_readdirres
 #define NFS3_ENTRYPLUS_BAGGAGE	(1 + 21 + 1 + (NFS3_FHSIZE >> 2))
 static int
 encode_entry(struct readdir_cd *ccd, const char *name, int namlen,
-	     loff_t offset, ino_t ino, unsigned int d_type, int plus)
+	     loff_t offset, u64 ino, unsigned int d_type, int plus)
 {
 	struct nfsd3_readdirres *cd = container_of(ccd, struct nfsd3_readdirres,
 		       					common);
--- linux-2.6.22.i686/fs/nfsd/nfsxdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfsxdr.c
@@ -523,6 +523,10 @@ nfssvc_encode_entry(void *ccdv, const ch
 		cd->common.err = nfserr_toosmall;
 		return -EINVAL;
 	}
+	if (ino > ~((u32) 0)) {
+		cd->common.err = nfserr_fbig;
+		return -EINVAL;
+	}
 	*p++ = xdr_one;				/* mark entry present */
 	*p++ = htonl((u32) ino);		/* file id */
 	p    = xdr_encode_array(p, name, namlen);/* name length & name */
--- linux-2.6.22.i686/fs/nfsd/nfs4xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs4xdr.c
@@ -1657,7 +1657,7 @@ out_acl:
 	if (bmval0 & FATTR4_WORD0_FILEID) {
 		if ((buflen -= 8) < 0)
 			goto out_resource;
-		WRITE64((u64) stat.ino);
+		WRITE64(stat.ino);
 	}
 	if (bmval0 & FATTR4_WORD0_FILES_AVAIL) {
 		if ((buflen -= 8) < 0)
@@ -1799,16 +1799,15 @@ out_acl:
 		WRITE32(stat.mtime.tv_nsec);
 	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
-		struct dentry *mnt_pnt, *mnt_root;
-
 		if ((buflen -= 8) < 0)
                 	goto out_resource;
-		mnt_root = exp->ex_mnt->mnt_root;
-		if (mnt_root->d_inode == dentry->d_inode) {
-			mnt_pnt = exp->ex_mnt->mnt_mountpoint;
-			WRITE64((u64) mnt_pnt->d_inode->i_ino);
-		} else
-                	WRITE64((u64) stat.ino);
+		if (exp->ex_mnt->mnt_root->d_inode == dentry->d_inode) {
+			err = vfs_getattr(exp->ex_mnt->mnt_parent,
+				exp->ex_mnt->mnt_mountpoint, &stat);
+			if (err)
+				goto out_nfserr;
+		}
+                WRITE64(stat.ino);
 	}
 	*attrlenp = htonl((char *)p - (char *)attrlenp - 4);
 	*countp = p - buffer;
--- linux-2.6.22.i686/include/linux/nfsd/nfsfh.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/nfsfh.h
@@ -150,17 +150,7 @@ typedef struct svc_fh {
 	struct timespec		fh_pre_ctime;	/* ctime before oper */
 
 	/* Post-op attributes saved in fh_unlock */
-	umode_t			fh_post_mode;	/* i_mode */
-	nlink_t			fh_post_nlink;	/* i_nlink */
-	uid_t			fh_post_uid;	/* i_uid */
-	gid_t			fh_post_gid;	/* i_gid */
-	__u64			fh_post_size;	/* i_size */
-	unsigned long		fh_post_blocks; /* i_blocks */
-	unsigned long		fh_post_blksize;/* i_blksize */
-	__be32			fh_post_rdev[2];/* i_rdev */
-	struct timespec		fh_post_atime;	/* i_atime */
-	struct timespec		fh_post_mtime;	/* i_mtime */
-	struct timespec		fh_post_ctime;	/* i_ctime */
+	struct kstat		fh_post_attr;	/* full attrs after operation */
 #endif /* CONFIG_NFSD_V3 */
 
 } svc_fh;
@@ -297,36 +287,12 @@ fill_pre_wcc(struct svc_fh *fhp)
 	if (!fhp->fh_pre_saved) {
 		fhp->fh_pre_mtime = inode->i_mtime;
 		fhp->fh_pre_ctime = inode->i_ctime;
-			fhp->fh_pre_size  = inode->i_size;
-			fhp->fh_pre_saved = 1;
+		fhp->fh_pre_size  = inode->i_size;
+		fhp->fh_pre_saved = 1;
 	}
 }
 
-/*
- * Fill in the post_op attr for the wcc data
- */
-static inline void
-fill_post_wcc(struct svc_fh *fhp)
-{
-	struct inode    *inode = fhp->fh_dentry->d_inode;
-
-	if (fhp->fh_post_saved)
-		printk("nfsd: inode locked twice during operation.\n");
-
-	fhp->fh_post_mode       = inode->i_mode;
-	fhp->fh_post_nlink      = inode->i_nlink;
-	fhp->fh_post_uid	= inode->i_uid;
-	fhp->fh_post_gid	= inode->i_gid;
-	fhp->fh_post_size       = inode->i_size;
-	fhp->fh_post_blksize    = BLOCK_SIZE;
-	fhp->fh_post_blocks     = inode->i_blocks;
-	fhp->fh_post_rdev[0]    = htonl((u32)imajor(inode));
-	fhp->fh_post_rdev[1]    = htonl((u32)iminor(inode));
-	fhp->fh_post_atime      = inode->i_atime;
-	fhp->fh_post_mtime      = inode->i_mtime;
-	fhp->fh_post_ctime      = inode->i_ctime;
-	fhp->fh_post_saved      = 1;
-}
+extern void fill_post_wcc(struct svc_fh *);
 #else
 #define	fill_pre_wcc(ignored)
 #define fill_post_wcc(notused)
--- linux-2.6.22.i686/include/linux/nfsd/xdr4.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/xdr4.h
@@ -421,8 +421,8 @@ set_change_info(struct nfsd4_change_info
 	cinfo->atomic = 1;
 	cinfo->before_ctime_sec = fhp->fh_pre_ctime.tv_sec;
 	cinfo->before_ctime_nsec = fhp->fh_pre_ctime.tv_nsec;
-	cinfo->after_ctime_sec = fhp->fh_post_ctime.tv_sec;
-	cinfo->after_ctime_nsec = fhp->fh_post_ctime.tv_nsec;
+	cinfo->after_ctime_sec = fhp->fh_post_attr.ctime.tv_sec;
+	cinfo->after_ctime_nsec = fhp->fh_post_attr.ctime.tv_nsec;
 }
 
 int nfs4svc_encode_voidres(struct svc_rqst *, __be32 *, void *);

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

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

Hi.

Attached is a patch to modify the NFS server code to support
64 bit ino's, as appropriate for the system and the NFS
protocol version.

The gist of the changes is to query the underlying file system
for attributes and not just to use the cached attributes in the
inode.  For this specific purpose, the inode only contains an
ino field which unsigned long, which is large enough on 64 bit
platforms, but is not large enough on 32 bit platforms.

   Thanx...

      ps

ps. Sorry about the retransmit.  I got tangled up with the mailer
and the trigger got pulled before I was ready...

Signed-off-by: Peter Staubach <staubach@redhat.com>

[-- Attachment #2: fc-6.ino64.server --]
[-- Type: text/plain, Size: 7065 bytes --]

--- linux-2.6.22.i686/fs/nfsd/nfs3xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs3xdr.c
@@ -191,7 +191,7 @@ encode_fattr3(struct svc_rqst *rqstp, __
 	*p++ = htonl((u32) MAJOR(stat->rdev));
 	*p++ = htonl((u32) MINOR(stat->rdev));
 	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) stat->ino);
+	p = xdr_encode_hyper(p, stat->ino);
 	p = encode_time3(p, &stat->atime);
 	lease_get_mtime(dentry->d_inode, &time); 
 	p = encode_time3(p, &time);
@@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
 static __be32 *
 encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
 {
-	struct inode	*inode = fhp->fh_dentry->d_inode;
+	if (!fhp->fh_post_saved) {
+		*p++ = xdr_zero;
+		return p;
+	}
 
 	/* Attributes to follow */
 	*p++ = xdr_one;
 
-	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
-	*p++ = htonl((u32) fhp->fh_post_mode);
-	*p++ = htonl((u32) fhp->fh_post_nlink);
-	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
-	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
-	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
-		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
-	} else {
-		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
-	}
-	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
-	*p++ = fhp->fh_post_rdev[0];
-	*p++ = fhp->fh_post_rdev[1];
-	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) inode->i_ino);
-	p = encode_time3(p, &fhp->fh_post_atime);
-	p = encode_time3(p, &fhp->fh_post_mtime);
-	p = encode_time3(p, &fhp->fh_post_ctime);
-
-	return p;
+	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
 }
 
 /*
@@ -284,6 +268,23 @@ encode_wcc_data(struct svc_rqst *rqstp, 
 	return encode_post_op_attr(rqstp, p, fhp);
 }
 
+/*
+ * Fill in the post_op attr for the wcc data
+ */
+void fill_post_wcc(struct svc_fh *fhp)
+{
+	int err;
+
+	if (fhp->fh_post_saved)
+		printk("nfsd: inode locked twice during operation.\n");
+
+	err = vfs_getattr(fhp->fh_export->ex_mnt, fhp->fh_dentry,
+			&fhp->fh_post_attr);
+	if (err)
+		fhp->fh_post_saved = 0;
+	else
+		fhp->fh_post_saved = 1;
+}
 
 /*
  * XDR decode functions
@@ -802,7 +803,7 @@ nfs3svc_encode_readdirres(struct svc_rqs
 
 static __be32 *
 encode_entry_baggage(struct nfsd3_readdirres *cd, __be32 *p, const char *name,
-	     int namlen, ino_t ino)
+	     int namlen, u64 ino)
 {
 	*p++ = xdr_one;				 /* mark entry present */
 	p    = xdr_encode_hyper(p, ino);	 /* file id */
@@ -873,7 +874,7 @@ compose_entry_fh(struct nfsd3_readdirres
 #define NFS3_ENTRYPLUS_BAGGAGE	(1 + 21 + 1 + (NFS3_FHSIZE >> 2))
 static int
 encode_entry(struct readdir_cd *ccd, const char *name, int namlen,
-	     loff_t offset, ino_t ino, unsigned int d_type, int plus)
+	     loff_t offset, u64 ino, unsigned int d_type, int plus)
 {
 	struct nfsd3_readdirres *cd = container_of(ccd, struct nfsd3_readdirres,
 		       					common);
--- linux-2.6.22.i686/fs/nfsd/nfsxdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfsxdr.c
@@ -523,6 +523,10 @@ nfssvc_encode_entry(void *ccdv, const ch
 		cd->common.err = nfserr_toosmall;
 		return -EINVAL;
 	}
+	if (ino > ~((u32) 0)) {
+		cd->common.err = nfserr_fbig;
+		return -EINVAL;
+	}
 	*p++ = xdr_one;				/* mark entry present */
 	*p++ = htonl((u32) ino);		/* file id */
 	p    = xdr_encode_array(p, name, namlen);/* name length & name */
--- linux-2.6.22.i686/fs/nfsd/nfs4xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs4xdr.c
@@ -1657,7 +1657,7 @@ out_acl:
 	if (bmval0 & FATTR4_WORD0_FILEID) {
 		if ((buflen -= 8) < 0)
 			goto out_resource;
-		WRITE64((u64) stat.ino);
+		WRITE64(stat.ino);
 	}
 	if (bmval0 & FATTR4_WORD0_FILES_AVAIL) {
 		if ((buflen -= 8) < 0)
@@ -1799,16 +1799,15 @@ out_acl:
 		WRITE32(stat.mtime.tv_nsec);
 	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
-		struct dentry *mnt_pnt, *mnt_root;
-
 		if ((buflen -= 8) < 0)
                 	goto out_resource;
-		mnt_root = exp->ex_mnt->mnt_root;
-		if (mnt_root->d_inode == dentry->d_inode) {
-			mnt_pnt = exp->ex_mnt->mnt_mountpoint;
-			WRITE64((u64) mnt_pnt->d_inode->i_ino);
-		} else
-                	WRITE64((u64) stat.ino);
+		if (exp->ex_mnt->mnt_root->d_inode == dentry->d_inode) {
+			err = vfs_getattr(exp->ex_mnt->mnt_parent,
+				exp->ex_mnt->mnt_mountpoint, &stat);
+			if (err)
+				goto out_nfserr;
+		}
+                WRITE64(stat.ino);
 	}
 	*attrlenp = htonl((char *)p - (char *)attrlenp - 4);
 	*countp = p - buffer;
--- linux-2.6.22.i686/include/linux/nfsd/nfsfh.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/nfsfh.h
@@ -150,17 +150,7 @@ typedef struct svc_fh {
 	struct timespec		fh_pre_ctime;	/* ctime before oper */
 
 	/* Post-op attributes saved in fh_unlock */
-	umode_t			fh_post_mode;	/* i_mode */
-	nlink_t			fh_post_nlink;	/* i_nlink */
-	uid_t			fh_post_uid;	/* i_uid */
-	gid_t			fh_post_gid;	/* i_gid */
-	__u64			fh_post_size;	/* i_size */
-	unsigned long		fh_post_blocks; /* i_blocks */
-	unsigned long		fh_post_blksize;/* i_blksize */
-	__be32			fh_post_rdev[2];/* i_rdev */
-	struct timespec		fh_post_atime;	/* i_atime */
-	struct timespec		fh_post_mtime;	/* i_mtime */
-	struct timespec		fh_post_ctime;	/* i_ctime */
+	struct kstat		fh_post_attr;	/* full attrs after operation */
 #endif /* CONFIG_NFSD_V3 */
 
 } svc_fh;
@@ -297,36 +287,12 @@ fill_pre_wcc(struct svc_fh *fhp)
 	if (!fhp->fh_pre_saved) {
 		fhp->fh_pre_mtime = inode->i_mtime;
 		fhp->fh_pre_ctime = inode->i_ctime;
-			fhp->fh_pre_size  = inode->i_size;
-			fhp->fh_pre_saved = 1;
+		fhp->fh_pre_size  = inode->i_size;
+		fhp->fh_pre_saved = 1;
 	}
 }
 
-/*
- * Fill in the post_op attr for the wcc data
- */
-static inline void
-fill_post_wcc(struct svc_fh *fhp)
-{
-	struct inode    *inode = fhp->fh_dentry->d_inode;
-
-	if (fhp->fh_post_saved)
-		printk("nfsd: inode locked twice during operation.\n");
-
-	fhp->fh_post_mode       = inode->i_mode;
-	fhp->fh_post_nlink      = inode->i_nlink;
-	fhp->fh_post_uid	= inode->i_uid;
-	fhp->fh_post_gid	= inode->i_gid;
-	fhp->fh_post_size       = inode->i_size;
-	fhp->fh_post_blksize    = BLOCK_SIZE;
-	fhp->fh_post_blocks     = inode->i_blocks;
-	fhp->fh_post_rdev[0]    = htonl((u32)imajor(inode));
-	fhp->fh_post_rdev[1]    = htonl((u32)iminor(inode));
-	fhp->fh_post_atime      = inode->i_atime;
-	fhp->fh_post_mtime      = inode->i_mtime;
-	fhp->fh_post_ctime      = inode->i_ctime;
-	fhp->fh_post_saved      = 1;
-}
+extern void fill_post_wcc(struct svc_fh *);
 #else
 #define	fill_pre_wcc(ignored)
 #define fill_post_wcc(notused)
--- linux-2.6.22.i686/include/linux/nfsd/xdr4.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/xdr4.h
@@ -421,8 +421,8 @@ set_change_info(struct nfsd4_change_info
 	cinfo->atomic = 1;
 	cinfo->before_ctime_sec = fhp->fh_pre_ctime.tv_sec;
 	cinfo->before_ctime_nsec = fhp->fh_pre_ctime.tv_nsec;
-	cinfo->after_ctime_sec = fhp->fh_post_ctime.tv_sec;
-	cinfo->after_ctime_nsec = fhp->fh_post_ctime.tv_nsec;
+	cinfo->after_ctime_sec = fhp->fh_post_attr.ctime.tv_sec;
+	cinfo->after_ctime_nsec = fhp->fh_post_attr.ctime.tv_nsec;
 }
 
 int nfs4svc_encode_voidres(struct svc_rqst *, __be32 *, void *);

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Fri, Aug 03, 2007 at 03:29:10PM -0400, Peter Staubach wrote:
> Hi.
>
> Attached is a patch to modify the NFS server code to support
> 64 bit ino's, as appropriate for the system and the NFS
> protocol version.
>
> The gist of the changes is to query the underlying file system
> for attributes and not just to use the cached attributes in the
> inode.  For this specific purpose, the inode only contains an
> ino field which unsigned long, which is large enough on 64 bit
> platforms, but is not large enough on 32 bit platforms.

Thanks!

> @@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
>  static __be32 *
>  encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>  {
> -	struct inode	*inode = fhp->fh_dentry->d_inode;
> +	if (!fhp->fh_post_saved) {
> +		*p++ = xdr_zero;
> +		return p;
> +	}

The caller, encode_wcc_data(), already did this check.

>  	/* Attributes to follow */
>  	*p++ = xdr_one;
>  
> -	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
> -	*p++ = htonl((u32) fhp->fh_post_mode);
> -	*p++ = htonl((u32) fhp->fh_post_nlink);
> -	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
> -	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
> -	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
> -		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
> -	} else {
> -		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
> -	}
> -	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
> -	*p++ = fhp->fh_post_rdev[0];
> -	*p++ = fhp->fh_post_rdev[1];
> -	p = encode_fsid(p, fhp);
> -	p = xdr_encode_hyper(p, (u64) inode->i_ino);
> -	p = encode_time3(p, &fhp->fh_post_atime);
> -	p = encode_time3(p, &fhp->fh_post_mtime);
> -	p = encode_time3(p, &fhp->fh_post_ctime);
> -
> -	return p;
> +	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);

Is there a problem with the lease_get_mtime() call in encode_fattr3()?
It looks like that could return the current time rather than the time
that was supposedly atomic with respect to the operation.

Dumb question: I assume it's always legal to call ->getattr while
holding the i_mutex?

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

J. Bruce Fields wrote:
> On Fri, Aug 03, 2007 at 03:29:10PM -0400, Peter Staubach wrote:
>   
>> Hi.
>>
>> Attached is a patch to modify the NFS server code to support
>> 64 bit ino's, as appropriate for the system and the NFS
>> protocol version.
>>
>> The gist of the changes is to query the underlying file system
>> for attributes and not just to use the cached attributes in the
>> inode.  For this specific purpose, the inode only contains an
>> ino field which unsigned long, which is large enough on 64 bit
>> platforms, but is not large enough on 32 bit platforms.
>>     
>
> Thanks!
>
>   
>> @@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
>>  static __be32 *
>>  encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>>  {
>> -	struct inode	*inode = fhp->fh_dentry->d_inode;
>> +	if (!fhp->fh_post_saved) {
>> +		*p++ = xdr_zero;
>> +		return p;
>> +	}
>>     
>
> The caller, encode_wcc_data(), already did this check.
>
>   

True.  What a twisty maze of small passages, all alike.

The current algorithms don't look quite right to me.  I don't
think that it is valid to return a post_op_attr, which is not
atomic w.r.t. the operation, even when not returning a pre_op_attr.
Perhaps a little more cleanup might be good.  I will look into
this.

>>  	/* Attributes to follow */
>>  	*p++ = xdr_one;
>>  
>> -	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
>> -	*p++ = htonl((u32) fhp->fh_post_mode);
>> -	*p++ = htonl((u32) fhp->fh_post_nlink);
>> -	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
>> -	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
>> -	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
>> -		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
>> -	} else {
>> -		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
>> -	}
>> -	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
>> -	*p++ = fhp->fh_post_rdev[0];
>> -	*p++ = fhp->fh_post_rdev[1];
>> -	p = encode_fsid(p, fhp);
>> -	p = xdr_encode_hyper(p, (u64) inode->i_ino);
>> -	p = encode_time3(p, &fhp->fh_post_atime);
>> -	p = encode_time3(p, &fhp->fh_post_mtime);
>> -	p = encode_time3(p, &fhp->fh_post_ctime);
>> -
>> -	return p;
>> +	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
>>     
>
> Is there a problem with the lease_get_mtime() call in encode_fattr3()?
> It looks like that could return the current time rather than the time
> that was supposedly atomic with respect to the operation.
>
>   

Sorry, I glossed over that one.  Let me address that.  Good catch.

> Dumb question: I assume it's always legal to call ->getattr while
> holding the i_mutex?
>   

Not so dumb, and I couldn't find an answer to that, either way.
I couldn't find any reason why it would be illegal, but neither
did I find explicit reasons for why it would be legal.

Does anyone else know?  This gets into the lack of complete
definition for the virtual file system layer...

Thanx for the review, Bruce.

    Thanx...

       ps


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Mon, Aug 06, 2007 at 11:40:58AM -0400, Peter Staubach wrote:
> J. Bruce Fields wrote:
>> On Fri, Aug 03, 2007 at 03:29:10PM -0400, Peter Staubach wrote:
>>> @@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
>>>  static __be32 *
>>>  encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh 
>>> *fhp)
>>>  {
>>> -	struct inode	*inode = fhp->fh_dentry->d_inode;
>>> +	if (!fhp->fh_post_saved) {
>>> +		*p++ = xdr_zero;
>>> +		return p;
>>> +	}
>>>     
>>
>> The caller, encode_wcc_data(), already did this check.
>
> True.  What a twisty maze of small passages, all alike.
>
> The current algorithms don't look quite right to me.  I don't
> think that it is valid to return a post_op_attr, which is not
> atomic w.r.t. the operation, even when not returning a pre_op_attr.
> Perhaps a little more cleanup might be good.  I will look into
> this.

I *think* the current code is correct, but I agree that it's worth
another check--thanks!

I've also never been really fond of the combined handling of fh_{un}lock
functions that combine the locking and attribute gathering.  But maybe
it's just me--I haven't really thought about that code.

>> Dumb question: I assume it's always legal to call ->getattr while
>> holding the i_mutex?
>>   
>
> Not so dumb, and I couldn't find an answer to that, either way.
> I couldn't find any reason why it would be illegal, but neither
> did I find explicit reasons for why it would be legal.
>
> Does anyone else know?  This gets into the lack of complete
> definition for the virtual file system layer...

There's a useful file Documentation/filesystems/Locking.  It has a table
that explains which operations take the i_mutex and which don't.  I
assume that a "no" in the table means that function isn't usually called
with the i_mutex, not that it necessarily *must* not be.  But I could be
wrong.

That table could use a caption....

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

J. Bruce Fields wrote:
> On Fri, Aug 03, 2007 at 03:29:10PM -0400, Peter Staubach wrote:
>   
>> Hi.
>>
>> Attached is a patch to modify the NFS server code to support
>> 64 bit ino's, as appropriate for the system and the NFS
>> protocol version.
>>
>> The gist of the changes is to query the underlying file system
>> for attributes and not just to use the cached attributes in the
>> inode.  For this specific purpose, the inode only contains an
>> ino field which unsigned long, which is large enough on 64 bit
>> platforms, but is not large enough on 32 bit platforms.
>>     
>
> Thanks!
>
>   
>> @@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
>>  static __be32 *
>>  encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>>  {
>> -	struct inode	*inode = fhp->fh_dentry->d_inode;
>> +	if (!fhp->fh_post_saved) {
>> +		*p++ = xdr_zero;
>> +		return p;
>> +	}
>>     
>
> The caller, encode_wcc_data(), already did this check.
>
>   
>>  	/* Attributes to follow */
>>  	*p++ = xdr_one;
>>  
>> -	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
>> -	*p++ = htonl((u32) fhp->fh_post_mode);
>> -	*p++ = htonl((u32) fhp->fh_post_nlink);
>> -	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
>> -	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
>> -	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
>> -		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
>> -	} else {
>> -		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
>> -	}
>> -	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
>> -	*p++ = fhp->fh_post_rdev[0];
>> -	*p++ = fhp->fh_post_rdev[1];
>> -	p = encode_fsid(p, fhp);
>> -	p = xdr_encode_hyper(p, (u64) inode->i_ino);
>> -	p = encode_time3(p, &fhp->fh_post_atime);
>> -	p = encode_time3(p, &fhp->fh_post_mtime);
>> -	p = encode_time3(p, &fhp->fh_post_ctime);
>> -
>> -	return p;
>> +	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
>>     
>
> Is there a problem with the lease_get_mtime() call in encode_fattr3()?
> It looks like that could return the current time rather than the time
> that was supposedly atomic with respect to the operation.
>
>   

I'm testing a new version of the patch, which addresses this
issue, but then I got to wondering, what is lease_get_mtime()
really for and is it solving a real problem, and if so, is it
solving it in a reasonable fashion?

It appears to me that it is used to detect when an application
on the server has acquired an exclusive lease for a file and
may be modifying it, but without informing the kernel.  If it
was informing the kernel, then the mtime would be updated.

NFS doesn't support leases like this, so it would need to be
an application running on the server, either as a standalone
application or as a server for some other service.

Outside of delegations, we don't support anything else like
this, so why is this a special case?  Was there a real world
problem that this solved?

And, why does it matter whether the attributes are being
returned via GETATTR, via a post_op_attr, or via a post_op_attr
as part of a wcc_data?  The first two cases invoke lease_get_mtime(),
while the third does not.  Could this not lead to time jumping
around on a particular file, forwards and backwards?

    Thanx...

       ps

> Dumb question: I assume it's always legal to call ->getattr while
> holding the i_mutex?
>
> --b.
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Wed, Aug 08, 2007 at 04:07:37PM -0400, Peter Staubach wrote:
> I'm testing a new version of the patch, which addresses this
> issue, but then I got to wondering, what is lease_get_mtime()
> really for and is it solving a real problem, and if so, is it
> solving it in a reasonable fashion?
>
> It appears to me that it is used to detect when an application
> on the server has acquired an exclusive lease for a file and
> may be modifying it, but without informing the kernel.  If it
> was informing the kernel, then the mtime would be updated.
>
> NFS doesn't support leases like this, so it would need to be
> an application running on the server, either as a standalone
> application or as a server for some other service.

Let's say it's Samba, since that's probably what is is.

So Samba is holding a write lease on the file on behalf of one of its
clients.  We never see an open from our NFSv2/v3 client--if it doesn't
have data cached, we may never even see a read, just access and
getattr--so we don't know to break that lease.

(Actually, if a write lease, like an NFSv4 write delegation, is meant to
cover file attributes as well as data, then perhaps stat should break
write leases.  That sounds painful.)

Anyway, the delay between the time the Samba client makes a change and
the time the NFS client sees it could be unbounded.  Whereas with
lease_get_mtime() the client will invalidate its cache and perform a
read, which *does* break the lease (in nfsd_open).

But I agree that this is a strange compromise, and I'd like to
understand better what the semantics for sharing with a Samba client are
supposed to be.

> And, why does it matter whether the attributes are being
> returned via GETATTR, via a post_op_attr, or via a post_op_attr
> as part of a wcc_data?  The first two cases invoke lease_get_mtime(),
> while the third does not.

What's the code path in the third case?

> Could this not lead to time jumping around on a particular file,
> forwards and backwards?

Yeah, sounds bad.

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

J. Bruce Fields wrote:
> On Wed, Aug 08, 2007 at 04:07:37PM -0400, Peter Staubach wrote:
>   
>> I'm testing a new version of the patch, which addresses this
>> issue, but then I got to wondering, what is lease_get_mtime()
>> really for and is it solving a real problem, and if so, is it
>> solving it in a reasonable fashion?
>>
>> It appears to me that it is used to detect when an application
>> on the server has acquired an exclusive lease for a file and
>> may be modifying it, but without informing the kernel.  If it
>> was informing the kernel, then the mtime would be updated.
>>
>> NFS doesn't support leases like this, so it would need to be
>> an application running on the server, either as a standalone
>> application or as a server for some other service.
>>     
>
> Let's say it's Samba, since that's probably what is is.
>
> So Samba is holding a write lease on the file on behalf of one of its
> clients.  We never see an open from our NFSv2/v3 client--if it doesn't
> have data cached, we may never even see a read, just access and
> getattr--so we don't know to break that lease.
>
> (Actually, if a write lease, like an NFSv4 write delegation, is meant to
> cover file attributes as well as data, then perhaps stat should break
> write leases.  That sounds painful.)
>
> Anyway, the delay between the time the Samba client makes a change and
> the time the NFS client sees it could be unbounded.  Whereas with
> lease_get_mtime() the client will invalidate its cache and perform a
> read, which *does* break the lease (in nfsd_open).
>
> But I agree that this is a strange compromise, and I'd like to
> understand better what the semantics for sharing with a Samba client are
> supposed to be.
>
>   

Yes.

>> And, why does it matter whether the attributes are being
>> returned via GETATTR, via a post_op_attr, or via a post_op_attr
>> as part of a wcc_data?  The first two cases invoke lease_get_mtime(),
>> while the third does not.
>>     
>
> What's the code path in the third case?
>
>   

The support in fill_post_wcc() doesn't invoke lease_get_mtime().

    Thanx...

       ps

>> Could this not lead to time jumping around on a particular file,
>> forwards and backwards?
>>     
>
> Yeah, sounds bad.
>
> --b.
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Wed, Aug 08, 2007 at 04:49:00PM -0400, Peter Staubach wrote:
> J. Bruce Fields wrote:
> > So Samba is holding a write lease on the file on behalf of one of its
> > clients.  We never see an open from our NFSv2/v3 client--if it doesn't
> > have data cached, we may never even see a read, just access and
> > getattr--so we don't know to break that lease.
> >
> > (Actually, if a write lease, like an NFSv4 write delegation, is meant to
> > cover file attributes as well as data, then perhaps stat should break
> > write leases.  That sounds painful.)
> >
> > Anyway, the delay between the time the Samba client makes a change and
> > the time the NFS client sees it could be unbounded.  Whereas with
> > lease_get_mtime() the client will invalidate its cache and perform a
> > read, which *does* break the lease (in nfsd_open).
> >
> > But I agree that this is a strange compromise, and I'd like to
> > understand better what the semantics for sharing with a Samba client are
> > supposed to be.
> 
> Yes.

There's a similar problem in the case of, say, a server with both a v3
and a v4 client.  If the v4 client has a write delegation, then we break
close-to-open semantics if the v3 client doesn't see change attribute
updates from the v4 client's local modifications.  So that's what we
have cb_getattr for.  And if you don't want to do cb_getattr calls then
I guess you should be breaking the lease.

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

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

J. Bruce Fields wrote:
> On Fri, Aug 03, 2007 at 03:29:10PM -0400, Peter Staubach wrote:
>   
>> Hi.
>>
>> Attached is a patch to modify the NFS server code to support
>> 64 bit ino's, as appropriate for the system and the NFS
>> protocol version.
>>
>> The gist of the changes is to query the underlying file system
>> for attributes and not just to use the cached attributes in the
>> inode.  For this specific purpose, the inode only contains an
>> ino field which unsigned long, which is large enough on 64 bit
>> platforms, but is not large enough on 32 bit platforms.
>>     
>
> Thanks!
>
>   
>> @@ -203,31 +203,15 @@ encode_fattr3(struct svc_rqst *rqstp, __
>>  static __be32 *
>>  encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
>>  {
>> -	struct inode	*inode = fhp->fh_dentry->d_inode;
>> +	if (!fhp->fh_post_saved) {
>> +		*p++ = xdr_zero;
>> +		return p;
>> +	}
>>     
>
> The caller, encode_wcc_data(), already did this check.
>
>   
>>  	/* Attributes to follow */
>>  	*p++ = xdr_one;
>>  
>> -	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
>> -	*p++ = htonl((u32) fhp->fh_post_mode);
>> -	*p++ = htonl((u32) fhp->fh_post_nlink);
>> -	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
>> -	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
>> -	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
>> -		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
>> -	} else {
>> -		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
>> -	}
>> -	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
>> -	*p++ = fhp->fh_post_rdev[0];
>> -	*p++ = fhp->fh_post_rdev[1];
>> -	p = encode_fsid(p, fhp);
>> -	p = xdr_encode_hyper(p, (u64) inode->i_ino);
>> -	p = encode_time3(p, &fhp->fh_post_atime);
>> -	p = encode_time3(p, &fhp->fh_post_mtime);
>> -	p = encode_time3(p, &fhp->fh_post_ctime);
>> -
>> -	return p;
>> +	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
>>     
>
> Is there a problem with the lease_get_mtime() call in encode_fattr3()?
> It looks like that could return the current time rather than the time
> that was supposedly atomic with respect to the operation.
>
> Dumb question: I assume it's always legal to call ->getattr while
> holding the i_mutex?

Hi.

Attached is a new patch which should address the issues raised
by Bruce.

I haven't been able to find any reason why ->getattr can't be called
while i_mutex.  The specification indicates that i_mutex is not
required to be held in order to invoke ->getattr, but it doesn't say
that i_mutex can't be held while invoking ->getattr.

I also haven't come to any conclusions regarding the value of
lease_get_mtime() and whether it should or should not be invoked
by fill_post_wcc() too.  I chose not to change this because I
thought that it was safer to leave well enough alone.  If we
decide to make a change, it can be done separately.

So, here we go again -- :-)

---

Attached is a patch to modify the NFS server code to support
64 bit ino's, as appropriate for the system and the NFS
protocol version.

The gist of the changes is to query the underlying file system
for attributes and not just to use the cached attributes in the
inode.  For this specific purpose, the inode only contains an
ino field which unsigned long, which is large enough on 64 bit
platforms, but is not large enough on 32 bit platforms.

    Thanx...

        ps

Signed-off-by: Peter Staubach <staubach@redhat.com>





[-- Attachment #2: fc-6.ino64.server.2 --]
[-- Type: text/plain, Size: 8062 bytes --]

--- linux-2.6.22.i686/fs/nfsd/nfs3xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs3xdr.c
@@ -174,9 +174,6 @@ static __be32 *
 encode_fattr3(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
 	      struct kstat *stat)
 {
-	struct dentry	*dentry = fhp->fh_dentry;
-	struct timespec time;
-
 	*p++ = htonl(nfs3_ftypes[(stat->mode & S_IFMT) >> 12]);
 	*p++ = htonl((u32) stat->mode);
 	*p++ = htonl((u32) stat->nlink);
@@ -191,10 +188,9 @@ encode_fattr3(struct svc_rqst *rqstp, __
 	*p++ = htonl((u32) MAJOR(stat->rdev));
 	*p++ = htonl((u32) MINOR(stat->rdev));
 	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) stat->ino);
+	p = xdr_encode_hyper(p, stat->ino);
 	p = encode_time3(p, &stat->atime);
-	lease_get_mtime(dentry->d_inode, &time); 
-	p = encode_time3(p, &time);
+	p = encode_time3(p, &stat->mtime);
 	p = encode_time3(p, &stat->ctime);
 
 	return p;
@@ -203,31 +199,9 @@ encode_fattr3(struct svc_rqst *rqstp, __
 static __be32 *
 encode_saved_post_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
 {
-	struct inode	*inode = fhp->fh_dentry->d_inode;
-
 	/* Attributes to follow */
 	*p++ = xdr_one;
-
-	*p++ = htonl(nfs3_ftypes[(fhp->fh_post_mode & S_IFMT) >> 12]);
-	*p++ = htonl((u32) fhp->fh_post_mode);
-	*p++ = htonl((u32) fhp->fh_post_nlink);
-	*p++ = htonl((u32) nfsd_ruid(rqstp, fhp->fh_post_uid));
-	*p++ = htonl((u32) nfsd_rgid(rqstp, fhp->fh_post_gid));
-	if (S_ISLNK(fhp->fh_post_mode) && fhp->fh_post_size > NFS3_MAXPATHLEN) {
-		p = xdr_encode_hyper(p, (u64) NFS3_MAXPATHLEN);
-	} else {
-		p = xdr_encode_hyper(p, (u64) fhp->fh_post_size);
-	}
-	p = xdr_encode_hyper(p, ((u64)fhp->fh_post_blocks) << 9);
-	*p++ = fhp->fh_post_rdev[0];
-	*p++ = fhp->fh_post_rdev[1];
-	p = encode_fsid(p, fhp);
-	p = xdr_encode_hyper(p, (u64) inode->i_ino);
-	p = encode_time3(p, &fhp->fh_post_atime);
-	p = encode_time3(p, &fhp->fh_post_mtime);
-	p = encode_time3(p, &fhp->fh_post_ctime);
-
-	return p;
+	return encode_fattr3(rqstp, p, fhp, &fhp->fh_post_attr);
 }
 
 /*
@@ -246,6 +220,7 @@ encode_post_op_attr(struct svc_rqst *rqs
 		err = vfs_getattr(fhp->fh_export->ex_mnt, dentry, &stat);
 		if (!err) {
 			*p++ = xdr_one;		/* attributes follow */
+			lease_get_mtime(dentry->d_inode, &stat.mtime); 
 			return encode_fattr3(rqstp, p, fhp, &stat);
 		}
 	}
@@ -284,6 +259,23 @@ encode_wcc_data(struct svc_rqst *rqstp, 
 	return encode_post_op_attr(rqstp, p, fhp);
 }
 
+/*
+ * Fill in the post_op attr for the wcc data
+ */
+void fill_post_wcc(struct svc_fh *fhp)
+{
+	int err;
+
+	if (fhp->fh_post_saved)
+		printk("nfsd: inode locked twice during operation.\n");
+
+	err = vfs_getattr(fhp->fh_export->ex_mnt, fhp->fh_dentry,
+			&fhp->fh_post_attr);
+	if (err)
+		fhp->fh_post_saved = 0;
+	else
+		fhp->fh_post_saved = 1;
+}
 
 /*
  * XDR decode functions
@@ -643,8 +635,11 @@ int
 nfs3svc_encode_attrstat(struct svc_rqst *rqstp, __be32 *p,
 					struct nfsd3_attrstat *resp)
 {
-	if (resp->status == 0)
+	if (resp->status == 0) {
+		lease_get_mtime(resp->fh.fh_dentry->d_inode,
+				&resp->stat.mtime); 
 		p = encode_fattr3(rqstp, p, &resp->fh, &resp->stat);
+	}
 	return xdr_ressize_check(rqstp, p);
 }
 
@@ -802,7 +797,7 @@ nfs3svc_encode_readdirres(struct svc_rqs
 
 static __be32 *
 encode_entry_baggage(struct nfsd3_readdirres *cd, __be32 *p, const char *name,
-	     int namlen, ino_t ino)
+	     int namlen, u64 ino)
 {
 	*p++ = xdr_one;				 /* mark entry present */
 	p    = xdr_encode_hyper(p, ino);	 /* file id */
@@ -873,7 +868,7 @@ compose_entry_fh(struct nfsd3_readdirres
 #define NFS3_ENTRYPLUS_BAGGAGE	(1 + 21 + 1 + (NFS3_FHSIZE >> 2))
 static int
 encode_entry(struct readdir_cd *ccd, const char *name, int namlen,
-	     loff_t offset, ino_t ino, unsigned int d_type, int plus)
+	     loff_t offset, u64 ino, unsigned int d_type, int plus)
 {
 	struct nfsd3_readdirres *cd = container_of(ccd, struct nfsd3_readdirres,
 		       					common);
--- linux-2.6.22.i686/fs/nfsd/nfsxdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfsxdr.c
@@ -523,6 +523,10 @@ nfssvc_encode_entry(void *ccdv, const ch
 		cd->common.err = nfserr_toosmall;
 		return -EINVAL;
 	}
+	if (ino > ~((u32) 0)) {
+		cd->common.err = nfserr_fbig;
+		return -EINVAL;
+	}
 	*p++ = xdr_one;				/* mark entry present */
 	*p++ = htonl((u32) ino);		/* file id */
 	p    = xdr_encode_array(p, name, namlen);/* name length & name */
--- linux-2.6.22.i686/fs/nfsd/nfs4xdr.c.org
+++ linux-2.6.22.i686/fs/nfsd/nfs4xdr.c
@@ -1657,7 +1657,7 @@ out_acl:
 	if (bmval0 & FATTR4_WORD0_FILEID) {
 		if ((buflen -= 8) < 0)
 			goto out_resource;
-		WRITE64((u64) stat.ino);
+		WRITE64(stat.ino);
 	}
 	if (bmval0 & FATTR4_WORD0_FILES_AVAIL) {
 		if ((buflen -= 8) < 0)
@@ -1799,16 +1799,15 @@ out_acl:
 		WRITE32(stat.mtime.tv_nsec);
 	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
-		struct dentry *mnt_pnt, *mnt_root;
-
 		if ((buflen -= 8) < 0)
                 	goto out_resource;
-		mnt_root = exp->ex_mnt->mnt_root;
-		if (mnt_root->d_inode == dentry->d_inode) {
-			mnt_pnt = exp->ex_mnt->mnt_mountpoint;
-			WRITE64((u64) mnt_pnt->d_inode->i_ino);
-		} else
-                	WRITE64((u64) stat.ino);
+		if (exp->ex_mnt->mnt_root->d_inode == dentry->d_inode) {
+			err = vfs_getattr(exp->ex_mnt->mnt_parent,
+				exp->ex_mnt->mnt_mountpoint, &stat);
+			if (err)
+				goto out_nfserr;
+		}
+                WRITE64(stat.ino);
 	}
 	*attrlenp = htonl((char *)p - (char *)attrlenp - 4);
 	*countp = p - buffer;
--- linux-2.6.22.i686/include/linux/nfsd/nfsfh.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/nfsfh.h
@@ -150,17 +150,7 @@ typedef struct svc_fh {
 	struct timespec		fh_pre_ctime;	/* ctime before oper */
 
 	/* Post-op attributes saved in fh_unlock */
-	umode_t			fh_post_mode;	/* i_mode */
-	nlink_t			fh_post_nlink;	/* i_nlink */
-	uid_t			fh_post_uid;	/* i_uid */
-	gid_t			fh_post_gid;	/* i_gid */
-	__u64			fh_post_size;	/* i_size */
-	unsigned long		fh_post_blocks; /* i_blocks */
-	unsigned long		fh_post_blksize;/* i_blksize */
-	__be32			fh_post_rdev[2];/* i_rdev */
-	struct timespec		fh_post_atime;	/* i_atime */
-	struct timespec		fh_post_mtime;	/* i_mtime */
-	struct timespec		fh_post_ctime;	/* i_ctime */
+	struct kstat		fh_post_attr;	/* full attrs after operation */
 #endif /* CONFIG_NFSD_V3 */
 
 } svc_fh;
@@ -297,36 +287,12 @@ fill_pre_wcc(struct svc_fh *fhp)
 	if (!fhp->fh_pre_saved) {
 		fhp->fh_pre_mtime = inode->i_mtime;
 		fhp->fh_pre_ctime = inode->i_ctime;
-			fhp->fh_pre_size  = inode->i_size;
-			fhp->fh_pre_saved = 1;
+		fhp->fh_pre_size  = inode->i_size;
+		fhp->fh_pre_saved = 1;
 	}
 }
 
-/*
- * Fill in the post_op attr for the wcc data
- */
-static inline void
-fill_post_wcc(struct svc_fh *fhp)
-{
-	struct inode    *inode = fhp->fh_dentry->d_inode;
-
-	if (fhp->fh_post_saved)
-		printk("nfsd: inode locked twice during operation.\n");
-
-	fhp->fh_post_mode       = inode->i_mode;
-	fhp->fh_post_nlink      = inode->i_nlink;
-	fhp->fh_post_uid	= inode->i_uid;
-	fhp->fh_post_gid	= inode->i_gid;
-	fhp->fh_post_size       = inode->i_size;
-	fhp->fh_post_blksize    = BLOCK_SIZE;
-	fhp->fh_post_blocks     = inode->i_blocks;
-	fhp->fh_post_rdev[0]    = htonl((u32)imajor(inode));
-	fhp->fh_post_rdev[1]    = htonl((u32)iminor(inode));
-	fhp->fh_post_atime      = inode->i_atime;
-	fhp->fh_post_mtime      = inode->i_mtime;
-	fhp->fh_post_ctime      = inode->i_ctime;
-	fhp->fh_post_saved      = 1;
-}
+extern void fill_post_wcc(struct svc_fh *);
 #else
 #define	fill_pre_wcc(ignored)
 #define fill_post_wcc(notused)
--- linux-2.6.22.i686/include/linux/nfsd/xdr4.h.org
+++ linux-2.6.22.i686/include/linux/nfsd/xdr4.h
@@ -421,8 +421,8 @@ set_change_info(struct nfsd4_change_info
 	cinfo->atomic = 1;
 	cinfo->before_ctime_sec = fhp->fh_pre_ctime.tv_sec;
 	cinfo->before_ctime_nsec = fhp->fh_pre_ctime.tv_nsec;
-	cinfo->after_ctime_sec = fhp->fh_post_ctime.tv_sec;
-	cinfo->after_ctime_nsec = fhp->fh_post_ctime.tv_nsec;
+	cinfo->after_ctime_sec = fhp->fh_post_attr.ctime.tv_sec;
+	cinfo->after_ctime_nsec = fhp->fh_post_attr.ctime.tv_nsec;
 }
 
 int nfs4svc_encode_voidres(struct svc_rqst *, __be32 *, void *);

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Thu, Aug 16, 2007 at 12:10:07PM -0400, Peter Staubach wrote:
> Attached is a new patch which should address the issues raised
> by Bruce.

Thanks!

> I also haven't come to any conclusions regarding the value of
> lease_get_mtime() and whether it should or should not be invoked
> by fill_post_wcc() too.  I chose not to change this because I
> thought that it was safer to leave well enough alone.  If we
> decide to make a change, it can be done separately.

OK.

Only superficial complaints:
	- There were some minor whitespace oddities; running the patch
	  through scripts/checkpatch.pl may be the quickest way to catch
	  those.
	- This would be better as two, maybe three separate patches;
	  e.g.  moving the lease_get_mtime out of encode_fattr3 could be
	  done separately first.  Ideally we'd do some trivial
	  transformations like that, followed by one change that
	  actually changes the inode behavior.  That makes the whole
	  thing trival to review.

I fixed up the first and added the result to

	git://linux-nfs.org/~bfields/linux.git for-mm

so it should show up in the next -mm.  I'd happily replace it by a more
finely split up version if that was something you could whip up in a few
minutes.

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[Peter Staubach]

J. Bruce Fields wrote:
> On Thu, Aug 16, 2007 at 12:10:07PM -0400, Peter Staubach wrote:
>   
>> Attached is a new patch which should address the issues raised
>> by Bruce.
>>     
>
> Thanks!
>
>   
>> I also haven't come to any conclusions regarding the value of
>> lease_get_mtime() and whether it should or should not be invoked
>> by fill_post_wcc() too.  I chose not to change this because I
>> thought that it was safer to leave well enough alone.  If we
>> decide to make a change, it can be done separately.
>>     
>
> OK.
>
> Only superficial complaints:
> 	- There were some minor whitespace oddities; running the patch
> 	  through scripts/checkpatch.pl may be the quickest way to catch
> 	  those.
>   

Ugg.  How did those get in there?  :-)

Thanx for pointing this out.  I will ensure that I run checkpatch
before submitting future patches.

> 	- This would be better as two, maybe three separate patches;
> 	  e.g.  moving the lease_get_mtime out of encode_fattr3 could be
> 	  done separately first.  Ideally we'd do some trivial
> 	  transformations like that, followed by one change that
> 	  actually changes the inode behavior.  That makes the whole
> 	  thing trival to review.
>
>   

My test system isn't at a place where I could factor the patch and
do testing.  For the time being, I'd prefer to stick with the single
patch, since it is tested.
> I fixed up the first and added the result to
>
> 	git://linux-nfs.org/~bfields/linux.git for-mm
>
> so it should show up in the next -mm.  I'd happily replace it by a more
> finely split up version if that was something you could whip up in a few
> minutes.

Thanx, Bruce!

       ps

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 64 bit ino support for NFS server

[J. Bruce Fields]

On Fri, Aug 17, 2007 at 02:30:31PM -0400, Peter Staubach wrote:
> J. Bruce Fields wrote:
>> 	- This would be better as two, maybe three separate patches;
>> 	  e.g.  moving the lease_get_mtime out of encode_fattr3 could be
>> 	  done separately first.  Ideally we'd do some trivial
>> 	  transformations like that, followed by one change that
>> 	  actually changes the inode behavior.  That makes the whole
>> 	  thing trival to review.
>
> My test system isn't at a place where I could factor the patch and
> do testing.  For the time being, I'd prefer to stick with the single
> patch, since it is tested.

It shouldn't be hard to factor the patch into patches which produce the
identical end result, and are individually trivial enough to be unlikely
to break things partway through.

But the current patch isn't large enough that I'd put my foot down at
this point.

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs