[PATCH 0/4] pnfs-submit: cleanup layoutcommit verison 2

[PATCH 0/4] pnfs-submit: cleanup layoutcommit verison 2

[andros]

Responded to comments.

This is against the pnfs-submit branch of the 2.6.34 tree. They will need to be
applied against the 2.6.35-rc1 tree which I can do after comments.

A cleanup, and call the async error handler.
0001-SQUASHME-pnfs-submit-cleanup-layoutcommit-call.patch
0002-SQUASHME-pnfs-submit-handle-async-layoutcommit-erro.patch

These next two patches moves the pnfs_layoutcommit_inode call to
nfs_write_inode, and it is the only call other than in layoutreturn.
(removed calls in __nfs4_close, nfs_commit_inode, nfs_wb_sync).

0003-SQUASHME-pnfs-remove-ifdef-around-layoutcommit_neede.patch
0004-SQUASHME-pnfs-submit-move-layoutcommit-to-nfs_write.patch

pnfs_layoutcommit_inode is called after nfs_commit_unstable_pages() so that
if LAYOUTCOMMIT fails, the unstable pages have been processed..

The error handlers (sync and async) call nfs4_map_errors, so unhandled
errors (such as NFS4ERR_BADLAYOUT) get returned to nfs_write_ioode as -EIO.

We will add code to the error handlers for errors such as NFS4ERR_BADLAYOUT
that require us to stop using and free the layout, and redo the I/O through
the MDS.

Testing:
With CONFIG_NFS_V4_1 set
NFSv4.1/pnfs passed Connectathon against write enabled GFS2/pNFS. Note: there
were exactly the same number of LAYOUTCOMMITS sent as were sent with
pnfs_layoutcommit_inode being called from __nfs4_close (never happened),
nfs_commit_inode and nfs_wb_sync.

V4.0 mount passes Connectathon tests

With CONFIG_NFS_V4_1 not set

V4.0 mount passes Connectathon tests

-->Andy

[PATCH 1/4] SQUASHME pnfs-submit: cleanup layoutcommit call

[andros]

From: Andy Adamson <andros@netapp.com>

pnfs_layoutcommit_inode prints its status upon exit.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/pnfs.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 8cc4412..3e940ec 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -766,15 +766,9 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
 				!pnfs_return_layout_barrier(nfsi, &arg));
 		}
 
-		if (layoutcommit_needed(nfsi)) {
-			status = pnfs_layoutcommit_inode(ino, wait);
-			if (status) {
-				dprintk("%s: layoutcommit failed, status=%d. "
-					"Returning layout anyway\n",
-					__func__, status);
-				status = 0;
-			}
-		}
+		if (layoutcommit_needed(nfsi))
+			/* Return layout even if layoutcommit fails */
+			pnfs_layoutcommit_inode(ino, wait);
 	}
 send_return:
 	status = return_layout(ino, &arg, stateid, type, lo, wait);
-- 
1.6.2.5

[PATCH 2/4] SQUASHME pnfs-submit: handle async layoutcommit errors

[andros]

From: Andy Adamson <andros@netapp.com>

nfs4_async_handle_error handles session level and grace,delay errors.

NOTE: Layout specific errors (NFS4ERR_BADIOMODE, NFS4ERR_BAD_LAYOUT,
NFS4ERR_UNKNOWN_LAYOUTTYPE) need to be handled.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4proc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 8b375a7..19ac602 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5628,6 +5628,9 @@ pnfs_layoutcommit_rpc_done(struct rpc_task *task, void *calldata)
 	pnfs_layoutcommit_done(data);
 
 	nfs4_sequence_done(server, &data->res.seq_res, task->tk_status);
+
+	if (nfs4_async_handle_error(task, server, NULL) == -EAGAIN)
+		nfs_restart_rpc(task, server->nfs_client);
 }
 
 static void pnfs_layoutcommit_release(void *lcdata)
-- 
1.6.2.5

[PATCH 3/4] SQUASHME pnfs remove ifdef around layoutcommit_needed

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/inode.c            |    2 --
 include/linux/nfs4_pnfs.h |    8 ++++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index c93b756..086d46f 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1106,7 +1106,6 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 			!test_bit(NFS_INO_MOUNTPOINT, &nfsi->flags))
 		server->fsid = fattr->fsid;
 
-#ifdef CONFIG_NFS_V4_1
 	/*
 	 * file needs layout commit, server attributes may be stale
 	 */
@@ -1115,7 +1114,6 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 			__func__, inode->i_sb->s_id, inode->i_ino);
 		return 0;
 	}
-#endif /* CONFIG_NFS_V4_1 */
 	/*
 	 * Update the read time so we don't revalidate too often.
 	 */
diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
index 84d2e95..c248ae9 100644
--- a/include/linux/nfs4_pnfs.h
+++ b/include/linux/nfs4_pnfs.h
@@ -86,6 +86,14 @@ layoutcommit_needed(struct nfs_inode *nfsi)
 	return nfsi->lo_cred != NULL;
 }
 
