[v3,04/25] NFS41: save layoutcommit cred after first successful layoutget

Commit Message

Jim Rees July 27, 2011, 6:40 p.m. UTC

From: Peng Tao <[e-mail]>

Save it in layout header instead of in every segment.

Signed-off-by: Peng Tao <[e-mail]>
---
 fs/nfs/pnfs.c |   12 ++++++------
 fs/nfs/pnfs.h |    1 -
 2 files changed, 6 insertions(+), 7 deletions(-)

Comments

Trond Myklebust July 27, 2011, 7:47 p.m. UTC | #1

On Wed, 2011-07-27 at 14:40 -0400, Jim Rees wrote: 
> From: Peng Tao <[e-mail]>
> 
> Save it in layout header instead of in every segment.
> 
> Signed-off-by: Peng Tao <[e-mail]>
> ---
>  fs/nfs/pnfs.c |   12 ++++++------
>  fs/nfs/pnfs.h |    1 -
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index dae19dd..fbebd2a 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -211,6 +211,7 @@ destroy_layout_hdr(struct pnfs_layout_hdr *lo)
>  	dprintk("%s: freeing layout cache %p\n", __func__, lo);
>  	BUG_ON(!list_empty(&lo->plh_layouts));
>  	NFS_I(lo->plh_inode)->layout = NULL;
> +	put_rpccred(lo->plh_lc_cred);
>  	pnfs_free_layout_hdr(lo);
>  }
>  
> @@ -1007,6 +1008,10 @@ pnfs_update_layout(struct inode *ino,
>  		list_del_init(&lo->plh_layouts);
>  		spin_unlock(&clp->cl_lock);
>  	}
> +	if (first) {
> +		lo->plh_lc_cred =
> +			get_rpccred(ctx->state->owner->so_cred);
> +	}

Hmm... This whole 'first' thing is inherently racy. What if two threads
are calling pnfs_update_layout at the same time: they both see
list_empty(&lo->plh_segs), and both conclude that they need to
initialise the layout header. _BOOM_....

The correct thing to do here is either to initialise the layout header
correctly inside pnfs_find_alloc_layout(), or to add a flag for
->plh_flags that can be used with test_and_clear_bit() to get _one_ of
the processes to perform the initialisation.
Personally, I'd prefer the former.

Patch

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index dae19dd..fbebd2a 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -211,6 +211,7 @@  destroy_layout_hdr(struct pnfs_layout_hdr *lo)
 	dprintk("%s: freeing layout cache %p\n", __func__, lo);
 	BUG_ON(!list_empty(&lo->plh_layouts));
 	NFS_I(lo->plh_inode)->layout = NULL;
+	put_rpccred(lo->plh_lc_cred);
 	pnfs_free_layout_hdr(lo);
 }
 
@@ -1007,6 +1008,10 @@  pnfs_update_layout(struct inode *ino,
 		list_del_init(&lo->plh_layouts);
 		spin_unlock(&clp->cl_lock);
 	}
+	if (first) {
+		lo->plh_lc_cred =
+			get_rpccred(ctx->state->owner->so_cred);
+	}
 	atomic_dec(&lo->plh_outstanding);
 	put_layout_hdr(lo);
 out:
@@ -1386,8 +1391,6 @@  pnfs_set_layoutcommit(struct nfs_write_data *wdata)
 	spin_lock(&nfsi->vfs_inode.i_lock);
 	if (!test_and_set_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags)) {
 		mark_as_dirty = true;
-		nfsi->layout->plh_lc_cred =
-			get_rpccred(wdata->args.context->state->owner->so_cred);
 		dprintk("%s: Set layoutcommit for inode %lu ",
 			__func__, wdata->inode->i_ino);
 	}
@@ -1421,7 +1424,6 @@  pnfs_layoutcommit_inode(struct inode *inode, bool sync)
 {
 	struct nfs4_layoutcommit_data *data;
 	struct nfs_inode *nfsi = NFS_I(inode);
-	struct rpc_cred *cred;
 	loff_t end_pos;
 	int status = 0;
 
@@ -1449,16 +1451,14 @@  pnfs_layoutcommit_inode(struct inode *inode, bool sync)
 	pnfs_list_write_lseg(inode, &data->lseg_list);
 
 	end_pos = nfsi->layout->plh_lwb;
-	cred = nfsi->layout->plh_lc_cred;
 	nfsi->layout->plh_lwb = 0;
-	nfsi->layout->plh_lc_cred = NULL;
 
 	memcpy(&data->args.stateid.data, nfsi->layout->plh_stateid.data,
 		sizeof(nfsi->layout->plh_stateid.data));
 	spin_unlock(&inode->i_lock);
 
 	data->args.inode = inode;
-	data->cred = cred;
+	data->cred = get_rpccred(nfsi->layout->plh_lc_cred);
 	nfs_fattr_init(&data->fattr);
 	data->args.bitmask = NFS_SERVER(inode)->cache_consistency_bitmask;
 	data->res.fattr = &data->fattr;
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 5f1b532..bddd8b9 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -46,7 +46,6 @@  struct pnfs_layout_segment {
 	atomic_t pls_refcount;
 	unsigned long pls_flags;
 	struct pnfs_layout_hdr *pls_layout;
-	struct rpc_cred	*pls_lc_cred; /* LAYOUTCOMMIT credential */
 };
 
 enum pnfs_try_status {