[pnfs] [PATCH 21/38] [v1] pnfs: client layout cache: {get_lock, put_unlock}_current_layout

Dean Hildebrand seattleplus at gmail.com
Mon Jan 7 18:57:25 EST 2008



Benny Halevy wrote:
>  
>  /*
> - * get, possibly allocate current_layout
> + * get, possibly allocate, and lock current_layout
>   
What I wouldn't mind here is a comment saying something to the effect of:
"Grab's lock on layout.  If successful, caller must unlock current_layout
by  ___fill in how layout lock is released____"

I think this will help future developers figure out the start and end 
points for the layout lock.
Thanks for re-sending the patches btw.
Dean
>   */
>  static struct pnfs_layout_type *
>  get_alloc_layout(struct inode *ino,
> @@ -467,7 +509,7 @@ get_alloc_layout(struct inode *ino,
>  
>  	dprintk("%s Begin\n", __FUNCTION__);
>  
> -	while ((lo = nfsi->current_layout) == NULL) {
> +	while ((lo = get_lock_current_layout(nfsi)) == NULL) {
>  		/* Compete against other threads on who's doing the allocation,
>  		 * wait until bit is cleared if we lost this race.
>  		 */
> @@ -480,22 +522,22 @@ get_alloc_layout(struct inode *ino,
>  		}
>  
>  		/* Was current_layout already allocated while we slept?
> -		 * If not, allocate it.
> +		 * If so, retry get_lock'ing it. Otherwise, allocate it.
>  		 */
> -		lo = nfsi->current_layout;
> -		if (!lo)
> -			lo = nfsi->current_layout =
> -				alloc_init_layout(ino, io_ops);
> +		if (nfsi->current_layout)
> +			continue;
> +
> +		lo = alloc_init_layout(ino, io_ops);
> +		if (lo) {
> +			/* must grab the layout lock */
> +			spin_lock(&nfsi->lo_lock);
>   
> +			nfsi->current_layout = lo;
> +		} else
> +			lo = ERR_PTR(-ENOMEM);
>  
>  		/* release the NFS_INO_LAYOUT_ALLOC bit and wake up waiters */
>  		clear_bit_unlock(NFS_INO_LAYOUT_ALLOC, &nfsi->pnfs_layout_state);
>  		wake_up_bit(&nfsi->pnfs_layout_state, NFS_INO_LAYOUT_ALLOC);
> -
> -		/* we're done here.
> -		 * just check whether alloc_init_layout succeeded.
> -		 */
> -		if (!lo)
> -			lo = ERR_PTR(-ENOMEM);
>  		break;
>  	}
>  
> @@ -505,7 +547,6 @@ get_alloc_layout(struct inode *ino,
>  	else
>  		dprintk("%s Return error %ld\n", __FUNCTION__, PTR_ERR(lo));
>  #endif
> -	return lo;
>  }
>  
>  /* Update the file's layout for the given range and iomode.
> @@ -631,6 +672,7 @@ out:
>  
>  	/* res.layout.buf kalloc'ed by the xdr decoder? */
>  	kfree(res.layout.buf);
> +	put_unlock_current_layout(nfsi, layout_new);
>  ret:
>  	dprintk("%s end (err:%d) state 0x%lx\n",
>  		__FUNCTION__, result, nfsi->pnfs_layout_state);
> @@ -721,9 +763,11 @@ pnfs_getboundary(struct inode *inode)
>  		goto out;
>  
>  	nfsi = NFS_I(inode);
> -	lo = nfsi->current_layout;
> -	if (lo)
> +	lo = get_lock_current_layout(nfsi);;
> +	if (lo) {
>  		stripe_size = policy_ops->get_stripesize(lo);
> +		put_unlock_current_layout(nfsi, lo);
> +	}
>  out:
>  	return stripe_size;
>  }
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 1a093b1..99dfe9c 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -181,6 +181,7 @@ struct nfs_inode {
>  #define NFS_INO_LAYOUT_ALLOC	0x0002	/* get layout failed, stop trying */
>  	time_t pnfs_layout_suspend;
>  	wait_queue_head_t lo_waitq;
> +	spinlock_t lo_lock;
>  	struct pnfs_layout_type *current_layout;
>  	/* use rpc_creds in this open_context to send LAYOUTCOMMIT to MDS */
>  	struct nfs_open_context *layoutcommit_ctx;
>   


More information about the pNFS mailing list