+#else /* CONFIG_NFS_V4_1 */
+
+static inline bool
+layoutcommit_needed(struct nfs_inode *nfsi)
+{
+	return 0;
+}
+
 #endif /* CONFIG_NFS_V4_1 */
 
 struct pnfs_layout_segment {
-- 
1.6.2.5

[PATCH 4/4] SQUASHME pnfs-submit: move layoutcommit to nfs_write_inode

[andros]

From: Andy Adamson <andros@netapp.com>

The LAYOUTCOMMIT call indicates an update to the file meta data is needed,
and should be called when clearing the I_DIRTY_SYNC state.

A call to LAYOUTCOMMIT in nfs_write_inode replaces the calls in nfs_wb_all,
nfs_commit_inode, and __nfs4_close.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4state.c |    2 --
 fs/nfs/pnfs.h      |    4 ++++
 fs/nfs/write.c     |   24 ++++++++++++------------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index d0dbdd4..e1207fa 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -589,8 +589,6 @@ static void __nfs4_close(struct path *path, struct nfs4_state *state, fmode_t fm
 #ifdef CONFIG_NFS_V4_1
 		struct nfs_inode *nfsi = NFS_I(state->inode);
 
-		if (layoutcommit_needed(nfsi))
-			pnfs_layoutcommit_inode(state->inode, wait);
 		if (has_layout(nfsi) && nfsi->layout.roc_iomode) {
 			struct nfs4_pnfs_layout_segment range;
 
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index c89be78..009393e 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -256,6 +256,10 @@ static inline int pnfs_use_rpc(struct nfs_server *nfss)
 	return 1;
 }
 
+static inline int pnfs_layoutcommit_inode(struct inode *inode, int sync)
+{
+	return 0;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 #endif /* FS_NFS_PNFS_H */
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 0fd33cb..513b308 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1489,13 +1489,6 @@ static int nfs_commit_inode(struct inode *inode, int how)
 			wait_on_bit(&NFS_I(inode)->flags, NFS_INO_COMMIT,
 					nfs_wait_bit_killable,
 					TASK_KILLABLE);
-#ifdef CONFIG_NFS_V4_1
-		if (may_wait && layoutcommit_needed(NFS_I(inode))) {
-			error = pnfs_layoutcommit_inode(inode, 1);
-			if (error < 0)
-				return error;
-		}
-#endif /* CONFIG_NFS_V4_1 */
 	} else
 		nfs_commit_clear_lock(NFS_I(inode));
 out:
@@ -1545,7 +1538,18 @@ static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_contr
 
 int nfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 {
-	return nfs_commit_unstable_pages(inode, wbc);
+	int ret;
+	ret = nfs_commit_unstable_pages(inode, wbc);
+	if (ret >= 0 && layoutcommit_needed(NFS_I(inode))) {
+		int err, sync = wbc->sync_mode;
+
+		if (wbc->nonblocking || wbc->for_background)
+			sync = 0;
+		err = pnfs_layoutcommit_inode(inode, sync);
+		if (err < 0)
+			ret = err;
+	}
+	return ret;
 }
 
 /*
@@ -1562,10 +1566,6 @@ int nfs_wb_all(struct inode *inode)
 	};
 
 	ret = sync_inode(inode, &wbc);
-#ifdef CONFIG_NFS_V4_1
-	if (!ret && layoutcommit_needed(NFS_I(inode)))
-		ret = pnfs_layoutcommit_inode(inode, 1);
-#endif
 	return ret;
 }
 
-- 
1.6.2.5

Re: [PATCH 0/4] pnfs-submit: cleanup layoutcommit verison 2

[Boaz Harrosh]

On 06/03/2010 07:25 PM, andros@netapp.com wrote:
> Responded to comments.
> 
> This is against the pnfs-submit branch of the 2.6.34 tree. They will need to be
> applied against the 2.6.35-rc1 tree which I can do after comments.
> 
> A cleanup, and call the async error handler.
> 0001-SQUASHME-pnfs-submit-cleanup-layoutcommit-call.patch
> 0002-SQUASHME-pnfs-submit-handle-async-layoutcommit-erro.patch
> 
> These next two patches moves the pnfs_layoutcommit_inode call to
> nfs_write_inode, and it is the only call other than in layoutreturn.
> (removed calls in __nfs4_close, nfs_commit_inode, nfs_wb_sync).
> 
> 0003-SQUASHME-pnfs-remove-ifdef-around-layoutcommit_neede.patch
> 0004-SQUASHME-pnfs-submit-move-layoutcommit-to-nfs_write.patch
> 
> pnfs_layoutcommit_inode is called after nfs_commit_unstable_pages() so that
> if LAYOUTCOMMIT fails, the unstable pages have been processed..
> 
> The error handlers (sync and async) call nfs4_map_errors, so unhandled
> errors (such as NFS4ERR_BADLAYOUT) get returned to nfs_write_ioode as -EIO.
> 
> We will add code to the error handlers for errors such as NFS4ERR_BADLAYOUT
> that require us to stop using and free the layout, and redo the I/O through
> the MDS.
> 
> Testing:
> With CONFIG_NFS_V4_1 set
> NFSv4.1/pnfs passed Connectathon against write enabled GFS2/pNFS. Note: there
> were exactly the same number of LAYOUTCOMMITS sent as were sent with
> pnfs_layoutcommit_inode being called from __nfs4_close (never happened),
> nfs_commit_inode and nfs_wb_sync.
> 
> V4.0 mount passes Connectathon tests
> 
> With CONFIG_NFS_V4_1 not set
> 
> V4.0 mount passes Connectathon tests
> 
> -->Andy
> 

Hi Handy, I'll review this and Test on Sunday. Happy weekend.

Thanks
Boaz

Re: [PATCH 4/4] SQUASHME pnfs-submit: move layoutcommit to nfs_write_inode

[Boaz Harrosh]

On 06/03/2010 07:25 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> The LAYOUTCOMMIT call indicates an update to the file meta data is needed,
> and should be called when clearing the I_DIRTY_SYNC state.
> 
> A call to LAYOUTCOMMIT in nfs_write_inode replaces the calls in nfs_wb_all,
> nfs_commit_inode, and __nfs4_close.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>

This patch no longer applies on 2.6.35-rc2 in write.c at nfs_commit_inode.
But it was easy enough to fix. (Benny tell me if you want a tree with these
in)

So I did a rough test "git clone linux" and cthon. It looks very good
(few comments below)

> ---
>  fs/nfs/nfs4state.c |    2 --
>  fs/nfs/pnfs.h      |    4 ++++
>  fs/nfs/write.c     |   24 ++++++++++++------------
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index d0dbdd4..e1207fa 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -589,8 +589,6 @@ static void __nfs4_close(struct path *path, struct nfs4_state *state, fmode_t fm
>  #ifdef CONFIG_NFS_V4_1
>  		struct nfs_inode *nfsi = NFS_I(state->inode);
>  
> -		if (layoutcommit_needed(nfsi))
> -			pnfs_layoutcommit_inode(state->inode, wait);
>  		if (has_layout(nfsi) && nfsi->layout.roc_iomode) {
>  			struct nfs4_pnfs_layout_segment range;
>  
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index c89be78..009393e 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -256,6 +256,10 @@ static inline int pnfs_use_rpc(struct nfs_server *nfss)
>  	return 1;
>  }
>  
> +static inline int pnfs_layoutcommit_inode(struct inode *inode, int sync)
> +{
> +	return 0;
> +}
>  #endif /* CONFIG_NFS_V4_1 */
>  

Just for correctness this patch belongs to [patch 3/4], for it to
compile independently.

If they are all squashed then that does not matter. Just as a note.
You might as well combine them.

>  #endif /* FS_NFS_PNFS_H */
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 0fd33cb..513b308 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -1489,13 +1489,6 @@ static int nfs_commit_inode(struct inode *inode, int how)
>  			wait_on_bit(&NFS_I(inode)->flags, NFS_INO_COMMIT,
>  					nfs_wait_bit_killable,
>  					TASK_KILLABLE);
> -#ifdef CONFIG_NFS_V4_1
> -		if (may_wait && layoutcommit_needed(NFS_I(inode))) {
> -			error = pnfs_layoutcommit_inode(inode, 1);
> -			if (error < 0)
> -				return error;
> -		}
> -#endif /* CONFIG_NFS_V4_1 */
>  	} else
>  		nfs_commit_clear_lock(NFS_I(inode));
>  out:
> @@ -1545,7 +1538,18 @@ static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_contr
>  
>  int nfs_write_inode(struct inode *inode, struct writeback_control *wbc)
>  {
> -	return nfs_commit_unstable_pages(inode, wbc);
> +	int ret;
> +	ret = nfs_commit_unstable_pages(inode, wbc);
> +	if (ret >= 0 && layoutcommit_needed(NFS_I(inode))) {
> +		int err, sync = wbc->sync_mode;
> +
> +		if (wbc->nonblocking || wbc->for_background)
> +			sync = 0;
> +		err = pnfs_layoutcommit_inode(inode, sync);
> +		if (err < 0)
> +			ret = err;
> +	}
> +	return ret;
>  }
>  
>  /*
> @@ -1562,10 +1566,6 @@ int nfs_wb_all(struct inode *inode)
>  	};
>  
>  	ret = sync_inode(inode, &wbc);
> -#ifdef CONFIG_NFS_V4_1
> -	if (!ret && layoutcommit_needed(NFS_I(inode)))
> -		ret = pnfs_layoutcommit_inode(inode, 1);
> -#endif
>  	return ret;
>  }
>  

Grate stuff.
Boaz