Subject: Re: mp->mnt_vnodelist change
To: Reinoud Zandijk <reinoud@netbsd.org>
From: Simon Burge <simonb@NetBSD.org>
List: tech-kern
Date: 10/20/2006 11:38:28
Reinoud Zandijk wrote:
> One userland program is affected, usr.sbin/pstat. AFAIK it should not break
> any binary compatibilities but and old `pstat -v' will now give `vnode size
> mismatch'.
Not much you can do about that - kmem grovellers will break each time
something like this changes (unless you want to sysctlise pstat, but
pstat will be "fun" to do this since there's lots of per-FS knowledge
in it).
In retrospect with the following suggestions, it would have been easier
to say "try to use TAILQ_FOREACH every time you see a for loop, rather
than list them all out...
> Index: sys/coda/coda_subr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/coda/coda_subr.c,v
> retrieving revision 1.20
> diff -u -r1.20 coda_subr.c
> --- sys/coda/coda_subr.c 12 Oct 2006 01:30:47 -0000 1.20
> +++ sys/coda/coda_subr.c 19 Oct 2006 20:57:12 -0000
> @@ -312,10 +312,10 @@
> struct cnode *cp;
> int count = 0, bad = 0;
> loop:
> - for (vp = mp->mnt_vnodelist.lh_first; vp; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp; vp = nvp) {
Can you use TAILQ_FOREACH here
> Index: sys/fs/msdosfs/msdosfs_vfsops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/fs/msdosfs/msdosfs_vfsops.c,v
> retrieving revision 1.36
> diff -u -r1.36 msdosfs_vfsops.c
> --- sys/fs/msdosfs/msdosfs_vfsops.c 12 Oct 2006 01:32:11 -0000 1.36
> +++ sys/fs/msdosfs/msdosfs_vfsops.c 19 Oct 2006 20:57:12 -0000
> @@ -904,7 +904,7 @@
> */
> simple_lock(&mntvnode_slock);
> loop:
> - for (vp = mp->mnt_vnodelist.lh_first; vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
and here...
> Index: sys/fs/smbfs/smbfs_vfsops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/fs/smbfs/smbfs_vfsops.c,v
> retrieving revision 1.55
> diff -u -r1.55 smbfs_vfsops.c
> --- sys/fs/smbfs/smbfs_vfsops.c 12 Oct 2006 01:32:14 -0000 1.55
> +++ sys/fs/smbfs/smbfs_vfsops.c 19 Oct 2006 20:57:12 -0000
> @@ -434,7 +434,7 @@
> */
> simple_lock(&mntvnode_slock);
> loop:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
and here? I looks like the "vp = nvp;" for each case is the same
as just saying "vp = vp->v_mntvnodes ....".
As they stand, wouldn't those changes above need to change the
nvp = vp->v_mntvnodes.le_next;
line to
nvp = TAILQ_NEXT(vp->v_mntvnodes)
?
> Index: sys/fs/union/union_vfsops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/fs/union/union_vfsops.c,v
> retrieving revision 1.38
> diff -u -r1.38 union_vfsops.c
> --- sys/fs/union/union_vfsops.c 12 Oct 2006 01:32:14 -0000 1.38
> +++ sys/fs/union/union_vfsops.c 19 Oct 2006 20:57:12 -0000
> @@ -350,9 +350,9 @@
> int n;
>
> /* count #vnodes held on mount list */
> - for (n = 0, vp = mp->mnt_vnodelist.lh_first;
> + for (n = 0, vp = TAILQ_FIRST(&mp->mnt_vnodelist);
> vp != NULLVP;
> - vp = vp->v_mntvnodes.le_next)
> + vp = TAILQ_NEXT(vp, v_mntvnodes))
I think this one can be a:
n = 0;
TAILQ_FOREACH( ... )
> Index: sys/kern/vfs_subr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
> retrieving revision 1.271
> diff -u -r1.271 vfs_subr.c
> --- sys/kern/vfs_subr.c 12 Oct 2006 01:32:19 -0000 1.271
> +++ sys/kern/vfs_subr.c 19 Oct 2006 20:57:13 -0000
> @@ -1411,10 +1416,10 @@
>
> simple_lock(&mntvnode_slock);
> loop:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp; vp = nvp) {
> if (vp->v_mount != mp)
> goto loop;
> - nvp = LIST_NEXT(vp, v_mntvnodes);
> + nvp = TAILQ_NEXT(vp, v_mntvnodes);
another TAILQ_FOREACH.
> @@ -2023,7 +2028,7 @@
> savebp = bp;
> again:
> simple_lock(&mntvnode_slock);
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist);
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist);
and again...
> Index: sys/ufs/ext2fs/ext2fs_vfsops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/ext2fs/ext2fs_vfsops.c,v
> retrieving revision 1.102
> diff -u -r1.102 ext2fs_vfsops.c
> --- sys/ufs/ext2fs/ext2fs_vfsops.c 12 Oct 2006 01:32:51 -0000 1.102
> +++ sys/ufs/ext2fs/ext2fs_vfsops.c 19 Oct 2006 20:57:14 -0000
> @@ -541,12 +541,12 @@
>
> loop:
> simple_lock(&mntvnode_slock);
> - for (vp = mountp->mnt_vnodelist.lh_first; vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mountp->mnt_vnodelist); vp != NULL; vp = nvp) {
and again...
> @@ -846,7 +846,7 @@
> */
> simple_lock(&mntvnode_slock);
> loop:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
and again...
> Index: sys/ufs/ffs/ffs_snapshot.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/ffs/ffs_snapshot.c,v
> retrieving revision 1.33
> diff -u -r1.33 ffs_snapshot.c
> --- sys/ufs/ffs/ffs_snapshot.c 12 Oct 2006 01:32:51 -0000 1.33
> +++ sys/ufs/ffs/ffs_snapshot.c 19 Oct 2006 20:57:14 -0000
> @@ -363,14 +363,14 @@
> FSMAXSNAP + 1 /* superblock */ + 1 /* last block */ + 1 /* size */;
> MNT_ILOCK(mp);
> loop:
> - for (xvp = LIST_FIRST(&mp->mnt_vnodelist); xvp; xvp = nvp) {
> + for (xvp = TAILQ_FIRST(&mp->mnt_vnodelist); xvp; xvp = nvp) {
and again...
> Index: sys/ufs/ffs/ffs_vfsops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v
> retrieving revision 1.187
> diff -u -r1.187 ffs_vfsops.c
> --- sys/ufs/ffs/ffs_vfsops.c 12 Oct 2006 01:32:51 -0000 1.187
> +++ sys/ufs/ffs/ffs_vfsops.c 19 Oct 2006 20:57:15 -0000
> @@ -650,12 +650,12 @@
>
> loop:
> simple_lock(&mntvnode_slock);
> - for (vp = mp->mnt_vnodelist.lh_first; vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
and again...
> @@ -1314,7 +1314,7 @@
> */
> simple_lock(&mntvnode_slock);
> loop:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nvp) {
and again (are we detecting a pattern yet? :-)...
> Index: sys/ufs/lfs/lfs_segment.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/lfs/lfs_segment.c,v
> retrieving revision 1.193
> diff -u -r1.193 lfs_segment.c
> --- sys/ufs/lfs/lfs_segment.c 12 Oct 2006 01:32:51 -0000 1.193
> +++ sys/ufs/lfs/lfs_segment.c 19 Oct 2006 20:57:15 -0000
> @@ -494,26 +494,16 @@
> int error = 0;
>
> ASSERT_SEGLOCK(fs);
> -#ifndef LFS_NO_BACKVP_HACK
> - /* BEGIN HACK */
> -#define VN_OFFSET \
> - (((caddr_t)&LIST_NEXT(vp, v_mntvnodes)) - (caddr_t)vp)
> -#define BACK_VP(VP) \
> - ((struct vnode *)(((caddr_t)(VP)->v_mntvnodes.le_prev) - VN_OFFSET))
> -#define BEG_OF_VLIST \
> - ((struct vnode *)(((caddr_t)&LIST_FIRST(&mp->mnt_vnodelist)) \
> - - VN_OFFSET))
> -
> - /* Find last vnode. */
> - loop: for (vp = LIST_FIRST(&mp->mnt_vnodelist);
> - vp && LIST_NEXT(vp, v_mntvnodes) != NULL;
> - vp = LIST_NEXT(vp, v_mntvnodes));
> - for (; vp && vp != BEG_OF_VLIST; vp = nvp) {
> - nvp = BACK_VP(vp);
> +#if 0
> + /* start at last vnode. */
> + loop:
> + for (vp = TAILQ_LAST(&mp->mnt_vnodelist, vnodelst); vp; vp = nvp) {
> + nvp = TAILQ_PREV(vp, vnodelst, v_mntvnodes);
and again...
> #else
> + /* start at oldest accessed vnode */
> loop:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp; vp = nvp) {
> - nvp = LIST_NEXT(vp, v_mntvnodes);
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp; vp = nvp) {
> + nvp = TAILQ_NEXT(vp, v_mntvnodes);
and again...
> Index: sys/ufs/ufs/ufs_quota.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/ufs/ufs_quota.c,v
> retrieving revision 1.41
> diff -u -r1.41 ufs_quota.c
> --- sys/ufs/ufs/ufs_quota.c 23 Jul 2006 22:06:15 -0000 1.41
> +++ sys/ufs/ufs/ufs_quota.c 19 Oct 2006 20:57:15 -0000
> @@ -384,8 +384,8 @@
> * NB: only need to add dquot's for inodes being modified.
> */
> again:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
> - nextvp = LIST_NEXT(vp, v_mntvnodes);
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
> + nextvp = TAILQ_NEXT(vp, v_mntvnodes);
and again...
> @@ -425,8 +425,8 @@
> * deleting any references to quota file being closed.
> */
> again:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
> - nextvp = LIST_NEXT(vp, v_mntvnodes);
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
> + nextvp = TAILQ_NEXT(vp, v_mntvnodes);
and again...
> @@ -595,10 +595,10 @@
> */
> simple_lock(&mntvnode_slock);
> again:
> - for (vp = LIST_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
> + for (vp = TAILQ_FIRST(&mp->mnt_vnodelist); vp != NULL; vp = nextvp) {
and again...
> Index: usr.sbin/pstat/pstat.c
> ===================================================================
> RCS file: /cvsroot/src/usr.sbin/pstat/pstat.c,v
> retrieving revision 1.95
> diff -u -r1.95 pstat.c
> --- usr.sbin/pstat/pstat.c 25 May 2006 01:49:30 -0000 1.95
> +++ usr.sbin/pstat/pstat.c 19 Oct 2006 20:57:16 -0000
> @@ -740,8 +740,8 @@
> for (mp = mountlist.cqh_first;;
> mp = mount.mnt_list.cqe_next) {
> KGET2(mp, &mount, sizeof(mount), "mount entry");
> - for (vp = mount.mnt_vnodelist.lh_first;
> - vp != NULL; vp = vnode.v_mntvnodes.le_next) {
> + for (vp = TAILQ_FIRST(&mount.mnt_vnodelist);
> + vp != NULL; vp = TAILQ_NEXT(&vnode, v_mntvnodes)) {
TAILQ_FOREACH should be OK here too I think.
Simon.