From owner-cvs-all@FreeBSD.ORG Fri Jun 8 15:07:15 2007 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 2B66816A4B3 for ; Fri, 8 Jun 2007 15:07:15 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from ug-out-1314.google.com (ug-out-1314.google.com [66.249.92.175]) by mx1.freebsd.org (Postfix) with ESMTP id D59D913C483 for ; Fri, 8 Jun 2007 15:07:13 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by ug-out-1314.google.com with SMTP id u2so1109486uge for ; Fri, 08 Jun 2007 08:07:12 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=YptTV9ThM7Wu57hPfLELihuhXOkylZ0FaiVbOK2oYnSXxmNFg+LB1SjqVjsAqansKCQIwCYV833kj7SyKxXa3OLHMLO9VVeUWrigSSwNR3goLesgOah1/G1JVZ5uyukkTW4qsL87Ew50u/YflE/25BTLHc1eNwS4L12rrGSfdx4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:reply-to:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=LpMVP4qgrxOuVT2lq7I25/NF6bDspIfUiSNWwHts3pVQojlvgoBq/izmB5MlD0Rfspb7C0LijleOwALsiyYKKNQ2xHzD8is282UkZYQcpoScVSjEu8gDnQJ0Ub5G8+9fIw+r58S1X6llk3qbMm0eXl6dX7gR1xZBgYd50GWT9LM= Received: by 10.82.116.15 with SMTP id o15mr5573416buc.1181315229545; Fri, 08 Jun 2007 08:07:09 -0700 (PDT) Received: from ?172.31.5.25? ( [89.97.252.178]) by mx.google.com with ESMTP id 6sm1022464nfv.2007.06.08.08.07.08 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 08 Jun 2007 08:07:09 -0700 (PDT) Message-ID: <4669708B.4090509@FreeBSD.org> Date: Fri, 08 Jun 2007 17:06:51 +0200 From: Attilio Rao User-Agent: Thunderbird 1.5 (X11/20060526) MIME-Version: 1.0 To: Bruce Evans References: <200706051420.l55EKEih018925@repoman.freebsd.org> <20070607163724.M7517@besplex.bde.org> <20070607180257.P7767@besplex.bde.org> <200706071059.41466.jhb@freebsd.org> <20070609002752.M13082@besplex.bde.org> In-Reply-To: <20070609002752.M13082@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: Attilio Rao Cc: src-committers@freebsd.org, John Baldwin , Kip Macy , cvs-all@freebsd.org, cvs-src@freebsd.org, Kostik Belousov , Jeff Roberson Subject: Re: cvs commit: src/sys/kern kern_mutex.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jun 2007 15:07:15 -0000 Bruce Evans wrote: > 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. No, there can't be this LOR since thread_lock is a spinmutex while vm_page_queue_mtx is a sleepable mutex, so for our well known-rules about locking you cannot have the opposite situation. And if you don't follow John's pattern I think you get a race too since there is a breaking point in the protected path. >>> 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. This is good since mtx_unlock will force a preemption point here. Attilio