[pnfs] some problems since last week

William A. (Andy) Adamson andros at citi.umich.edu
Mon Jun 4 11:10:26 EDT 2007


Hi Marc

Thanks for the bug fixex.

Here is the last merge of the sessions branch with the master.

I just applied benny's latest patch which should take care of problem

Author: Andy Adamson < andros at umich.edu>  2007-06-01 14:29:13
Committer: Andy Adamson <andros at umich.edu>  2007-06-01 14:29:13
Parent: cb6590a765749e4cfab14a8b1f7859ad6d181325 (no callback for DS)
Parent: 2aee0c52489d99d5afb1aa14701c0949b7f90a11 (nfsd4_exchange_id cmp_cred
bug fix)
Child:  da94b53d1b66258c4bf23ea73c91f828f38b07c8 (cb_layout: send
cb_sequence in compound)
Branches: remotes/origin/master, my-master-6-1-update

Problem #3 and #5 were known. #3 is code benny included in his non-sessions
version of the server side of his cb_sequence patch set, and is added with
his latest patch. problem #5 was mentioned by me during the last phone
conference - how does the server know if it is a DS or MDS, does it matter
WRT setting exchange_id flags.

I've applied benny's latest patch which should correctly solve problem #3
and benny's 'check read_buf result in decode_pnfs_layoutrecall_args' patch.
i'll test prior to pushing.

Please examine the master branch to ensure all outstanding patches have been
applied.

-->Andy

On 6/2/07, Marc Eshel < eshel at almaden.ibm.com> wrote:
>
> 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 = &current_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 */
>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070604/190b370b/attachment-0001.htm 


More information about the pNFS mailing list