NFS corruption in recent HEAD.

[Rick Macklem]

Pawel Jakub Dawidek wrote:
> On Sat, Nov 26, 2011 at 09:03:46PM -0500, Rick Macklem wrote:
> > Pawel Jakub Dawidek wrote:
> > > Hi.
> > >
> > > I'm booting machine over the network using new NFS client and I'm
> > > getting those warnings on boot:
> > >
> > > /etc/rc.subr: 666: Syntax error: "(" unexpected (expecting ";;")
> [...]
> > Oh, and maybe you could try reverting r227543 in the client
> > (assuming
> > the client is post-r227543). Maybe that file's vnode type isn't set
> > to
> > VREG early in the diskless booting and needs the ncl_flush() for
> > some
> > reason.
> >
> > I don't actually have a bug that needs r227543 to fix it. It just
> > seemed
> > incorrect to flush non-VREG files (particularily VDIR). As such,
> > reverting
> > it wouldn't be a big deal.
> 
> I haven't tried reverting anything yet, but I think I was able to
> reproduce this with old NFS client as well. The problem goes away when
> I
> comment out root mount point from /etc/fstab or remove mntudp from
> mount
> options. NFS root is mounted using TCP, AFAIK and it probably happens
> when startup scripts (rc.d/mountcritremote) remounts root with mntudp
> flag. The rc.subr warning starts to appear just after mountcritremote
> is
> called.
> 
I finally got around to poking at this and found there are 2 bugs in the
NFS clients (the new and old use essentially the same code).

1 - If the rsize/wsize is greater than NFS_MAXDGRAMDATA (which will be the
    case for a default TCP mount) and an I/O RPC is in progress when
    "mount -u -o udp" is done, the thread(s) will get stuck retrying the
    RPC forever, since it tries to do an I/O greater than NFS_MAXDGRAMDATA.
    - The only fix I can see for this is to not allow the switch to UDP for
      this case. I'm not sure if "udp" should be silently ignored or
      "mount -u" should fail with an error. I've asked this question in a
      separate email.
    patch for "fail" version at http://people.freebsd.org/~rmacklem/udpupdate.patch

2 - If the rsize/wsize is changed by a "mount -u", data in the buffer cache
    can be corrupted. I think this is what you saw, above. I think the problem
    that causes this one is that the code above the buffer cache uses mnt_stat.f_iosize
    (which changes when rsize/wsize changes) instead of v_bufobj.bo_bsize,
    which is set to the value of mnt_stat.f_iosize when the vnode is created.
    patch at http://people.freebsd.org/~rmacklem/bobsize.patch

Pawel, I don't know if it's convenient, but maybe you could test the patch for #2?
(If you apply the patch for #1, the mount update will just fail, so you'll never
 exercise #2.)

Also, if someone reading this is conversant with the buffer cache code, maybe they
could review patch for #2?

Thanks, rick
ps: The above patches are for the new NFS client, but essentially the same patches
    could be applied to the old one.