Date: Wed, 18 May 2016 07:30:25 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Evans <brde@optusnet.com.au> Cc: Konstantin Belousov <kostikbel@gmail.com>, fs@freebsd.org Subject: Re: fix for per-mount i/o counting in ffs Message-ID: <20160518070252.F6121@besplex.bde.org> In-Reply-To: <20160518061040.D5948@besplex.bde.org> References: <20160517072104.I2137@besplex.bde.org> <20160517084241.GY89104@kib.kiev.ua> <20160518061040.D5948@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 18 May 2016, Bruce Evans wrote:
> On Tue, 17 May 2016, Konstantin Belousov wrote:
>> ...
>> For the accounting patch, don't we want to account for all io, including
>> the mount-time metadata reads and initial superblock update ?
>>
>> diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
>> index 9776554..712fc21 100644
>> --- a/sys/ufs/ffs/ffs_vfsops.c
>> +++ b/sys/ufs/ffs/ffs_vfsops.c
>> @@ -780,6 +780,8 @@ ffs_mountfs(devvp, mp, td)
>> mp->mnt_iosize_max = MAXPHYS;
>>
>> devvp->v_bufobj.bo_ops = &ffs_ops;
>> + if (devvp->v_type == VCHR)
>> + devvp->v_rdev->si_mountpt = mp;
>>
>> fs = NULL;
>> sblockloc = 0;
>> @@ -1049,8 +1051,6 @@ ffs_mountfs(devvp, mp, td)
>> ffs_flushfiles(mp, FORCECLOSE, td);
>> goto out;
>> }
>> - if (devvp->v_type == VCHR && devvp->v_rdev != NULL)
>> - devvp->v_rdev->si_mountpt = mp;
>> if (fs->fs_snapinum[0] != 0)
>> ffs_snapshot_mount(mp);
>> fs->fs_fmod = 1;
>> @@ -1083,6 +1083,8 @@ 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();
>
> Yes, that looks better.
Further cleanups:
- the null pointer check is bogus since we already dereferenced
devvp->v_rdev. We also assigned devvp->v_rdev to the variable
dev but spelled out devvp->v_rdev in a couple of other places.
- the VCHR check is bogus since we only work for VCHR and have
already checked for VCHR in vn_isdisk().
Similarly in ffs_umount() except there is no dev variable there.
Similarly in msdosfs.
NOT similarly in ext2fs. I was looking at the wrong tree again.
Only 1 of my trees has the patch to do this in ext2fs. The patch
for ffs applies almost verbatim.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160518070252.F6121>