On 06/29/2010 07:42 PM, andros@xxxxxxxxxx wrote:
> From: Fred Isaman <iisaman@xxxxxxxxxx>
>
> Prepare to embed struct pnfs_layout_tupe into layout driver private structs
> and to make layout a pointer. Since a temp error on first LAYOUTGET
> erases the layout, the suspend timer needs to be moved.
>
OK Fred, Andy. (I'm also referring to the 1/5 and 2/5 issues here)
There is a total mess up. LAYOUTGET has nothing to do with layout
(.eg struct pnfs_layout_type) and the layout must not be deallocated
if there is any kind of error and/or if IO is actually to be done with
MDS or not. LAYOUTGET corresponds to layout_seg held on a list in layout.
The list might be empty and so it's flags and state.
Half of the problem was before and this here made it worse. The
life time of nfsi->layout should be the same as nfsi itself just
as it was before.
Once the life_time of nfsi && nfsi->layout are unified then all your
problems go away because you are not checking for existence of nfsi
anywhere right? that's because there is no such problem.
Look at it this way. If a layout_driver is in the game it gets to
allocate it's own area in NFS_I. part of that area is for generic
pnfs.c use, i.e struct pnfs_layout_type, the rest is for private use.
once existing it is part of the NFS_I life up till death.
If it's not so it should be fixed, not some members leaking out
to generic structures.
Boaz
> Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx>
> ---
> fs/nfs/inode.c | 1 +
> fs/nfs/pnfs.c | 8 ++++----
> include/linux/nfs_fs.h | 2 +-
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 66d7be2..cb12d33 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1420,6 +1420,7 @@ static void pnfs_init_once(struct nfs_inode *nfsi)
> #ifdef CONFIG_NFS_V4_1
> init_waitqueue_head(&nfsi->lo_waitq);
> nfsi->pnfs_layout_state = 0;
> + nfsi->pnfs_layout_suspend = 0;
> seqlock_init(&nfsi->layout.seqlock);
> INIT_LIST_HEAD(&nfsi->layout.lo_layouts);
> INIT_LIST_HEAD(&nfsi->layout.segs);
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index d05cb03..7f6ace2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1105,12 +1105,12 @@ _pnfs_update_layout(struct inode *ino,
>
> /* if get layout already failed once goto out */
> if (test_bit(lo_fail_bit(iomode), &nfsi->pnfs_layout_state)) {
> - if (unlikely(nfsi->layout.pnfs_layout_suspend &&
> - get_seconds() >= nfsi->layout.pnfs_layout_suspend)) {
> + if (unlikely(nfsi->pnfs_layout_suspend &&
> + get_seconds() >= nfsi->pnfs_layout_suspend)) {
> dprintk("%s: layout_get resumed\n", __func__);
> clear_bit(lo_fail_bit(iomode),
> &nfsi->pnfs_layout_state);
> - nfsi->layout.pnfs_layout_suspend = 0;
> + nfsi->pnfs_layout_suspend = 0;
> } else
> goto out_put;
> }
> @@ -1224,7 +1224,7 @@ pnfs_get_layout_done(struct nfs4_pnfs_layoutget *lgp, int rpc_status)
> }
>
> /* remember that get layout failed and suspend trying */
> - nfsi->layout.pnfs_layout_suspend = suspend;
> + nfsi->pnfs_layout_suspend = suspend;
> set_bit(lo_fail_bit(lgp->args.lseg.iomode),
> &nfsi->pnfs_layout_state);
> dprintk("%s: layout_get suspended until %ld\n",
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index e216e24..cebc958 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -105,7 +105,6 @@ struct pnfs_layout_type {
> seqlock_t seqlock; /* Protects the stateid */
> nfs4_stateid stateid;
> void *ld_data; /* layout driver private data */
> - time_t pnfs_layout_suspend;
> struct rpc_cred *lo_cred; /* layoutcommit credential */
> /* DH: These vars keep track of the maximum write range
> * so the values can be used for layoutcommit.
> @@ -208,6 +207,7 @@ struct nfs_inode {
> #define NFS_INO_RW_LAYOUT_FAILED 1 /* rw LAYOUTGET failed stop trying */
> #define NFS_INO_LAYOUT_ALLOC 2 /* bit lock for layout allocation */
> #define NFS_INO_LAYOUTCOMMIT 3 /* LAYOUTCOMMIT needed */
> + time_t pnfs_layout_suspend;
> #endif /* CONFIG_NFS_V4_1 */
> #endif /* CONFIG_NFS_V4*/
> #ifdef CONFIG_NFS_FSCACHE
--
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
