Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 5 May 2012 13:00:36 +0400
From:      Sergey Kandaurov <pluknet@freebsd.org>
To:        Kirk McKusick <mckusick@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r234386 - in head/sys: fs/coda fs/ext2fs fs/msdosfs fs/nfsclient kern nfsclient sys ufs/ffs ufs/ufs
Message-ID:  <CAE-mSOJmjHL-%2BdLEDeNuhKFKJGMPZa=4wsrjmHw9fUjroVZkOA@mail.gmail.com>
In-Reply-To: <201204171628.q3HGSM9P089821@svn.freebsd.org>
References:  <201204171628.q3HGSM9P089821@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 17 April 2012 20:28, Kirk McKusick <mckusick@freebsd.org> wrote:
> Author: mckusick
> Date: Tue Apr 17 16:28:22 2012
> New Revision: 234386
> URL: http://svn.freebsd.org/changeset/base/234386
>
> Log:
> =A0Replace the MNT_VNODE_FOREACH interface with MNT_VNODE_FOREACH_ALL.
> =A0The primary changes are that the user of the interface no longer
> =A0needs to manage the mount-mutex locking and that the vnode that
> =A0is returned has its mutex locked (thus avoiding the need to check
> =A0to see if its is DOOMED or other possible end of life senarios).
>
> =A0To minimize compatibility issues for third-party developers, the
> =A0old MNT_VNODE_FOREACH interface will remain available so that this
> =A0change can be MFC'ed to 9. Following the MFC to 9, MNT_VNODE_FOREACH
> =A0will be removed in head.
>
> =A0The reason for this update is to prepare for the addition of the
> =A0MNT_VNODE_FOREACH_ACTIVE interface that will loop over just the
> =A0active vnodes associated with a mount point (typically less than
> =A01% of the vnodes associated with the mount point).
>
> =A0Reviewed by: kib
> =A0Tested by: =A0 Peter Holm
> =A0MFC after: =A0 2 weeks
>

Hi.

This commit crashes on old nfsclient. Looks like this change is missed.

Index: nfsclient/nfs_vfsops.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- nfsclient/nfs_vfsops.c      (revision 235051)
+++ nfsclient/nfs_vfsops.c      (working copy)
@@ -1452,6 +1452,7 @@
                MNT_IUNLOCK(mp);
                return (EBADF);
        }
