Re: nfs mp vnode list remove safe
On Tue, Jan 14, 2020 at 01:32:50PM -0500, Ted Unangst wrote: > Alexander Bluhm wrote: > > loop: > > - TAILQ_FOREACH_SAFE(vp, &mp->mnt_vnodelist, v_mntvnodes, nvp) { > > + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) { > > if (vp->v_mount != mp) /* Paranoia */ > > goto loop; > > that looks like an infinite loop? if there's a bad vnode on the list, it'll > still be there next time around. Yes, that is stupid. We have it in multiple loops. Should be a kassert. Here it was added at Sat May 5 17:07:46 1990 UTC: https://svnweb.freebsd.org/csrg/sys/kern/vfs_subr.c?r1=41420&r2=41421&; That was the oldest commit I have found. From there it spreaded over the tree. Have to check whether a kassert fits everywhere. bluhm
Re: nfs mp vnode list remove safe
Alexander Bluhm wrote: > Hi, > > There is no remove and no sleep in this loop. So _SAFE is unnecessary. > > ok? sure, with one bonus note. > > bluhm > > Index: nfs/nfs_subs.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v > retrieving revision 1.141 > diff -u -p -r1.141 nfs_subs.c > --- nfs/nfs_subs.c10 Jan 2020 10:33:35 - 1.141 > +++ nfs/nfs_subs.c13 Jan 2020 18:02:54 - > @@ -1509,16 +1509,16 @@ netaddr_match(int family, union nethosta > void > nfs_clearcommit(struct mount *mp) > { > - struct vnode *vp, *nvp; > - struct buf *bp, *nbp; > + struct vnode *vp; > + struct buf *bp; > int s; > > s = splbio(); > loop: > - TAILQ_FOREACH_SAFE(vp, &mp->mnt_vnodelist, v_mntvnodes, nvp) { > + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) { > if (vp->v_mount != mp) /* Paranoia */ > goto loop; that looks like an infinite loop? if there's a bad vnode on the list, it'll still be there next time around.
nfs mp vnode list remove safe
Hi, There is no remove and no sleep in this loop. So _SAFE is unnecessary. ok? bluhm Index: nfs/nfs_subs.c === RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v retrieving revision 1.141 diff -u -p -r1.141 nfs_subs.c --- nfs/nfs_subs.c 10 Jan 2020 10:33:35 - 1.141 +++ nfs/nfs_subs.c 13 Jan 2020 18:02:54 - @@ -1509,16 +1509,16 @@ netaddr_match(int family, union nethosta void nfs_clearcommit(struct mount *mp) { - struct vnode *vp, *nvp; - struct buf *bp, *nbp; + struct vnode *vp; + struct buf *bp; int s; s = splbio(); loop: - TAILQ_FOREACH_SAFE(vp, &mp->mnt_vnodelist, v_mntvnodes, nvp) { + TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) { if (vp->v_mount != mp) /* Paranoia */ goto loop; - LIST_FOREACH_SAFE(bp, &vp->v_dirtyblkhd, b_vnbufs, nbp) { + LIST_FOREACH(bp, &vp->v_dirtyblkhd, b_vnbufs) { if ((bp->b_flags & (B_BUSY | B_DELWRI | B_NEEDCOMMIT)) == (B_DELWRI | B_NEEDCOMMIT)) bp->b_flags &= ~B_NEEDCOMMIT;