panic/lock on 9.3-RELEASE with nullfs/nfs/zfs combination

[Konstantin Belousov]

On Sun, Jul 27, 2014 at 08:24:13PM +0200, Harald Schmalzbauer wrote:
>  Bez?glich Konstantin Belousov's Nachricht vom 27.07.2014 19:47
> (localtime):
> > On Sun, Jul 27, 2014 at 04:44:42PM +0200, Harald Schmalzbauer wrote:
> >>  Bez??glich Konstantin Belousov's Nachricht vom 26.07.2014 13:48
> >> (localtime):
> >> I can't emphasize how much I hate my lousy C skills... Got "format '%#x'
> >> expects type 'unsigned int', but argument 3 has type 'u_long'
> >> [-Wformat]" from the compiler and found out that I can't read/understand
> >> any part of your patch; not even the syntax :-(
> >>
> >> After reading printf(3) I change the 3rd additional line into
> >> '("ldvp %p fl %#lx dvp %p fl %#lx flags %#x", ldvp, ldvp->v_vflag,'
> >> ??? A blind man may sometimes hit the mark.
> > This is because the patch was against HEAD, where I changed type of flags
> > to not waste space.  It was not merged to stable/9 to keep KBI.
> >
> >> Here's the panic:
> >> panic: ldvp 0xfffffe001bafb000 fl 0x1 dvp 0xfffffe01b4e5a9d8 fl 0 flags
> >> 0x120e144
> >> vnode vnode 0xfffffe014bc03dc8: 0xfffffe014bc03dc8: tag null, type VDIR
> >> tag null, type VDIR
> >> usecount 1, writecount 0, refcount 1 mountedhere 0
> >> usecount 1, writecount 0, refcount 1 mountedhere 0
> >> flags (VV_ROOT|VI_ACTIVE)
> >> flags (VV_ROOT|VI_ACTIVE)
> >> v_object 0xfffffe014bbf4828 ref 0 pages 0 cleanbuf 0 dirtybuf 0
> >> v_object 0xfffffe014bbf4828 ref 0 pages 0 cleanbuf 0 dirtybuf 0
> >> lock type zfs: SHARED (count 1)
> >> lock type zfs: SHARED (count 1)
> >> vp=0xfffffe014bc03dc8, lowervp=0xfffffe001bafb000
> >> vp=0xfffffe014bc03dc8, lowervp=0xfffffe001bafb000
> > So the root vnode for underlying ZFS filesystem somehow managed to get
> > two different upper vnodes for nullfs mount.  My guess is that the real
> > problem is due to inconsistent initialization of v_hash for zfs vnodes.
> > It only get set in zfs_vget(), which causes root vnode to have zero
> > hash initially.
> >
> > Please, keep the KASSERT() patch applied, revert the VV_ROOT patch for
> > nullfs, and apply the following change.  Then retest.
> >
> > diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
> > index 04160c6..33748fe 100644
> > --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
> > +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
> > @@ -2075,8 +2075,6 @@ zfs_vget(vfs_t *vfsp, ino_t ino, int flags, vnode_t **vpp)
> >  		err = vn_lock(*vpp, flags);
> >  	if (err != 0)
> >  		*vpp = NULL;
> > -	else
> > -		(*vpp)->v_hash = ino;
> >  	return (err);
> >  }
> >  
> > diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
> > index ac55996..c8f86b8 100644
> > --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
> > +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
> > @@ -1228,9 +1228,10 @@ again:
> >  		vnode_t *vp = ZTOV(zp);
> >  
> >  		err = insmntque(vp, zfsvfs->z_vfs);
> > -		if (err == 0)
> > +		if (err == 0) {
> > +			vp->v_hash = obj_num;
> >  			VOP_UNLOCK(vp, 0);
> > -		else {
> > +		} else {
> >  			zp->z_vnode = NULL;
> >  			zfs_znode_dmu_fini(zp);
> >  			zfs_znode_free(zp);
> 
> Applyed these two patches along with the KASSERT patch (reverted
> ROOT_VV) and did the same simple test: No panic, no debug message. So
> this also fixes my limited and simple test case.
Great, I believe that zfs patch fixes the root cause of the problem.
Thank you for the testing.

> 
> Tell me if I can provide anything else, I'll keep that latest patched
> version running, tomorrow will be regular usage and I'll report again.
> 
> Thanks,
> 
> -Harry
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20140727/bf6b7cc2/attachment.sig>