[pnfs] CVS: nfsv4

[Iyer, Rahul]

Hi Dean,
I agree with this. This isn't exactly the most elegant way to solve this
problem. I'll take another look at this...
Regards
Rahul


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

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?
>
>   
Well, we have resisted putting layout specific code in the general code
path, and now we have stripe specific code for a particular layout in
the general code path.  Any way around this?

If we must, then I wonder if there is a way we can group all the 'dense'

striping changes in a common module instead of 'sprinkling' them
throughout the code.

For example, maybe a set of functions that handle offset management for
dense striping (or all striping methods for that matter).

Maybe instead of

#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

we just put
pnfs_reset_offset(&data->args.offset, data->orig_offset)

(which contains the above code)

Yet another way would be to create a general new proc function for
pnfs_v4_clientops that handled setting the offset (and would be called
when setting up the write/read request).  This would involve a lot of
changes to the code though and I don't know if it would really work.
Dean
>
>
>
>
> 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
>   

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