fix for per-mount i/o counting in ffs

[Bruce Evans]

PS:

On Fri, 20 May 2016, Bruce Evans wrote:

> On Thu, 19 May 2016, Konstantin Belousov wrote:
>> diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
>> index 712fc21..21425f5 100644
>> --- a/sys/ufs/ffs/ffs_vfsops.c
>> +++ b/sys/ufs/ffs/ffs_vfsops.c
>> @@ -764,24 +764,29 @@ ffs_mountfs(devvp, mp, td)
>> 	cred = td ? td->td_ucred : NOCRED;
>> 	ronly = (mp->mnt_flag & MNT_RDONLY) != 0;
>> 
>> +	KASSERT(devvp->v_type == VCHR, ("reclaimed devvp"));

I still don't like this.  The source code tends to fill up with assertions
(and comments) about simple things.

>> 	dev = devvp->v_rdev;
>> 	dev_ref(dev);
>> +	if (!atomic_cmpset_ptr(&dev->si_mountpt, 0, mp)) {

I used != 0.

>> @@ -1083,8 +1088,6 @@ ffs_mountfs(devvp, mp, td)
>> out:
>> 	if (bp)
>> 		brelse(bp);
>> -	if (devvp->v_type == VCHR && devvp->v_rdev != NULL)
>> -		devvp->v_rdev->si_mountpt = NULL;
>> 	if (cp != NULL) {
>> 		DROP_GIANT();
>> 		g_topology_lock();
>> @@ -1102,6 +1105,7 @@ out:
>> 		free(ump, M_UFSMNT);
>> 		mp->mnt_data = NULL;
>> 	}
>> +	dev->si_mountpt = NULL;
>
> This should remain before the vnode unlock.  Otherwise a new mount can
> fail unnecessarily.
>
>> 	dev_rel(dev);
>> 	return (error);
>> }

Oops.  The vnode lock is not held here, so the order in ffs_mount() cannot
be duplicated.  I moved the resetting of si_mountpt down to here too.

Not locking here and elsewhere makes the locking for v_bufobj even
more obscure.  v_bufobj is a whole struct living in the vnode.  It has
no locking annotation, but has a style bug (a stray '*') where its
locking annotation should be.  g_vfs_open() sets sc->sc_bo to
&vp->v_bufobj and I think most uses of this don't lock the vnode or
check if has been revoked.  Perhaps ro accesses are OK (revoke() must
not clean v_bufobj).  Cleaning v_bufobj on mount failure without the
vnode lock would be a bug.  I think it is just not cleaned or used
until the next mount changes it.

Bruce