[PATCH 2/2] SQUASHME: pnfs-submit: clean up nfs_lock_alloc_layout

[Benny Halevy <bhalevy@xxxxxxxxxxx>]

Fix a bug where the function returned without taking the i_lock
if the layout hdr was already allocated.
Simplify by moving inode locking to caller.

Rename function as it no longer grabs the lock.
Clean up the implementation so it's clearer what's going on
and what are the likely cases vs. the unlikely ones.

Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
---
 fs/nfs/pnfs.c |   42 ++++++++++++++++++++++--------------------
 1 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 4ba7595..053a5c1 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -938,36 +938,37 @@ alloc_init_layout(struct inode *ino)
 }
 
 /*
- * Lock and possibly allocate the inode layout
+ * Retrieve and possibly allocate the inode layout
  *
- * If successful, ino->i_lock is taken, and the caller must unlock.
+ * ino->i_lock must be taken by the caller.
  */
 static struct pnfs_layout_type *
-nfs_lock_alloc_layout(struct inode *ino)
+pnfs_alloc_layout(struct inode *ino)
 {
+	struct nfs_inode *nfsi = NFS_I(ino);
 	struct pnfs_layout_type *new = NULL;
 
-	dprintk("%s Begin ino=%p layout=%p\n", __func__, ino, NFS_I(ino)->layout);
+	dprintk("%s Begin ino=%p layout=%p\n", __func__, ino, nfsi->layout);
 
-	if (NFS_I(ino)->layout == NULL) {
-		new = alloc_init_layout(ino);
-		if (new == NULL)
-			return NULL;
-		spin_lock(&ino->i_lock);
-		if (NFS_I(ino)->layout == NULL) {
-			NFS_I(ino)->layout = new;
-			new = NULL;
-		}
-	}
-	if (new) {
+	BUG_ON(!spin_is_locked(&ino->i_lock));
+	if (likely(nfsi->layout))
+		return nfsi->layout;
+
+	spin_unlock(&ino->i_lock);
+	new = alloc_init_layout(ino);
+	spin_lock(&ino->i_lock);
+
+	if (likely(nfsi->layout == NULL)) {	/* Won the race? */
+		nfsi->layout = new;
+	} else if (new) {
 		/* Reference the layout accross i_lock release and grab */
-		get_layout(NFS_I(ino)->layout);
+		get_layout(nfsi->layout);
 		spin_unlock(&ino->i_lock);
 		NFS_SERVER(ino)->pnfs_curr_ld->ld_io_ops->free_layout(new);
 		spin_lock(&ino->i_lock);
-		put_layout_locked(NFS_I(ino)->layout);
+		put_layout_locked(nfsi->layout);
 	}
-	return NFS_I(ino)->layout;
+	return nfsi->layout;
 }
 
 /*
@@ -1055,10 +1056,11 @@ _pnfs_update_layout(struct inode *ino,
 
 	if (take_ref)
 		*lsegpp = NULL;
-	lo = nfs_lock_alloc_layout(ino);
+	spin_lock(&ino->i_lock);
+	lo = pnfs_alloc_layout(ino);
 	if (lo == NULL) {
 		dprintk("%s ERROR: can't get pnfs_layout_type\n", __func__);
-		goto out;
+		goto out_unlock;
 	}
 
 	/* Check to see if the layout for the given range already exists */
-- 
1.7.1.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html