[pnfs] CVS: nfsv4

[Iyer, Rahul]

Hi,
I've replied inline
Thanks
Rahul 

-----Original Message-----
From: Dean Hildebrand [mailto:dhildebz at eecs.umich.edu] 
Sent: Tuesday, July 18, 2006 12:10 PM
To: Iyer, Rahul
Cc: pnfs at linux-nfs.org
Subject: Re: [pnfs] CVS: nfsv4

Couple comments below...

Iyer, Rahul wrote:
> Hey!
> Here it is...
> Thanks
> Rahul
>
> <----patch start---->
>
> Index: fs/nfs/nfs4filelayout.c
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfs/nfs4filelayout.c,
<snip>
> 262a326,328
>   
>>                 /* In the case of synchronous writes, we reset the
>>     
> offset here */
>   
>>                 data->args.offset = data->orig_offset;
>>     
Why set and then reset? Just set the offset where it needs to be (inside
the else portion)

I set it because it is needed in both the if and the else parts. The
if() part is for synchronous reads/writes and the else for their
asynchronous equivalents. In the case of the synchronous ones, I can
reset the offset immediately after the call is done, but for the
asynchronous calls I have to rreset them in their call_done() routines.



> 265a332,334
>   
>>                 /* In the case of aync writes, the offset will be
>>     
> reset in the
>   
>>                  * call_ops->rpc_call_done() routine
>>                  */
>>     
> Index: fs/nfs/read.c
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfs/read.c,v
> retrieving revision 1.13
> diff -r1.13 read.c
> 526a527,534
>   
>> #ifdef CONFIG_NFS_V4
>>       /* Restore the data->args->offset pointer to the actual offset
>>        * if it was changed
>>        */
>>       if (data->orig_offset)
>>               data->args.offset = data->orig_offset; #endif
>>     
For here and below, is "#ifdef CONFIG_NFS_V4" what you really mean?  All
this means is that nfsv4 was configured, but your code will be executed
for nfsv3, etc.  This seems like another case where the proc functions
might be useful.


The reason it is in CONFIG_NFS_V4 is because the orig_offset field is
declared within CONFIG_NFS_V4 ifdefs too. I agree it will be executed
even by v3 etc. (only if v4 is also configured, that is), but doesn't
have any adverse effect as data->offset is never changed in those
codepaths. 

I'm sorry, I didn't get the part about the proc functions. Which
functions are these? 






Dean
>>     
> 569a578,585
>   
>> #ifdef CONFIG_NFS_V4
>>         /* Restore the data->args->offset pointer to the actual
offset
>>          * if it was changed
>>          */
>>         if (data->orig_offset)
>>                 data->args.offset = data->orig_offset; #endif
>>     
> Index: fs/nfs/write.c
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfs/write.c,v
> retrieving revision 1.24
> diff -r1.24 write.c
> 1222a1223,1230
>   
>> #ifdef CONFIG_NFS_V4
>>         /* Restore the data->args->offset pointer to the actual
offset
>>          * if it was changed
>>          */
>>         if (data->orig_offset)
>>                 data->args.offset = data->orig_offset; #endif
>>
>>     
> 1313a1322,1329
>   
>> #ifdef CONFIG_NFS_V4
>>         /* Restore the data->args->offset pointer to the actual
offset
>>          * if it was changed
>>          */
>>         if (data->orig_offset)
>>                 data->args.offset = data->orig_offset; #endif
>>     
> Index: include/linux/nfs_xdr.h
> ===================================================================
> RCS file: /cvs/nfsv4/cvs/pnfs/include/linux/nfs_xdr.h,v
> retrieving revision 1.13
> diff -r1.13 nfs_xdr.h
> 702a703
>   
>>       __u64                   orig_offset;
>>     
> 729a731
>   
>>       __u64                   orig_offset;
>>     
>
> <----end patch---->
>
> -----Original Message-----
> From: Dean Hildebrand [mailto:dhildebz at eecs.umich.edu]
> Sent: Wednesday, July 12, 2006 1:24 PM
> To: Rahul Iyer (NetApp)
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] CVS: nfsv4
>
> Hi Rahul,
>
> Can you send out your patch for this as well?
>
> Thanks,
> Dean
>
> Rahul Iyer (NetApp) wrote:
>   
>> CVSROOT:	/cvs
>> Module name:	nfsv4
>> Changes by:	iyer at citi.	2006/07/11 21:47:01
>>
>> Modified files:
>> 	cvs/pnfs/fs/nfs: nfs4filelayout.c nfs4filelayout.h read.c 
>> 	                 write.c 
>> 	cvs/pnfs/include/linux: nfs_xdr.h
>>
>> Log message:
>> Added code to support dense file layouts for the pnfs file layout 
>> driver
>>
>> _______________________________________________
>> pNFS mailing list
>> pNFS at linux-nfs.org
>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>>   
>>     
>
> --
> Dean Hildebrand
> Ph.D. Candidate
> University of Michigan
>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>   

--
Dean Hildebrand
Ph.D. Candidate
University of Michigan

_______________________________________________
pNFS mailing list
pNFS at linux-nfs.org
http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs