[pnfs] some problems since last week

Marc Eshel eshel at almaden.ibm.com
Mon Jun 4 12:34:28 EDT 2007


Can you please merge the changes with the master.
Thanks, Marc.




"William A. (Andy) Adamson" <andros at citi.umich.edu> 
Sent by: androsadamson at gmail.com
06/04/2007 08:10 AM

To
"Marc Eshel" <eshel at almaden.ibm.com>
cc
pnfs at linux-nfs.org
Subject
Re: [pnfs] some problems since last week






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 





More information about the pNFS mailing list