[pnfs] [PATCH 12/18] pnfs client layoutcommit

[William A. (Andy) Adamson]

On Jan 14, 2008 10:54 AM, Benny Halevy <bhalevy at panasas.com> wrote:

> Andy, this patch causes the holey test failure
> when doing a straight, non-pnfs nfs41 mount.
> See comment below...
>
> Benny
>
> On Jan. 07, 2008, 22:47 +0200, andros at umich.edu wrote:
> > From: Andy Adamson <andros at umich.edu>
> >
> > Implement the layoutcommit operation
> >
> > Signed-off by: Andy Adamson<andros at umich.edu>
> > ---
> >  fs/nfs/inode.c     |   16 ++++++
> >  fs/nfs/nfs4proc.c  |   57 +++++++++++++++++++---
> >  fs/nfs/nfs4state.c |    2 +
> >  fs/nfs/nfs4xdr.c   |  133
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/nfs/write.c     |    6 ++
> >  5 files changed, 206 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 4c47ecb..136d870 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -1005,6 +1005,16 @@ static int nfs_update_inode(struct inode *inode,
> struct nfs_fattr *fattr)
> >                       && !nfs_fsid_equal(&server->fsid, &fattr->fsid))
> >               server->fsid = fattr->fsid;
> >
> > +#ifdef CONFIG_PNFS
> > +     /*
> > +     * file needs layout commit, server attributes may be stale
> > +     */
> > +     if (nfsi->layoutcommit_ctx && nfsi->change_attr >=
> fattr->change_attr) {
> > +             dprintk("NFS: %s: layoutcommit is needed for file
> %s/%ld\n",
> > +                             __FUNCTION__, inode->i_sb->s_id,
> inode->i_ino);
> > +             return 0;
> > +     }
> > +#endif /* CONFIG_PNFS */
> >       /*
> >        * Update the read time so we don't revalidate too often.
> >        */
> > @@ -1041,7 +1051,13 @@ static int nfs_update_inode(struct inode *inode,
> struct nfs_fattr *fattr)
> >       if (new_isize != cur_isize) {
> >               /* Do we perhaps have any outstanding writes, or has
> >                * the file grown beyond our last write? */
> > +#ifdef CONFIG_PNFS
> > +             /* XXX Andros: is the check for layoutcommit_ctx needed?
> */
> > +             if (nfsi->npages == 0 || new_isize > cur_isize ||
> > +                 !nfsi->layoutcommit_ctx) {
> > +#else
> >               if (nfsi->npages == 0 || new_isize > cur_isize) {
> > +#endif /* CONFIG_PNFS */
> >                       inode->i_size = new_isize;
> >                       invalid |=
> NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
>
> With some instrumentation help is appears the your question above was in
> place ;-)


Hi Benny. Good job finding my bug!

>
>
> here's the test failure:
> test holey file support
> read (hole) offset 17654, sz = 43334, bytes = 8192 (ret 0), holesz = 9012
>
> and here are the printk's I added for this case:
> Jan 14 17:45:25 bh-testlin1 kernel: NFS: isize change on server for file
> 0:18/4489245 npages=11 new_i
> size=4321 old_isize=70000 layoutcommit_ctx=0000000000000000
>
> so with nfs41 mount nfsi->layoutcommit_ctx is always NULL which clearly
> should effectively follow the !defined(CONFIG_PNFS) path.
> What did you try to achieve in this check?


The check  exists in the 2.6.18.3 codebase - reviewing it now, I see that I
did not port the 2.6.18.3 logic forward. Sorry 'bout that.  The check should
be removed.

-->Andy



>
> Benny
>
>
> _______________________________________________
> 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/20080114/16454331/attachment.htm