Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 3 Jan 2012 13:16:18 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Don Lewis <truckman@freebsd.org>
Cc:        attilio@freebsd.org, flo@freebsd.org, current@freebsd.org, mckusick@mckusick.com, phk@phk.freebsd.dk, seanbru@yahoo-inc.com
Subject:   Re: dogfooding over in clusteradm land
Message-ID:  <20120103111618.GP50300@deviant.kiev.zoral.com.ua>
In-Reply-To: <201201031057.q03AvHur007141@gw.catspoiler.org>
References:  <20120103102607.GO50300@deviant.kiev.zoral.com.ua> <201201031057.q03AvHur007141@gw.catspoiler.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--GGOD4F4C0+BqRx9L
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 03, 2012 at 02:57:17AM -0800, Don Lewis wrote:
> On  3 Jan, Kostik Belousov wrote:
> > On Tue, Jan 03, 2012 at 01:45:26AM -0800, Don Lewis wrote:
> >> On  3 Jan, Kostik Belousov wrote:
> >>=20
> >> > This sounds very plausible. I think that there is no sense in restar=
ting
> >> > the scan if it is requested in async mode at all. See below.
> >> >=20
> >> > Would be thrilled if this finally solves the svn2cvs issues.
> >> >=20
> >> > commit 41aaafe5e3be5387949f303b8766da64ee4a521f
> >> > Author: Kostik Belousov <kostik@sirion>
> >> > Date:   Tue Jan 3 11:16:30 2012 +0200
> >> >=20
> >> >     Do not restart the scan in vm_object_page_clean() if requested
> >> >     mode is async.
> >> >    =20
> >> >     Proposed by:	truckman
> >> >=20
> >> > diff --git a/sys/vm/vm_object.c b/sys/vm/vm_object.c
> >> > index 716916f..52fc08b 100644
> >> > --- a/sys/vm/vm_object.c
> >> > +++ b/sys/vm/vm_object.c
> >> > @@ -841,7 +841,8 @@ rescan:
> >> >  		if (p->valid =3D=3D 0)
> >> >  			continue;
> >> >  		if (vm_page_sleep_if_busy(p, TRUE, "vpcwai")) {
> >> > -			if (object->generation !=3D curgeneration)
> >> > +			if ((flags & OBJPC_SYNC) !=3D 0 &&
> >> > +			    object->generation !=3D curgeneration)
> >> >  				goto rescan;
> >> >  			np =3D vm_page_find_least(object, pi);
> >> >  			continue;
> >>=20
> >> I wonder if it would make more sense to just skip the busy pages in
> >> async mode instead of sleeping ...
> >>=20
> > It would be too much weakening the guarantee of the vfs_msync(MNT_NOWAI=
T)
> > to not write such pages, IMO. Busy state indeed means that the page most
> > likely undergoing the i/o, but in case it is not, we would not write it
> > at all.
>=20
> If the original code detects a busy page, it sleeps and then continues
> with the next page if generation hasn't changed.  If generation has
> changed, then it restarts the scan.
>=20
> With your change above, the code will skip the busy page after sleeping
> if it is running in async mode.  It won't make another attempt to write
> this page because it no longer attempts to rescan.
Why would it skip it ? Please note the call to vm_page_find_least()
with the pindex of the busy page right after the check for
generation. If a page with the pindex is still present in the object,
vm_page_find_least() should return it, and vm_object_page_clean() should
make another attempt at processing it.

Am I missing something ?
>=20
> My suggestion just omits the sleep in this particular case.
>=20
> The syncer should write the page the next time it runs, unless we're
> particularly unlucky ...
>=20
> > Lets see whether the change alone helps. Do you agree ?
>=20
> Your patch is definitely worth trying as-is.  My latest suggestion is
> probably a minor additional optimization.

--GGOD4F4C0+BqRx9L
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)

iEYEARECAAYFAk8C44IACgkQC3+MBN1Mb4i75wCfbri2O2weq/4g56l4Ax4aFfuF
kAIAn0tUmudAn/xURv02cu/wPxeLLnw0
=q8Ep
-----END PGP SIGNATURE-----

--GGOD4F4C0+BqRx9L--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120103111618.GP50300>