svn commit: r234386 - in head/sys: fs/coda fs/ext2fs fs/msdosfs fs/nfsclient kern nfsclient sys ufs/ffs ufs/ufs

[Sergey Kandaurov]

On 17 April 2012 20:28, Kirk McKusick <mckusick at 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:
>  Replace the MNT_VNODE_FOREACH interface with MNT_VNODE_FOREACH_ALL.
>  The primary changes are that the user of the interface no longer
>  needs to manage the mount-mutex locking and that the vnode that
>  is returned has its mutex locked (thus avoiding the need to check
>  to see if its is DOOMED or other possible end of life senarios).
>
>  To minimize compatibility issues for third-party developers, the
>  old MNT_VNODE_FOREACH interface will remain available so that this
>  change can be MFC'ed to 9. Following the MFC to 9, MNT_VNODE_FOREACH
>  will be removed in head.
>
>  The reason for this update is to prepare for the addition of the
>  MNT_VNODE_FOREACH_ACTIVE interface that will loop over just the
>  active vnodes associated with a mount point (typically less than
>  1% of the vnodes associated with the mount point).
>
>  Reviewed by: kib
>  Tested by:   Peter Holm
>  MFC after:   2 weeks
>

Hi.

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

Index: nfsclient/nfs_vfsops.c
===================================================================
--- 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
> ==============================================================================
> --- 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)
> +{
> +       struct vnode *vp;
> +

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

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

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

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

-- 
wbr,
pluknet