Re: dogfooding over in clusteradm land

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:
>> 
>> > This sounds very plausible. I think that there is no sense in restarting
>> > the scan if it is requested in async mode at all. See below.
>> > 
>> > Would be thrilled if this finally solves the svn2cvs issues.
>> > 
>> > commit 41aaafe5e3be5387949f303b8766da64ee4a521f
>> > Author: Kostik Belousov <kostik_at_sirion>
>> > Date:   Tue Jan 3 11:16:30 2012 +0200
>> > 
>> >     Do not restart the scan in vm_object_page_clean() if requested
>> >     mode is async.
>> >     
>> >     Proposed by:	truckman
>> > 
>> > 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
>> > _at__at_ -841,7 +841,8 _at__at_ rescan:
>> >  		if (p->valid == 0)
>> >  			continue;
>> >  		if (vm_page_sleep_if_busy(p, TRUE, "vpcwai")) {
>> > -			if (object->generation != curgeneration)
>> > +			if ((flags & OBJPC_SYNC) != 0 &&
>> > +			    object->generation != curgeneration)
>> >  				goto rescan;
>> >  			np = vm_page_find_least(object, pi);
>> >  			continue;
>> 
>> I wonder if it would make more sense to just skip the busy pages in
>> async mode instead of sleeping ...
>> 
> It would be too much weakening the guarantee of the vfs_msync(MNT_NOWAIT)
> 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.

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.

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.

My suggestion just omits the sleep in this particular case.

The syncer should write the page the next time it runs, unless we're
particularly unlucky ...

> Lets see whether the change alone helps. Do you agree ?

Your patch is definitely worth trying as-is.  My latest suggestion is
probably a minor additional optimization.