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>