[pnfs] pNFS CVS 2.6.16 to 2.6.17 merge notes

[Iyer, Rahul]

Hi!
I've been looking into this... Yes, the corruption was from the dense
striping changes. This stemmed from the fact that a patch got applied
twice, without whining the second time around! I did not expect it to
fail noiselessly! 

Anyways, I've looked at this and have corrected it. I have just tested
the code against our setup. The patch to fix it is given below. 
My apologies for the mess up!
Regards
Rahul


<----Start patch---->

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index efa9af2..607b4f9 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -519,6 +519,14 @@ static void nfs_readpage_result_partial(
 	struct nfs_page *req = data->req;
 	struct page *page = req->wb_page;
 
+	#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
+		
 	if (likely(task->tk_status >= 0))
 		nfs_readpage_truncate_uninitialised_page(data);
 	else
@@ -579,6 +587,14 @@ static void nfs_readpage_result_full(str
 {
 	struct nfs_read_data *data = calldata;
 
+	#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
+		
 	/*
 	 * Note: nfs_readpage_result may change the values of
 	 * data->args. In the multi-page case, we therefore need
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 1068d42..e141a63 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1251,15 +1251,7 @@ static void nfs_writeback_done_partial(s
         if (data->orig_offset)
                 data->args.offset = data->orig_offset;
 #endif
-
-#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
-
+	
 	dprintk("NFS: write (%s/%Ld %d@%Ld)",
 		req->wb_context->dentry->d_inode->i_sb->s_id,
 		(long long)NFS_FILEID(req->wb_context->dentry->d_inode),

<---end patch--->


-----Original Message-----
From: Dean Hildebrand [mailto:dhildebz at eecs.umich.edu] 
Sent: Monday, July 17, 2006 2:36 PM
To: pnfs at linux-nfs.org
Subject: Re: [pnfs] pNFS CVS 2.6.16 to 2.6.17 merge notes

Does anyone know if all of the corruption problems Andy noticed in the
2.6.16 kernel are now fixed in 2.6.17?  Were they all related to the
striping changes?

Dean

William A.(Andy) Adamson wrote:
> as you've seen, i've updated the pNFS CVS tree to the 2.6.17 kernel 
> with the
> 2.6.17- CITI_NFS4_ALL-1.diff patch applied. the merge is complete.
>
> i've only tested normal NFSv4.0 with the connectathon tests - so the 
> pNFS code path is untested. so please, test the code!
>
> here are the merge notes:
>
>  fs/nfs/read.c:
>
> pnfs_readpage_result_norpc() is the callback from a non RPC pNFS 
> layout driver that used the pagecache. consequently, it needs all of 
> the page management functionality, and non of the RPC stuff in 
> nfs_result_partial() and nfs_result_full.
>
> in the 2.6.16 code, nfs_result_partial() and nfs_result_full() both 
> called
> nfs_result() as the first call. so, i separated out the page 
> management pieces of nfs_result_partial() to 
> nfs_result_partial_pages() and of
> nfs_result_full()  to nfs_result_full_pages() to be able to call them 
> in pnfs_readpage_result_norpc().
>
> the 2.6.17 code has changed nfs_result_partial/full(). so no more
> nfs_result_partial_pages() or nfs_result_full_pages. now
> pnfs_readpage_result_norpc() calls the page management code copied 
> from these functions.
>
> NOTE: the 2.6.16 cvs tree was corrupted! fs/nfs/read.c has this code 4

> times. twice  in nfs_readpage_result_partial() once outside of any 
> function, and once in nfs_readpage_result_full()
>
> #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
>
>
> i don't know where this goes - so it has been removed.
>
>
> NOTE: corruption in fs/nfs/nfs4filelayout.c - i removed the obvious.
>
> filelayout_get_dserver_offset() is declared twice (full function
occurs
> twice) plus the following:
>
>                 /* Now get the file offset on the dserver
>                  * Set the read offset to this offset, and
>                  * save the original offset in orig_offset
>                  */
>                 data->args.offset =
filelayout_get_dserver_offset(offset, 
> nfslay);
>                 data->orig_offset = offset;
> /* Perform sync or async reads.*
>  * An optimization for the NFS file layout driver
>  * allows the original read/write data structs to be passed in the
>  * last argument.
>  *
>
>  * This is called after the pNFS client has already created, so I pass
it
>  * in via the last argument (void*).  I think this is the only way as
there
>  * are just too many NFS specific arguments in the read/write data
structs
>  * to pass to the layout drivers.
>
>                 /* In the case of synchronous reads, we reset the
offset here 
> */                data->args.offset = data->orig_offset;
>  *
>  * TODO:
>  * 1. This is a lot of arguments, create special non-nfs-specific
structure?
>                 /* In the case of aync reads, the offset will be reset
in the
>                  * call_ops->rpc_call_done() routine
>                  */
>  */
> ssize_t filelayout_read_pagelist(
>
>
>
> _______________________________________________
> 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