This is needed by next patch that changes refcounting
Signed-off-by: Fred Isaman <[email protected]>
---
fs/nfs/pnfs.c | 59 +++++++++++++++++++++++++++++----------------------------
1 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 025675b..ed4c72e 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -313,30 +313,20 @@ pnfs_unregister_layoutdriver(struct pnfs_layoutdriver_type *ld_type)
#define BUG_ON_UNLOCKED_LO(lo) do {} while (0)
#endif /* CONFIG_SMP */
-/*
- * get and lock nfsi->layout
- */
static inline struct pnfs_layout_type *
-get_lock_current_layout(struct nfs_inode *nfsi)
+get_current_layout(struct nfs_inode *nfsi)
{
- struct pnfs_layout_type *lo;
+ struct pnfs_layout_type *lo = &nfsi->layout;
- lo = &nfsi->layout;
- spin_lock(&nfsi->vfs_inode.i_lock);
- if (!lo->ld_data) {
- spin_unlock(&nfsi->vfs_inode.i_lock);
+ BUG_ON_UNLOCKED_LO(lo);
+ if (!lo->ld_data)
return NULL;
- }
-
lo->refcount++;
return lo;
}
-/*
- * put and unlock nfs->layout
- */
static inline void
-put_unlock_current_layout(struct pnfs_layout_type *lo)
+put_current_layout(struct pnfs_layout_type *lo)
{
struct inode *inode = PNFS_INODE(lo);
struct nfs_client *clp;
@@ -358,7 +348,6 @@ put_unlock_current_layout(struct pnfs_layout_type *lo)
list_del_init(&lo->lo_layouts);
spin_unlock(&clp->cl_lock);
}
- spin_unlock(&inode->i_lock);
}
void
@@ -370,7 +359,8 @@ pnfs_layout_release(struct pnfs_layout_type *lo,
spin_lock(&nfsi->vfs_inode.i_lock);
if (range)
pnfs_free_layout(lo, range);
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
+ spin_unlock(&nfsi->vfs_inode.i_lock);
wake_up_all(&nfsi->lo_waitq);
}
@@ -384,11 +374,13 @@ pnfs_destroy_layout(struct nfs_inode *nfsi)
.length = NFS4_MAX_UINT64,
};
- lo = get_lock_current_layout(nfsi);
+ spin_lock(&nfsi->vfs_inode.i_lock);
+ lo = get_current_layout(nfsi);
if (lo) {
pnfs_free_layout(lo, &range);
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
}
+ spin_unlock(&nfsi->vfs_inode.i_lock);
}
static inline void
@@ -751,12 +743,14 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
arg.length = NFS4_MAX_UINT64;
if (type == RETURN_FILE) {
- lo = get_lock_current_layout(nfsi);
+ spin_lock(&ino->i_lock);
+ lo = get_current_layout(nfsi);
if (lo && !has_layout_to_return(lo, &arg)) {
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
lo = NULL;
}
if (!lo) {
+ spin_unlock(&ino->i_lock);
dprintk("%s: no layout segments to return\n", __func__);
goto out;
}
@@ -770,7 +764,8 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
if (stateid) { /* callback */
status = -EAGAIN;
spin_lock(&ino->i_lock);
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
+ spin_unlock(&ino->i_lock);
goto out;
}
dprintk("%s: waiting\n", __func__);
@@ -914,8 +909,7 @@ static int pnfs_wait_schedule(void *word)
* get, possibly allocate, and lock current_layout
*
* Note: If successful, ino->i_lock is taken and the caller
- * must put and unlock current_layout by using put_unlock_current_layout()
- * when the returned layout is released.
+ * must put and unlock current_layout when the returned layout is released.
*/
static struct pnfs_layout_type *
get_lock_alloc_layout(struct inode *ino)
@@ -926,7 +920,9 @@ get_lock_alloc_layout(struct inode *ino)
dprintk("%s Begin\n", __func__);
- while ((lo = get_lock_current_layout(nfsi)) == NULL) {
+ spin_lock(&ino->i_lock);
+ while ((lo = get_current_layout(nfsi)) == NULL) {
+ spin_unlock(&ino->i_lock);
/* Compete against other threads on who's doing the allocation,
* wait until bit is cleared if we lost this race.
*/
@@ -942,8 +938,10 @@ get_lock_alloc_layout(struct inode *ino)
/* Was current_layout already allocated while we slept?
* If so, retry get_lock'ing it. Otherwise, allocate it.
*/
- if (nfsi->layout.ld_data)
+ if (nfsi->layout.ld_data) {
+ spin_lock(&ino->i_lock);
continue;
+ }
lo = alloc_init_layout(ino);
if (lo) {
@@ -1112,7 +1110,8 @@ out:
out_put:
if (lsegpp)
*lsegpp = lseg;
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
+ spin_unlock(&ino->i_lock);
goto out;
}
@@ -1328,11 +1327,13 @@ pnfs_getboundary(struct inode *inode)
goto out;
nfsi = NFS_I(inode);
- lo = get_lock_current_layout(nfsi);;
+ spin_lock(&inode->i_lock);
+ lo = get_current_layout(nfsi);;
if (lo) {
stripe_size = policy_ops->get_stripesize(lo);
- put_unlock_current_layout(lo);
+ put_current_layout(lo);
}
+ spin_unlock(&inode->i_lock);
out:
return stripe_size;
}
--
1.6.6.1
The check on list empty before destroying a layout has always bothered me.
Get rid of it by having lsegs take a reference on their backpointer.
Signed-off-by: Fred Isaman <[email protected]>
---
fs/nfs/pnfs.c | 55 +++++++++++++++++++++++++++++++++----------------------
1 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 49093a0..c939cb0 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -60,6 +60,7 @@ static int pnfs_initialized;
static void pnfs_free_layout(struct pnfs_layout_type *lo,
struct nfs4_pnfs_layout_segment *range);
static enum pnfs_try_status pnfs_commit(struct nfs_write_data *data, int sync);
+static inline void get_layout(struct pnfs_layout_type *lo);
/* Locking:
*
@@ -153,6 +154,7 @@ pnfs_need_layoutcommit(struct nfs_inode *nfsi, struct nfs_open_context *ctx)
spin_lock(&nfsi->vfs_inode.i_lock);
if (has_layout(nfsi) && !layoutcommit_needed(nfsi)) {
nfsi->layout.lo_cred = get_rpccred(ctx->state->owner->so_cred);
+ get_layout(&nfsi->layout);
nfsi->change_attr++;
spin_unlock(&nfsi->vfs_inode.i_lock);
dprintk("%s: Set layoutcommit\n", __func__);
@@ -335,25 +337,18 @@ grab_current_layout(struct nfs_inode *nfsi)
static inline void
put_layout(struct pnfs_layout_type *lo)
{
- struct inode *inode = PNFS_INODE(lo);
- struct nfs_client *clp;
-
BUG_ON_UNLOCKED_LO(lo);
BUG_ON(lo->refcount <= 0);
- if (--lo->refcount == 0 && list_empty(&lo->segs)) {
+ lo->refcount--;
+ if (!lo->refcount) {
struct layoutdriver_io_operations *io_ops =
PNFS_LD_IO_OPS(lo);
dprintk("%s: freeing layout %p\n", __func__, lo->ld_data);
+ WARN_ON(!list_empty(&lo->lo_layouts));
io_ops->free_layout(lo->ld_data);
lo->ld_data = NULL;
-
- /* Unlist the layout. */
- clp = NFS_SERVER(inode)->nfs_client;
- spin_lock(&clp->cl_lock);
- list_del_init(&lo->lo_layouts);
- spin_unlock(&clp->cl_lock);
}
}
@@ -397,6 +392,7 @@ init_lseg(struct pnfs_layout_type *lo, struct pnfs_layout_segment *lseg)
kref_init(&lseg->kref);
lseg->valid = true;
lseg->layout = lo;
+ get_layout(lo);
}
static void
@@ -406,6 +402,7 @@ destroy_lseg(struct kref *kref)
container_of(kref, struct pnfs_layout_segment, kref);
dprintk("--> %s\n", __func__);
+ put_layout(lseg->layout);
PNFS_LD_IO_OPS(lseg->layout)->free_lseg(lseg);
}
@@ -669,6 +666,15 @@ pnfs_free_layout(struct pnfs_layout_type *lo,
lseg->range.length);
list_del(&lseg->fi_list);
put_lseg_locked(lseg);
+ if (list_empty(&lo->segs)) {
+ struct nfs_client *clp;
+
+ clp = PNFS_NFS_SERVER(lo)->nfs_client;
+ spin_lock(&clp->cl_lock);
+ list_del_init(&lo->lo_layouts);
+ spin_unlock(&clp->cl_lock);
+ put_layout(lo);
+ }
}
dprintk("%s:Return\n", __func__);
@@ -854,6 +860,15 @@ pnfs_insert_layout(struct pnfs_layout_type *lo,
dprintk("%s:Begin\n", __func__);
BUG_ON_UNLOCKED_LO(lo);
+ if (list_empty(&lo->segs)) {
+ struct nfs_client *clp = PNFS_NFS_SERVER(lo)->nfs_client;
+
+ spin_lock(&clp->cl_lock);
+ BUG_ON(!list_empty(&lo->lo_layouts));
+ list_add_tail(&lo->lo_layouts, &clp->cl_layouts);
+ spin_unlock(&clp->cl_lock);
+ get_layout(lo);
+ }
list_for_each_entry (lp, &lo->segs, fi_list) {
if (cmp_layout(&lp->range, &lseg->range) > 0)
continue;
@@ -896,11 +911,13 @@ alloc_init_layout(struct inode *ino)
return NULL;
}
+ spin_lock(&ino->i_lock);
BUG_ON(lo->ld_data != NULL);
lo->ld_data = ld_data;
memset(&lo->stateid, 0, NFS4_STATEID_SIZE);
lo->refcount = 1;
lo->roc_iomode = 0;
+ spin_unlock(&ino->i_lock);
return lo;
}
@@ -951,17 +968,9 @@ get_lock_alloc_layout(struct inode *ino)
}
lo = alloc_init_layout(ino);
- if (lo) {
- struct nfs_client *clp = NFS_SERVER(ino)->nfs_client;
-
- /* must grab the layout lock before the client lock */
+ if (lo)
spin_lock(&ino->i_lock);
-
- spin_lock(&clp->cl_lock);
- if (list_empty(&lo->lo_layouts))
- list_add_tail(&lo->lo_layouts, &clp->cl_layouts);
- spin_unlock(&clp->cl_lock);
- } else
+ else
lo = ERR_PTR(-ENOMEM);
/* release the NFS_INO_LAYOUT_ALLOC bit and wake up waiters */
@@ -1247,14 +1256,13 @@ pnfs_layout_process(struct nfs4_pnfs_layoutget *lgp)
goto out;
}
+ spin_lock(&ino->i_lock);
init_lseg(lo, lseg);
lseg->range = res->lseg;
if (lgp->lsegpp) {
get_lseg(lseg);
*lgp->lsegpp = lseg;
}
-
- spin_lock(&ino->i_lock);
pnfs_insert_layout(lo, lseg);
if (res->return_on_close) {
@@ -1642,6 +1650,9 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync)
}
status = pnfs4_proc_layoutcommit(data, sync);
out:
+ spin_lock(&inode->i_lock);
+ put_layout(&nfsi->layout);
+ spin_unlock(&inode->i_lock);
dprintk("%s end (err:%d)\n", __func__, status);
return status;
out_free:
--
1.6.6.1
Split get_layout (inc refcount) and grab_current_layout (find layout and inc refcount)
functionality and rename appropriately.
Signed-off-by: Fred Isaman <[email protected]>
---
fs/nfs/pnfs.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index ed4c72e..49093a0 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -313,20 +313,27 @@ pnfs_unregister_layoutdriver(struct pnfs_layoutdriver_type *ld_type)
#define BUG_ON_UNLOCKED_LO(lo) do {} while (0)
#endif /* CONFIG_SMP */
+static inline void
+get_layout(struct pnfs_layout_type *lo)
+{
+ BUG_ON_UNLOCKED_LO(lo);
+ lo->refcount++;
+}
+
static inline struct pnfs_layout_type *
-get_current_layout(struct nfs_inode *nfsi)
+grab_current_layout(struct nfs_inode *nfsi)
{
struct pnfs_layout_type *lo = &nfsi->layout;
BUG_ON_UNLOCKED_LO(lo);
if (!lo->ld_data)
return NULL;
- lo->refcount++;
+ get_layout(lo);
return lo;
}
static inline void
-put_current_layout(struct pnfs_layout_type *lo)
+put_layout(struct pnfs_layout_type *lo)
{
struct inode *inode = PNFS_INODE(lo);
struct nfs_client *clp;
@@ -359,7 +366,7 @@ pnfs_layout_release(struct pnfs_layout_type *lo,
spin_lock(&nfsi->vfs_inode.i_lock);
if (range)
pnfs_free_layout(lo, range);
- put_current_layout(lo);
+ put_layout(lo);
spin_unlock(&nfsi->vfs_inode.i_lock);
wake_up_all(&nfsi->lo_waitq);
}
@@ -375,10 +382,10 @@ pnfs_destroy_layout(struct nfs_inode *nfsi)
};
spin_lock(&nfsi->vfs_inode.i_lock);
- lo = get_current_layout(nfsi);
+ lo = grab_current_layout(nfsi);
if (lo) {
pnfs_free_layout(lo, &range);
- put_current_layout(lo);
+ put_layout(lo);
}
spin_unlock(&nfsi->vfs_inode.i_lock);
}
@@ -548,7 +555,7 @@ pnfs_layout_from_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
* arg->length: all ones
*/
static int
-get_layout(struct inode *ino,
+send_layoutget(struct inode *ino,
struct nfs_open_context *ctx,
struct nfs4_pnfs_layout_segment *range,
struct pnfs_layout_segment **lsegpp,
@@ -744,9 +751,9 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
if (type == RETURN_FILE) {
spin_lock(&ino->i_lock);
- lo = get_current_layout(nfsi);
+ lo = grab_current_layout(nfsi);
if (lo && !has_layout_to_return(lo, &arg)) {
- put_current_layout(lo);
+ put_layout(lo);
lo = NULL;
}
if (!lo) {
@@ -764,7 +771,7 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
if (stateid) { /* callback */
status = -EAGAIN;
spin_lock(&ino->i_lock);
- put_current_layout(lo);
+ put_layout(lo);
spin_unlock(&ino->i_lock);
goto out;
}
@@ -921,7 +928,7 @@ get_lock_alloc_layout(struct inode *ino)
dprintk("%s Begin\n", __func__);
spin_lock(&ino->i_lock);
- while ((lo = get_current_layout(nfsi)) == NULL) {
+ while ((lo = grab_current_layout(nfsi)) == NULL) {
spin_unlock(&ino->i_lock);
/* Compete against other threads on who's doing the allocation,
* wait until bit is cleared if we lost this race.
@@ -1102,7 +1109,7 @@ _pnfs_update_layout(struct inode *ino,
/* Lose lock, but not reference, match this with pnfs_layout_release */
spin_unlock(&ino->i_lock);
- get_layout(ino, ctx, &arg, lsegpp, lo);
+ send_layoutget(ino, ctx, &arg, lsegpp, lo);
out:
dprintk("%s end, state 0x%lx lseg %p\n", __func__,
nfsi->layout.pnfs_layout_state, lseg);
@@ -1110,7 +1117,7 @@ out:
out_put:
if (lsegpp)
*lsegpp = lseg;
- put_current_layout(lo);
+ put_layout(lo);
spin_unlock(&ino->i_lock);
goto out;
}
@@ -1328,10 +1335,10 @@ pnfs_getboundary(struct inode *inode)
nfsi = NFS_I(inode);
spin_lock(&inode->i_lock);
- lo = get_current_layout(nfsi);;
+ lo = grab_current_layout(nfsi);;
if (lo) {
stripe_size = policy_ops->get_stripesize(lo);
- put_current_layout(lo);
+ put_layout(lo);
}
spin_unlock(&inode->i_lock);
out:
--
1.6.6.1
On 2010-06-16 19:44, Fred Isaman wrote:
> The check on list empty before destroying a layout has always bothered me.
> Get rid of it by having lsegs take a reference on their backpointer.
>
> Signed-off-by: Fred Isaman <[email protected]>
> ---
> fs/nfs/pnfs.c | 55 +++++++++++++++++++++++++++++++++----------------------
> 1 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 49093a0..c939cb0 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -60,6 +60,7 @@ static int pnfs_initialized;
> static void pnfs_free_layout(struct pnfs_layout_type *lo,
> struct nfs4_pnfs_layout_segment *range);
> static enum pnfs_try_status pnfs_commit(struct nfs_write_data *data, int sync);
> +static inline void get_layout(struct pnfs_layout_type *lo);
>
> /* Locking:
> *
> @@ -153,6 +154,7 @@ pnfs_need_layoutcommit(struct nfs_inode *nfsi, struct nfs_open_context *ctx)
> spin_lock(&nfsi->vfs_inode.i_lock);
> if (has_layout(nfsi) && !layoutcommit_needed(nfsi)) {
> nfsi->layout.lo_cred = get_rpccred(ctx->state->owner->so_cred);
> + get_layout(&nfsi->layout);
looks like a fallout from a different patch?
> nfsi->change_attr++;
> spin_unlock(&nfsi->vfs_inode.i_lock);
> dprintk("%s: Set layoutcommit\n", __func__);
> @@ -335,25 +337,18 @@ grab_current_layout(struct nfs_inode *nfsi)
> static inline void
> put_layout(struct pnfs_layout_type *lo)
> {
> - struct inode *inode = PNFS_INODE(lo);
> - struct nfs_client *clp;
> -
> BUG_ON_UNLOCKED_LO(lo);
> BUG_ON(lo->refcount <= 0);
>
> - if (--lo->refcount == 0 && list_empty(&lo->segs)) {
> + lo->refcount--;
> + if (!lo->refcount) {
> struct layoutdriver_io_operations *io_ops =
> PNFS_LD_IO_OPS(lo);
>
> dprintk("%s: freeing layout %p\n", __func__, lo->ld_data);
> + WARN_ON(!list_empty(&lo->lo_layouts));
> io_ops->free_layout(lo->ld_data);
> lo->ld_data = NULL;
> -
> - /* Unlist the layout. */
> - clp = NFS_SERVER(inode)->nfs_client;
> - spin_lock(&clp->cl_lock);
> - list_del_init(&lo->lo_layouts);
> - spin_unlock(&clp->cl_lock);
> }
> }
>
> @@ -397,6 +392,7 @@ init_lseg(struct pnfs_layout_type *lo, struct pnfs_layout_segment *lseg)
> kref_init(&lseg->kref);
> lseg->valid = true;
> lseg->layout = lo;
> + get_layout(lo);
> }
>
> static void
> @@ -406,6 +402,7 @@ destroy_lseg(struct kref *kref)
> container_of(kref, struct pnfs_layout_segment, kref);
>
> dprintk("--> %s\n", __func__);
> + put_layout(lseg->layout);
> PNFS_LD_IO_OPS(lseg->layout)->free_lseg(lseg);
> }
>
> @@ -669,6 +666,15 @@ pnfs_free_layout(struct pnfs_layout_type *lo,
> lseg->range.length);
> list_del(&lseg->fi_list);
> put_lseg_locked(lseg);
> + if (list_empty(&lo->segs)) {
> + struct nfs_client *clp;
> +
> + clp = PNFS_NFS_SERVER(lo)->nfs_client;
> + spin_lock(&clp->cl_lock);
> + list_del_init(&lo->lo_layouts);
> + spin_unlock(&clp->cl_lock);
> + put_layout(lo);
> + }
How about doing this outside the list_for_each_entry_safe loop?
I don't see a need for checking after every list_del...
> }
>
> dprintk("%s:Return\n", __func__);
> @@ -854,6 +860,15 @@ pnfs_insert_layout(struct pnfs_layout_type *lo,
> dprintk("%s:Begin\n", __func__);
>
> BUG_ON_UNLOCKED_LO(lo);
> + if (list_empty(&lo->segs)) {
> + struct nfs_client *clp = PNFS_NFS_SERVER(lo)->nfs_client;
> +
> + spin_lock(&clp->cl_lock);
> + BUG_ON(!list_empty(&lo->lo_layouts));
> + list_add_tail(&lo->lo_layouts, &clp->cl_layouts);
> + spin_unlock(&clp->cl_lock);
> + get_layout(lo);
> + }
> list_for_each_entry (lp, &lo->segs, fi_list) {
> if (cmp_layout(&lp->range, &lseg->range) > 0)
> continue;
> @@ -896,11 +911,13 @@ alloc_init_layout(struct inode *ino)
> return NULL;
> }
>
> + spin_lock(&ino->i_lock);
> BUG_ON(lo->ld_data != NULL);
> lo->ld_data = ld_data;
> memset(&lo->stateid, 0, NFS4_STATEID_SIZE);
> lo->refcount = 1;
> lo->roc_iomode = 0;
> + spin_unlock(&ino->i_lock);
this hunk seems unrelated to this patch, no?
> return lo;
> }
>
> @@ -951,17 +968,9 @@ get_lock_alloc_layout(struct inode *ino)
> }
>
> lo = alloc_init_layout(ino);
> - if (lo) {
> - struct nfs_client *clp = NFS_SERVER(ino)->nfs_client;
> -
> - /* must grab the layout lock before the client lock */
> + if (lo)
> spin_lock(&ino->i_lock);
> -
> - spin_lock(&clp->cl_lock);
> - if (list_empty(&lo->lo_layouts))
> - list_add_tail(&lo->lo_layouts, &clp->cl_layouts);
> - spin_unlock(&clp->cl_lock);
> - } else
> + else
> lo = ERR_PTR(-ENOMEM);
>
> /* release the NFS_INO_LAYOUT_ALLOC bit and wake up waiters */
> @@ -1247,14 +1256,13 @@ pnfs_layout_process(struct nfs4_pnfs_layoutget *lgp)
> goto out;
> }
>
> + spin_lock(&ino->i_lock);
> init_lseg(lo, lseg);
> lseg->range = res->lseg;
> if (lgp->lsegpp) {
> get_lseg(lseg);
> *lgp->lsegpp = lseg;
> }
> -
> - spin_lock(&ino->i_lock);
> pnfs_insert_layout(lo, lseg);
>
> if (res->return_on_close) {
> @@ -1642,6 +1650,9 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync)
> }
> status = pnfs4_proc_layoutcommit(data, sync);
> out:
> + spin_lock(&inode->i_lock);
> + put_layout(&nfsi->layout);
> + spin_unlock(&inode->i_lock);
same fallout as earlier?
Otherwise, the patchset looks good!
Benny
> dprintk("%s end (err:%d)\n", __func__, status);
> return status;
> out_free: