From owner-freebsd-current@FreeBSD.ORG Tue Jan 3 10:57:32 2012 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A1A3D106566C; Tue, 3 Jan 2012 10:57:32 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (gw.catspoiler.org [75.1.14.242]) by mx1.freebsd.org (Postfix) with ESMTP id 658238FC18; Tue, 3 Jan 2012 10:57:31 +0000 (UTC) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.13.3/8.13.3) with ESMTP id q03AvHur007141; Tue, 3 Jan 2012 02:57:21 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <201201031057.q03AvHur007141@gw.catspoiler.org> Date: Tue, 3 Jan 2012 02:57:17 -0800 (PST) From: Don Lewis To: kostikbel@gmail.com In-Reply-To: <20120103102607.GO50300@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii 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 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Jan 2012 10:57:32 -0000 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 >> > 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 >> > @@ -841,7 +841,8 @@ 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.