Without this diff my regress machines became so unstable, that they
often do not finish the test run.
With this diff, they run fine. No regressions.
Any ok?
bluhm
On Sat, Jan 04, 2020 at 10:55:46PM +0100, Alexander Bluhm wrote:
> On my amd64 test machine regress/sys/kern/mount run-regress-unmount-busy
> triggers the dangling vnode panic regulary. Something has changed
> in the previous months that makes it more likely.
>
> Problem is that while flushing the mnt_vnodelist list, the unmount
> process sleeps. Then active processes can write new dirty vnodes
> to the list. This happens without softdep.
>
> My first attempt was to disable insertions while unmounting. That
> does not work. A buffer flush can cause softdep to add new dirty
> vnodes to the list.
>
> This diff adds the dirty vnodes to the end of the list. vflush()
> trverses it from the begining. Then new vnodes, that are added
> during syncing, will be flushed. This fixes my test.
>
> Please test with and without softdep.
>
> ok?
>
> bluhm
>
> Index: sys/kern/vfs_subr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_subr.c,v
> retrieving revision 1.297
> diff -u -p -r1.297 vfs_subr.c
> --- sys/kern/vfs_subr.c 30 Dec 2019 13:49:40 -0000 1.297
> +++ sys/kern/vfs_subr.c 4 Jan 2020 18:42:25 -0000
> @@ -178,7 +178,7 @@ vfs_mount_alloc(struct vnode *vp, struct
> rw_init_flags(&mp->mnt_lock, "vfslock", RWL_IS_VNODE);
> (void)vfs_busy(mp, VB_READ|VB_NOWAIT);
>
> - LIST_INIT(&mp->mnt_vnodelist);
> + TAILQ_INIT(&mp->mnt_vnodelist);
> mp->mnt_vnodecovered = vp;
>
> atomic_inc_int(&vfsp->vfc_refcount);
> @@ -476,12 +476,12 @@ insmntque(struct vnode *vp, struct mount
> * Delete from old mount point vnode list, if on one.
> */
> if (vp->v_mount != NULL)
> - LIST_REMOVE(vp, v_mntvnodes);
> + TAILQ_REMOVE(&vp->v_mount->mnt_vnodelist, vp, v_mntvnodes);
> /*
> * Insert into list of vnodes for the new mount point, if available.
> */
> if ((vp->v_mount = mp) != NULL)
> - LIST_INSERT_HEAD(&mp->mnt_vnodelist, vp, v_mntvnodes);
> + TAILQ_INSERT_TAIL(&mp->mnt_vnodelist, vp, v_mntvnodes);
> }
>
> /*
> @@ -872,7 +872,7 @@ vfs_mount_foreach_vnode(struct mount *mp
> int error = 0;
>
> loop:
> - LIST_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> + TAILQ_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> if (vp->v_mount != mp)
> goto loop;
>
> @@ -1299,7 +1299,7 @@ printlockedvnodes(void)
> TAILQ_FOREACH(mp, &mountlist, mnt_list) {
> if (vfs_busy(mp, VB_READ|VB_NOWAIT))
> continue;
> - LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> if (VOP_ISLOCKED(vp))
> vprint(NULL, vp);
> }
> @@ -2287,7 +2287,7 @@ vfs_mount_print(struct mount *mp, int fu
> (*pr)("locked vnodes:");
> /* XXX would take mountlist lock, except ddb has no context */
> cnt = 0;
> - LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> if (VOP_ISLOCKED(vp)) {
> if (cnt == 0)
> (*pr)("\n %p", vp);
> @@ -2304,7 +2304,7 @@ vfs_mount_print(struct mount *mp, int fu
> (*pr)("all vnodes:");
> /* XXX would take mountlist lock, except ddb has no context */
> cnt = 0;
> - LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> if (cnt == 0)
> (*pr)("\n %p", vp);
> else if ((cnt % (72 / (sizeof(void *) * 2 + 4))) == 0)
> Index: sys/kern/vfs_syscalls.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.338
> diff -u -p -r1.338 vfs_syscalls.c
> --- sys/kern/vfs_syscalls.c 29 Nov 2019 20:58:17 -0000 1.338
> +++ sys/kern/vfs_syscalls.c 4 Jan 2020 18:41:20 -0000
> @@ -489,7 +489,7 @@ dounmount_leaf(struct mount *mp, int fla
> * Before calling file system unmount, make sure
> * all unveils to vnodes in here are dropped.
> */
> - LIST_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> + TAILQ_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> unveil_removevnode(vp);
> }
>
> @@ -511,7 +511,7 @@ dounmount_leaf(struct mount *mp, int fla
> vrele(coveredvp);
> }
>
> - if (!LIST_EMPTY(&mp->mnt_vnodelist))
> + if (!TAILQ_EMPTY(&mp->mnt_vnodelist))
> panic("unmount: dangling vnode");
>
> vfs_unbusy(mp);
> Index: sys/nfs/nfs_subs.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v
> retrieving revision 1.140
> diff -u -p -r1.140 nfs_subs.c
> --- sys/nfs/nfs_subs.c 25 Dec 2019 12:31:12 -0000 1.140
> +++ sys/nfs/nfs_subs.c 4 Jan 2020 18:41:20 -0000
> @@ -1515,7 +1515,7 @@ nfs_clearcommit(struct mount *mp)
>
> s = splbio();
> loop:
> - LIST_FOREACH_SAFE(vp, &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> + TAILQ_FOREACH_SAFE(vp, &mp->mnt_vnodelist, v_mntvnodes, nvp) {
> if (vp->v_mount != mp) /* Paranoia */
> goto loop;
> LIST_FOREACH_SAFE(bp, &vp->v_dirtyblkhd, b_vnbufs, nbp) {
> Index: sys/nfs/nfs_vfsops.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_vfsops.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 nfs_vfsops.c
> --- sys/nfs/nfs_vfsops.c 26 Dec 2019 13:28:49 -0000 1.123
> +++ sys/nfs/nfs_vfsops.c 4 Jan 2020 18:41:20 -0000
> @@ -805,7 +805,7 @@ nfs_sync(struct mount *mp, int waitfor,
> * Force stale buffer cache information to be flushed.
> */
> loop:
> - LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
> /*
> * If the vnode that we are about to sync is no longer
> * associated with this mount point, start over.
> Index: sys/sys/mount.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mount.h,v
> retrieving revision 1.145
> diff -u -p -r1.145 mount.h
> --- sys/sys/mount.h 26 Dec 2019 13:30:54 -0000 1.145
> +++ sys/sys/mount.h 4 Jan 2020 18:56:00 -0000
> @@ -331,8 +331,6 @@ struct statfs {
> * array of operations and an instance record. The file systems are
> * put on a doubly linked list.
> */
> -LIST_HEAD(vnodelst, vnode);
> -
> struct mount {
> TAILQ_ENTRY(mount) mnt_list; /* mount list */
> SLIST_ENTRY(mount) mnt_dounmount; /* unmount work queue */
> @@ -340,7 +338,7 @@ struct mount {
> struct vfsconf *mnt_vfc; /* configuration info */
> struct vnode *mnt_vnodecovered; /* vnode we mounted on */
> struct vnode *mnt_syncer; /* syncer vnode */
> - struct vnodelst mnt_vnodelist; /* list of vnodes this mount */
> + TAILQ_HEAD(, vnode) mnt_vnodelist; /* list of vnodes this mount */
> struct rwlock mnt_lock; /* mount structure lock */
> int mnt_flag; /* flags */
> struct statfs mnt_stat; /* cache of filesystem stats */
> Index: sys/sys/vnode.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/vnode.h,v
> retrieving revision 1.153
> diff -u -p -r1.153 vnode.h
> --- sys/sys/vnode.h 26 Aug 2019 18:56:29 -0000 1.153
> +++ sys/sys/vnode.h 4 Jan 2020 18:41:20 -0000
> @@ -104,7 +104,7 @@ struct vnode {
> u_int v_inflight;
> struct mount *v_mount; /* ptr to vfs we are in */
> TAILQ_ENTRY(vnode) v_freelist; /* vnode freelist */
> - LIST_ENTRY(vnode) v_mntvnodes; /* vnodes for mount point */
> + TAILQ_ENTRY(vnode) v_mntvnodes; /* vnodes for mount point */
> struct buf_rb_bufs v_bufs_tree; /* lookup of all bufs */
> struct buflists v_cleanblkhd; /* clean blocklist head */
> struct buflists v_dirtyblkhd; /* dirty blocklist head */
> Index: usr.sbin/pstat/pstat.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/pstat/pstat.c,v
> retrieving revision 1.121
> diff -u -p -r1.121 pstat.c
> --- usr.sbin/pstat/pstat.c 5 Feb 2019 02:17:32 -0000 1.121
> +++ usr.sbin/pstat/pstat.c 4 Jan 2020 18:59:03 -0000
> @@ -855,8 +855,7 @@ kinfo_vnodes(void)
> for (mp = TAILQ_FIRST(&kvm_mountlist); mp != NULL;
> mp = TAILQ_NEXT(&mount, mnt_list)) {
> KGETRET(mp, &mount, sizeof(mount), "mount entry");
> - for (vp = LIST_FIRST(&mount.mnt_vnodelist);
> - vp != NULL; vp = LIST_NEXT(&vnode, v_mntvnodes)) {
> + TAILQ_FOREACH(vp, &mount.mnt_vnodelist, v_mntvnodes) {
> KGETRET(vp, &vnode, sizeof(vnode), "vnode");
> if ((bp + sizeof(struct vnode *) +
> sizeof(struct vnode)) > evbuf)
>
