On Jul. 08, 2010, 1:34 +0300, andros@xxxxxxxxxx wrote:
> From: Andy Adamson <andros@xxxxxxxxxx>
>
> Don't wait on a bit if layout is not allocated. Instead, allocate,
> check for existance, and possibly free.
>
> As per new layout refcounting scheme,don't take a reference on the layout.
>
> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
> ---
> fs/nfs/pnfs.c | 84 +++++++++++++----------------------------------
> include/linux/nfs_fs.h | 1 -
> 2 files changed, 23 insertions(+), 62 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 6d435ed..92b7772 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -914,7 +914,6 @@ alloc_init_layout(struct inode *ino)
> struct pnfs_layout_type *lo;
> struct layoutdriver_io_operations *io_ops;
>
> - BUG_ON(NFS_I(ino)->layout != NULL);
> io_ops = NFS_SERVER(ino)->pnfs_curr_ld->ld_io_ops;
> lo = io_ops->alloc_layout(ino);
> if (!lo) {
> @@ -923,84 +922,47 @@ alloc_init_layout(struct inode *ino)
> __func__);
> return NULL;
> }
> - spin_lock(&ino->i_lock);
> lo->refcount = 1;
> INIT_LIST_HEAD(&lo->lo_layouts);
> INIT_LIST_HEAD(&lo->segs);
> seqlock_init(&lo->seqlock);
> lo->lo_inode = ino;
> - NFS_I(ino)->layout = lo;
> - spin_unlock(&ino->i_lock);
> return lo;
> }
>
> -static int pnfs_wait_schedule(void *word)
> -{
> - if (signal_pending(current))
> - return -ERESTARTSYS;
> - schedule();
> - return 0;
> -}
> -
> /*
> - * get, possibly allocate, and lock current_layout
> + * Lock and possibly allocate the inode layout
> *
> - * Note: If successful, ino->i_lock is taken and the caller
> - * must put and unlock current_layout when the returned layout is released.
> + * If successful, ino->i_lock is taken, and the caller must unlock.
> */
> static struct pnfs_layout_type *
> -get_lock_alloc_layout(struct inode *ino)
> +nfs_lock_alloc_layout(struct inode *ino)
> {
> - struct nfs_inode *nfsi = NFS_I(ino);
> - struct pnfs_layout_type *lo;
> - int res;
> + struct pnfs_layout_type *new = NULL;
>
> dprintk("%s Begin\n", __func__);
>
> spin_lock(&ino->i_lock);
> - while ((lo = grab_current_layout(nfsi)) == NULL) {
> + if (NFS_I(ino)->layout == NULL) {
> spin_unlock(&ino->i_lock);
There's no real need to take the lock here just for checking for NULL,
only down below before actually committing the newly allocated struct.
[I'll make this fix)
Benny
> - /* Compete against other threads on who's doing the allocation,
> - * wait until bit is cleared if we lost this race.
> - */
> - res = wait_on_bit_lock(
> - &nfsi->layout->pnfs_layout_state,
> - NFS_INO_LAYOUT_ALLOC,
> - pnfs_wait_schedule, TASK_KILLABLE);
> - if (res) {
> - lo = ERR_PTR(res);
> - break;
> - }
> -
> - /* Was current_layout already allocated while we slept?
> - * If so, retry get_lock'ing it. Otherwise, allocate it.
> - */
> - if (nfsi->layout) {
> - spin_lock(&ino->i_lock);
> - continue;
> + 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;
> }
> -
> - lo = alloc_init_layout(ino);
> - if (lo)
> - spin_lock(&ino->i_lock);
> - else
> - lo = ERR_PTR(-ENOMEM);
> -
> - /* release the NFS_INO_LAYOUT_ALLOC bit and wake up waiters */
> - clear_bit_unlock(NFS_INO_LAYOUT_ALLOC,
> - &nfsi->layout->pnfs_layout_state);
> - wake_up_bit(&nfsi->layout->pnfs_layout_state,
> - NFS_INO_LAYOUT_ALLOC);
> - break;
> }
> -
> -#ifdef NFS_DEBUG
> - if (!IS_ERR(lo))
> - dprintk("%s Return %p\n", __func__, lo);
> - else
> - dprintk("%s Return error %ld\n", __func__, PTR_ERR(lo));
> -#endif
> - return lo;
> + if (new) {
> + /* Reference the layout accross i_lock release and grab */
> + get_layout(NFS_I(ino)->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(NFS_I(ino)->layout);
> + }
> + return NFS_I(ino)->layout;
> }
>
> /*
> @@ -1088,8 +1050,8 @@ _pnfs_update_layout(struct inode *ino,
>
> if (take_ref)
> *lsegpp = NULL;
> - lo = get_lock_alloc_layout(ino);
> - if (IS_ERR(lo)) {
> + lo = nfs_lock_alloc_layout(ino);
> + if (lo == NULL) {
> dprintk("%s ERROR: can't get pnfs_layout_type\n", __func__);
> goto out;
> }
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 7b999a4..005b3ea 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -107,7 +107,6 @@ struct pnfs_layout_type {
> unsigned long pnfs_layout_state;
> #define NFS_INO_RO_LAYOUT_FAILED 0 /* get ro layout failed stop trying */
> #define NFS_INO_RW_LAYOUT_FAILED 1 /* get rw layout failed stop trying */
> - #define NFS_INO_LAYOUT_ALLOC 2 /* bit lock for layout allocation */
> #define NFS_INO_LAYOUTCOMMIT 3 /* LAYOUTCOMMIT needed */
> struct rpc_cred *lo_cred; /* layoutcommit credential */
> /* DH: These vars keep track of the maximum write range
--
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
