[pnfs] some problems since last week
Marc Eshel
eshel at almaden.ibm.com
Sat Jun 2 20:18:16 EDT 2007
Here is a list of few problem I found in the code that was added this week
since last week version was working. Some of the fixes are temporary so
once it is reviewed I will break it to a separate fix for each problem.
Lets try at this point to check in only code that was tested.
1. The export operation layout_get() must pass the file handle to the file
system. I added it back and made few changes in how the fh is returned
from this operation.
2. nops is initialized to 1 which cause all callbacks on the client to be
refused with NFS4ERR_OP_NOT_IN_SESSION
3. encode_cb_compound_hdr() is hard coded to set minorversion to ZERO
4. The export operation cb_layout_recall() need to pass fsid for the
RECALL_FSID option, so I added it back to the struct nfsd4_pnfs_cb_layout
5. client exchange id flags should be save before overwritten by serve
flags.
6. The serve is not waiting for the client to return the layout before
calling destroy_layoutrecall() it because the hold count clr_ref is not
set correctly. This problem is not fixed except for removing the BUG that
causes the crash.
Marc.
> From: Marc Eshel <eshel at almaden.ibm.com>
>
>
> ---
>
> fs/nfs/callback_xdr.c | 6 ++++--
> fs/nfsd/nfs4callback.c | 8 ++++++--
> fs/nfsd/nfs4filelayoutxdr.c | 15 +++------------
> fs/nfsd/nfs4state.c | 10 ++++++----
> include/linux/nfsd/nfs4layoutxdr.h | 2 +-
> include/linux/nfsd/nfsd4_pnfs.h | 3 ++-
> include/linux/nfsd/state.h | 1 +
> 7 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index 4fdcbc9..d68e933 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
>
> @@ -618,7 +619,8 @@ static int nfs4_callback_compound(struct
> struct xdr_stream xdr_in, xdr_out;
> uint32_t *p;
> unsigned int status;
-- problem 2 --
> - unsigned int nops = 1;
> + unsigned int nops = 0;
>
> dprintk("%s: start\n", __FUNCTION__);
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 2e8e46e..b1dfe8b 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -267,7 +267,8 @@ encode_cb_compound_hdr(struct xdr_stream
>
> RESERVE_SPACE(16);
> WRITE32(0); /* tag length is always 0 */
-- problem 3 --
> - WRITE32(0); /* minorversion */
> + WRITE32(1); /* minorversion */
> WRITE32(hdr->ident);
> WRITE32(hdr->nops);
> return 0;
> @@ -322,7 +323,7 @@ encode_cb_layout(struct xdr_stream *xdr,
> WRITE32(clr->cb.cbl_layoutchanged);
> WRITE32(clr->cb.cbl_recall_type);
> if (unlikely(clr->cb.cbl_recall_type == RECALL_FSID)) {
-- probel 4 --
> - struct nfs4_fsid fsid = clr->clr_file->fi_fsid;
> + struct nfs4_fsid fsid = clr->cb.cbl_fsid;
> WRITE64(fsid.major);
> WRITE64(fsid.minor);
> dprintk("%s: type %d iomode %d changed %d recall_type %d fsid
> 0x%llx-0x%llx\n",
> diff --git a/fs/nfsd/nfs4filelayoutxdr.c b/fs/nfsd/nfs4filelayoutxdr.c
> index 1258dea..68ea45d 100644
> --- a/fs/nfsd/nfs4filelayoutxdr.c
> +++ b/fs/nfsd/nfs4filelayoutxdr.c
> @@ -87,7 +87,7 @@ static int
> filelayout_encode_layoutlist_item(u32 *p, u32 *end, struct
> nfsd4_pnfs_layoutlist *item)
> {
> int len;
-- problem 1 --
> - unsigned int fhlen = item->fhp->fh_size;
> + unsigned int fhlen = item->dev_fh.fh_size;
>
> len = 16 + fhlen;
> if (p + XDR_QUADLEN(len) > end)
> @@ -96,7 +96,7 @@ filelayout_encode_layoutlist_item(u32 *p
> WRITE32(item->dev_id);
> WRITE32(item->dev_index);
> WRITE32(fhlen);
> - WRITEMEM(&item->fhp->fh_base, fhlen);
> + WRITEMEM(&item->dev_fh.fh_base, fhlen);
> return len;
> }
>
> @@ -150,21 +150,12 @@ void
> filelayout_free_layout(void *layout)
> {
> struct nfsd4_pnfs_filelayout *flp;
> - struct nfsd4_pnfs_layoutlist *item;
> - int i;
>
> flp = (struct nfsd4_pnfs_filelayout *)layout;
>
> if (!flp || !flp->lg_llist)
> return;
> - item = flp->lg_llist;
> - for (i=0; i < flp->lg_llistlen; i++) {
> -#if 0 /* the fh is part of nfsd4_pnfs_layoutget struct */
> - if (item->fhp)
> - kfree(item->fhp);
> -#endif
> - item++;
> - }
> +
> kfree(flp->lg_llist);
> }
> EXPORT_SYMBOL(filelayout_free_layout);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ee4125b..dbd936c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1202,6 +1202,7 @@ out_copy:
> clid->clientid.cl_id = new->cl_clientid.cl_id;
>
-- problem 5 --
> new->cl_seqid = clid->seqid = 1;
> + new->cl_exchange_flags = clid->flags;
> nfsd4_set_ex_flags(new, clid);
>
> dprintk("nfsd4_exchange_id seqid %d flags %x\n",
> @@ -4028,7 +4029,7 @@ destroy_layoutrecall(struct kref *kref)
> container_of(kref, struct nfs4_layoutrecall, clr_ref);
> dprintk("pNFS %s: clr %p fp %p clp %p\n", __FUNCTION__, clr,
> clr->clr_file, clr->clr_client);
-- problem 6 --
> - BUG_ON(!list_empty(&clr->clr_perclnt));
> if (clr->clr_file)
> put_nfs4_file(clr->clr_file);
> kmem_cache_free(pnfs_layoutrecall_slab, clr);
> @@ -4224,6 +4225,7 @@ int nfs4_pnfs_get_layout(struct super_bl
> }
>
-- problem 1 --
> BUG_ON(!sb->s_export_op->layout_get);
> + lgp->lg_fh = ¤t_fh->fh_handle;
> status = sb->s_export_op->layout_get(current_fh->fh_dentry->d_inode,
> (void *)lgp);
>
> @@ -4357,7 +4359,7 @@ recall_return_partial_match(struct nfs4_
> /* fsid matches? */
> if (clr->cb.cbl_recall_type == RECALL_FSID ||
> lrp->lr_return_type == RETURN_FSID)
-- problem 4 --
> - return same_fsid(&clr->clr_file->fi_fsid, current_fh);
> + return same_fsid(&clr->cb.cbl_fsid, current_fh);
>
> /* file matches, range overlapping? */
> return clr->clr_file == fp &&
> @@ -4500,7 +4502,7 @@ cl_has_fsid_layout(struct nfs4_client *c
>
> /* note: minor version unused */
> list_for_each_entry(lp, &clp->cl_layouts, lo_perclnt)
> - if (lp->lo_file->fi_fsid.major == clr->clr_file->fi_fsid.major)
> + if (lp->lo_file->fi_fsid.major == clr->cb.cbl_fsid.major)
> return 1;
>
> return 0;
> @@ -4619,7 +4621,7 @@ int nfsd_layout_recall_cb(struct inode *
> BUG_ON(cbl->cbl_recall_type != RECALL_FILE &&
> cbl->cbl_recall_type != RECALL_FSID &&
> cbl->cbl_recall_type != RECALL_ALL);
> - BUG_ON(cbl->cbl_recall_type != RECALL_ALL && !inode);
> + BUG_ON(cbl->cbl_recall_type == RECALL_FILE && !inode);
>
> clr = alloc_init_layoutrecall(NULL);
> if (!clr)
> diff --git a/include/linux/nfsd/nfs4layoutxdr.h
> b/include/linux/nfsd/nfs4layoutxdr.h
> index 64f8216..9f69431 100644
> --- a/include/linux/nfsd/nfs4layoutxdr.h
> +++ b/include/linux/nfsd/nfs4layoutxdr.h
> @@ -67,7 +67,7 @@ struct nfsd4_pnfs_layoutlist {
> u32 dev_layout_type;
> u32 dev_id;
> u32 dev_index;
-- problem 1 --
> - struct knfsd_fh *fhp;
> + struct knfsd_fh dev_fh;
> };
>
> struct nfsd4_pnfs_filelayout {
> diff --git a/include/linux/nfsd/nfsd4_pnfs.h
b/include/linux/nfsd/nfsd4_pnfs.h
> index e133c9c..c25ede7 100644
> --- a/include/linux/nfsd/nfsd4_pnfs.h
> +++ b/include/linux/nfsd/nfsd4_pnfs.h
> @@ -84,7 +84,7 @@ struct nfsd4_pnfs_layoutget {
> u64 lg_minlength; /* request */
> u32 lg_mxcnt; /* request */
> struct export_operations *lg_ops;
> -
> + struct knfsd_fh *lg_fh;
> u32 lg_return_on_close; /* response */
> void *lg_layout; /* response callback encoded */
> };
> @@ -117,6 +117,7 @@ struct nfsd4_pnfs_cb_layout {
> u32 cbl_recall_type; /* request */
> struct nfsd4_layout_seg cbl_seg; /* request */
> u32 cbl_layoutchanged; /* request */
-- problem 4 --
> + struct nfs4_fsid cbl_fsid;
> };
>
> #endif /* _LINUX_NFSD_NFSD4_PNFS_H */
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index 7ed3863..953c1d2 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -269,6 +269,7 @@ struct nfs4_layoutrecall {
> struct nfs4_file *clr_file;
> int clr_status;
> struct timespec clr_time; /* last activity */
> + struct nfs4_fsid cbr_fsid;
> };
>
> #endif /* CONFIG_PNFS */
More information about the pNFS
mailing list