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

[Harald Schmalzbauer]

 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.

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: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20140727/de26d270/attachment.sig>