[pnfs] Fwd: [PATCH 0/14] linux-pnfs-2.6-latest pNFS client read I/O - Draft 13

[William A. (Andy) Adamson]

ummm! i didn't 'cc the list!!!!


---------- Forwarded message ----------
From: William A. (Andy) Adamson <andros at citi.umich.edu>
Date: Nov 15, 2007 12:38 PM
Subject: Re: [pnfs] [PATCH 0/14] linux-pnfs-2.6-latest pNFS client
read I/O - Draft 13
To: Benny Halevy <bhalevy at panasas.com>


On Nov 15, 2007 6:12 AM, Benny Halevy <bhalevy at panasas.com> wrote:
>
> On Nov. 14, 2007, 19:23 +0200, Trond Myklebust <trond.myklebust at fys.uio.no> wrote:
> > On Wed, 2007-11-14 at 12:11 -0500, William A. (Andy) Adamson wrote:
> >
> >> for example in fs/nfs/read.c: nfs_read_rpcsetup()
> >>
> >> ifdef CONFIG_PNFS
> >>         /* XXX pnfs_try_to_read_data should never return an error less than 0.
> >>         * ret == 0 means pnfs read succees.
> >>         * ret == 1 means do an NFSv4.1 read to the MDS.
> >>         */
> >>         if ((pnfs_try_to_read_data(data, call_ops)) <= 0)
> >>                 return;
> >> #endif /* CONFIG_PNFS*/
> >>
> >>         nfs_initiate_read(data, NFS_CLIENT(inode), call_ops);
> >> }
> >
> > The above is a seriously brilliant comment! Are we perhaps missing a
> > BUG_ON() there?
>
> Actually, I think it's just wrong.
> > 0 means do MDS read
> = 0 means success
> < 0 means pnfs_readpages failed and returned an error
>   in this case the 2.6.18.3 code base returned an error from nfs_read_rpcsetup
>   which is sort of legitimate (not that I like doing the I/O form within
>   nfs_read_rpcsetup, but that's a separate issue)
>   in the future we will want to retry the I/O via the MDS.

the future is now! a failure from pnfs_try_to_read means try via the
MDS. there is no reason to put this off any longer.  the 2.6.18.3 code
is wrong.

> Anyhow, this is a good example why having a thorough process for porting
> the patches is so important.  Making changes in the porting process is
> inevitable since the environment changed (a lot), however changing the
> functionality and/or fixing bugs (or what we think are bugs) in the
> process is quite risky.

this is a bug. there is no reason to propagate bugs. instead, i
propose that in the "porting process" we take the time to improve the
code and handle all the issues that we have ignored in order to get a
prototype. the 2.6.18.3 code does not many error cases. the 2.6-latest
code should.

>
> I tried pulling the changes from linux-pnfs to linux-pnfs-2.6
> in order to establish a depot for the patches we they can be manipulated
> more easily in git by resetting a branch in linux-pnfs-2.6 to v2.6.18
> which is a common ancestor of both trees. This went ok, but attempting
> to rebase the patchset onto a more recent point went horribly wrong.
>
> Basically every merge point in which sessions patches were pushed to
> the master branch presented conflicts when trying to do the rebase and
> the merge has to be replayed (and since we didn't enabled rerere there
> doesn't seem to be an easy way to do that since git-show of the merge
> commit is insufficient to resolve the merge conflict.  Is there
> an easy way to do this with git that I'm not aware of?

there is no reason to worry about this. the 2.6.18.3 code is no good.
it gave us experience - fine. but it has glaring problems that should
be fixed, not propoagated.

>
> After discussing this with Boaz we came to the conclusion that
> using rebase rather than merge should do a better job in integrating
> the pnfs patches on top of the sessions patches as the each committed
> patch after the rebase will be in sync with the branch so I'm going to
> retry this by applying the sessions patches onto a 2.6.18.3 branch
> (off of linux-pnfs-2.6) and having a sub-branch of that for the pnfs-patches
> which I'll rebase at the merge points we have in the 2.6.18.3 tree so that
> we'll have a coherent set of patches to play with.

i disagree with a merge. if you want all the bugs, use 2.6.18.3.

-->Andy


>
> Benny
>
> >
> > Trond
> >
>
>
> --
> Benny Halevy
> Software Architect
> Tel/Fax: +972-3-647-8340
> Mobile: +972-54-802-8340
> US:      +1-412-203-3187
> bhalevy at panasas.com
>
> Panasas, Inc.
> Leader in Parallel Storage
> www.panasas.com
>
>