+       MNT_IUNLOCK(mp);

        /*
         * Force stale buffer cache information to be flushed.


[...]
>
> Modified: head/sys/nfsclient/nfs_vfsops.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/nfsclient/nfs_vfsops.c     Tue Apr 17 14:54:00 2012        (=
r234385)
> +++ head/sys/nfsclient/nfs_vfsops.c     Tue Apr 17 16:28:22 2012        (=
r234386)
> @@ -1457,19 +1457,15 @@ nfs_sync(struct mount *mp, int waitfor)
>         * Force stale buffer cache information to be flushed.
>         */
>  loop:
> -       MNT_VNODE_FOREACH(vp, mp, mvp) {
> -               VI_LOCK(vp);
> -               MNT_IUNLOCK(mp);
> +       MNT_VNODE_FOREACH_ALL(vp, mp, mvp) {

Now this call results in malloc() and MNT_ILOCK(mp) inside
__mnt_vnode_first_all().
But MNT_ILOCK(mp) is already done few lines above (probably because
its MNT_IUNLOCK() counterpart was missed in this commit?).


[...]
> +struct vnode *
> +__mnt_vnode_first_all(struct vnode **mvp, struct mount *mp)
> +{
> + =A0 =A0 =A0 struct vnode *vp;
> +

To the moment it already holds MNT_ILOCK(mp) from
sys/nfsclient/nfs_vfsops.c:1445

> + =A0 =A0 =A0 *mvp =3D malloc(sizeof(struct vnode), M_VNODE_MARKER, M_WAI=
TOK | M_ZERO);
> + =A0 =A0 =A0 MNT_ILOCK(mp);
> + =A0 =A0 =A0 MNT_REF(mp);
> + =A0 =A0 =A0 (*mvp)->v_type =3D VMARKER;
> +
> + =A0 =A0 =A0 vp =3D TAILQ_FIRST(&mp->mnt_nvnodelist);
> + =A0 =A0 =A0 while (vp !=3D NULL && (vp->v_type =3D=3D VMARKER ||
> + =A0 =A0 =A0 =A0 =A0 (vp->v_iflag & VI_DOOMED) !=3D 0))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 vp =3D TAILQ_NEXT(vp, v_nmntvnodes);
> +
> + =A0 =A0 =A0 /* Check if we are done */
> + =A0 =A0 =A0 if (vp =3D=3D NULL) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *mvp =3D NULL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNT_REL(mp);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNT_IUNLOCK(mp);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 free(*mvp, M_VNODE_MARKER);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (NULL);
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 (*mvp)->v_mount =3D mp;
> + =A0 =A0 =A0 TAILQ_INSERT_AFTER(&mp->mnt_nvnodelist, vp, *mvp, v_nmntvno=
des);
> + =A0 =A0 =A0 VI_LOCK(vp);
> + =A0 =A0 =A0 MNT_IUNLOCK(mp);
> + =A0 =A0 =A0 return (vp);
> +}
[...]

uma_zalloc_arg: zone "1024" with the following non-sleepable locks held:
exclusive sleep mutex struct mount mtx (struct mount mtx) r =3D 0
(0xfffffe0002907750) locked @ /usr/src/sys/nfsclient/nfs_vfsops.c:1445
KDB: stack backtrace:
db_trace_self_wrapper() at 0xffffffff802c75aa =3D db_trace_self_wrapper+0x2=
a
kdb_backtrace() at 0xffffffff80476547 =3D kdb_backtrace+0x37
_witness_debugger() at 0xffffffff8048d48c =3D _witness_debugger+0x2c
witness_warn() at 0xffffffff8048e274 =3D witness_warn+0x2c4
uma_zalloc_arg() at 0xffffffff8068be24 =3D uma_zalloc_arg+0x384
malloc() at 0xffffffff80425026 =3D malloc+0xc6
__mnt_vnode_first_all() at 0xffffffff804dd3f9 =3D __mnt_vnode_first_all+0x2=
9
nfs_sync() at 0xffffffff805f231d =3D nfs_sync+0x8d
sys_sync() at 0xffffffff804e8906 =3D sys_sync+0x146
amd64_syscall() at 0xffffffff806c780c =3D amd64_syscall+0x38c
Xfast_syscall() at 0xffffffff806b2c47 =3D Xfast_syscall+0xf7
--- syscall (36, FreeBSD ELF64, sys_sync), rip =3D 0x800a95a0c, rsp =3D
0x7fffffffd958, rbp =3D 0x7fffffffdd50 ---
panic: _mtx_lock_sleep: recursed on non-recursive mutex struct mount
mtx @ /usr/src/sys/kern/vfs_subr.c:4595

cpuid =3D 1
KDB: stack backtrace:
db_trace_self_wrapper() at 0xffffffff802c75aa =3D db_trace_self_wrapper+0x2=
a
kdb_backtrace() at 0xffffffff80476547 =3D kdb_backtrace+0x37
panic() at 0xffffffff8043bc3e =3D panic+0x1ce
_mtx_lock_sleep() at 0xffffffff80429058 =3D _mtx_lock_sleep+0x538
_mtx_lock_flags() at 0xffffffff804291e4 =3D _mtx_lock_flags+0x184
__mnt_vnode_first_all() at 0xffffffff804dd413 =3D __mnt_vnode_first_all+0x4=
3
nfs_sync() at 0xffffffff805f231d =3D nfs_sync+0x8d
sys_sync() at 0xffffffff804e8906 =3D sys_sync+0x146
amd64_syscall() at 0xffffffff806c780c =3D amd64_syscall+0x38c
Xfast_syscall() at 0xffffffff806b2c47 =3D Xfast_syscall+0xf7
--- syscall (36, FreeBSD ELF64, sys_sync), rip =3D 0x800a95a0c, rsp =3D
0x7fffffffd958, rbp =3D 0x7fffffffdd50 ---
KDB: enter: panic
[ thread pid 1268 tid 100085 ]
Stopped at      0xffffffff8047620b =3D kdb_enter+0x3b:    movq
$0,0x75d252(%rip)
db>

--=20
wbr,
pluknet



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAE-mSOJmjHL-%2BdLEDeNuhKFKJGMPZa=4wsrjmHw9fUjroVZkOA>