From owner-cvs-src@FreeBSD.ORG Fri Jun 8 16:10:55 2007 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id EF6E516A41F; Fri, 8 Jun 2007 16:10:55 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.freebsd.org (Postfix) with ESMTP id 6B2A713C45A; Fri, 8 Jun 2007 16:10:55 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l58GAmLo057733; Fri, 8 Jun 2007 12:10:48 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Bruce Evans Date: Fri, 8 Jun 2007 12:10:23 -0400 User-Agent: KMail/1.9.6 References: <200706051420.l55EKEih018925@repoman.freebsd.org> <200706071059.41466.jhb@freebsd.org> <20070609002752.M13082@besplex.bde.org> In-Reply-To: <20070609002752.M13082@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200706081210.24393.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Fri, 08 Jun 2007 12:10:48 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/3380/Fri Jun 8 08:34:26 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: src-committers@freebsd.org, Kip Macy , cvs-all@freebsd.org, Attilio Rao , cvs-src@freebsd.org, Kostik Belousov , Jeff Roberson Subject: Re: cvs commit: src/sys/kern kern_mutex.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jun 2007 16:10:56 -0000 On Friday 08 June 2007 10:50:15 am Bruce Evans wrote: > On Thu, 7 Jun 2007, John Baldwin wrote: > > > On Thursday 07 June 2007 04:15:13 am Bruce Evans wrote: > >>> The next run will have pagezero resetting its priority when this priority > >>> gets clobbered. > >> > >> That gave only mainly more voluntary context switches (13.5+ million instead > >> of the best observed value of 1.3+ million or the value of 2.9+ million > >> without priority resetting. It reduced the pagezero time from 30 seconds to > >> 24. It didn't change the real time significantly. > > > > Hmm, one problem with the non-preemption page zero is that it doesn't yield > > the lock when it voluntarily yields. I wonder if something like this patch > > would help things for the non-preemption case: > > This works well. It fixed the extra 1.4 million voluntary context switches > and even reduced the number by 10-100k. The real runtime is still up a bit, > but user+sys+pgzero time is down a bit. > > > Index: vm_zeroidle.c > > =================================================================== > > RCS file: /usr/cvs/src/sys/vm/vm_zeroidle.c,v > > retrieving revision 1.45 > > diff -u -r1.45 vm_zeroidle.c > > --- vm_zeroidle.c 18 May 2007 07:10:50 -0000 1.45 > > +++ vm_zeroidle.c 7 Jun 2007 14:56:02 -0000 > > @@ -147,8 +147,10 @@ > > #ifndef PREEMPTION > > if (sched_runnable()) { > > mtx_lock_spin(&sched_lock); > > + mtx_unlock(&vm_page_queue_free_mtx); > > mi_switch(SW_VOL, NULL); > > mtx_unlock_spin(&sched_lock); > > + mtx_lock(&vm_page_queue_free_mtx); > > } > > #endif > > } else { > > The sched_locks are now of course thread_locks. I put the vm unlock > before the thread lock since the above order seems to risk a LOR. That > may have been a mistake -- we would prefer not to be switched after > deciding to do it ourself. Actually, the order is on purpose so that we don't waste time doing a preemption when we're about to switch anyway. > > We could simulate this behavior some by using a critical section to control > > when preemptions happen so that we wait until we drop the lock perhaps to > > allow preemptions. Something like this: > > Simulate? Do you mean simulate the revious pbehaviour? Well, simulate the non-PREEMPTION behavior (with the above patch) in the PREEMPTION case. > I think the voluntary context switch in the above only worked well > because it rarely happened. Apparently, switches away from pagezero > normally happened due to the previous behaviour when the vm lock is > released. > > >> Index: vm_zeroidle.c > > =================================================================== > > RCS file: /usr/cvs/src/sys/vm/vm_zeroidle.c,v > > retrieving revision 1.45 > > diff -u -r1.45 vm_zeroidle.c > > --- vm_zeroidle.c 18 May 2007 07:10:50 -0000 1.45 > > +++ vm_zeroidle.c 7 Jun 2007 14:58:39 -0000 > > @@ -110,8 +110,10 @@ > > if (m != NULL && (m->flags & PG_ZERO) == 0) { > > vm_pageq_remove_nowakeup(m); > > mtx_unlock(&vm_page_queue_free_mtx); > > + critical_exit(); > > pmap_zero_page_idle(m); > > mtx_lock(&vm_page_queue_free_mtx); > > + critical_enter(); > > m->flags |= PG_ZERO; > > vm_pageq_enqueue(PQ_FREE + m->pc, m); > > ++vm_page_zero_count; > > Next I will try this. I put the critical_exit() before the vm unlock. > mtx_unlock() should be allowed to switch if it wants. However, we > would prefer to keep context switches disabled in the above -- just drop > the lock so that other CPUs can proceed. Again, the order here is on purpose to make sure we don't preempt while holding the lock. > > @@ -141,20 +143,25 @@ > > idlezero_enable = idlezero_enable_default; > > > > mtx_lock(&vm_page_queue_free_mtx); > > + critical_enter(); > > for (;;) { > > if (vm_page_zero_check()) { > > vm_page_zero_idle(); > > #ifndef PREEMPTION > > if (sched_runnable()) { > > mtx_lock_spin(&sched_lock); > > + mtx_unlock(&vm_page_queue_free_mtx); > > mi_switch(SW_VOL, NULL); > > mtx_unlock_spin(&sched_lock); > > + mtx_lock(&vm_page_queue_free_mtx); > > } > > #endif > > I added the necessary critical_exit/enter() calls here. They aren't needed in the !PREEMPTION case since the context switch is explicit. The critical sections are only needed for the PREEMPTION case really. > > } else { > > + critical_exit(); > > wakeup_needed = TRUE; > > msleep(&zero_state, &vm_page_queue_free_mtx, 0, > > "pgzero", hz * 300); > > + critical_enter(); > > } > > } > > } > > Bruce > -- John Baldwin