From owner-cvs-src@FreeBSD.ORG Mon Aug 29 15:13:23 2005 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BA85616A422; Mon, 29 Aug 2005 15:13:23 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from mv.twc.weather.com (mv.twc.weather.com [65.212.71.225]) by mx1.FreeBSD.org (Postfix) with ESMTP id 767FC43D58; Mon, 29 Aug 2005 15:13:12 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from [10.50.40.201] (Not Verified[10.50.40.201]) by mv.twc.weather.com with NetIQ MailMarshal (v6, 0, 3, 8) id ; Mon, 29 Aug 2005 11:28:27 -0400 From: John Baldwin To: Don Lewis Date: Mon, 29 Aug 2005 11:13:58 -0400 User-Agent: KMail/1.8 References: <200508250347.j7P3lbux099751@repoman.freebsd.org> In-Reply-To: <200508250347.j7P3lbux099751@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-6" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200508291114.00148.jhb@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern subr_witness.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: Mon, 29 Aug 2005 15:13:24 -0000 On Wednesday 24 August 2005 11:47 pm, Don Lewis wrote: > truckman 2005-08-25 03:47:37 UTC > > FreeBSD src repository > > Modified files: > sys/kern subr_witness.c > Log: > Track all lock relationships instead of pruning direct relationships > if an indirect relationship exists (keep both A->B->C and A->C). > This allows witness_checkorder() to use isitmychild() instead of > the much more expensive isitmydescendant() to check for valid lock > ordering. > > Don't do an expensive tree walk to update the w_level values when > the tree is updated. Only update the w_level values when using the > debugger to display the tree. > > Nuke the experimental "witness_watch > 1" mode that only compared > w_level for the two locks. This information is no longer maintained > at run time, and the use of isitmychild() in witness_checkorder > should bring performance close enough to the acceptable level that > this hack is not needed. > > Report witness data structure allocation statistics under the > debug.witness sysctl. > > Reviewed by: jhb > MFC after: 30 days > > Revision Changes Path > 1.198 +31 -71 src/sys/kern/subr_witness.c I didn't think of this until now, but I think this breaks indirect lock order relationships that aren't explicit. That is, suppose at some point the following lock order pairs are recorded: A -> B C -> D That will give you a tree structure of something like: A --> B --> C If you then do C -> A, since C isn't a direct descendant of A, witness won't catch it anymore. So, you might need to back this out until a solution to this problem is solved. What you changed is the situation where one does A -> B and A -> C first, establishing a tree like this: A --> B \-> C And then does B -> C. The old code moved C under B: A --> B --> C and would still catch C -> A because it checked the full tree. With your changes you would end up with: A --> B --> C \-> C Which is fine except that it only catches reversals of explicit reversals, it doesn't actually handle any indirect lock orders as in my first scenario above. Sorry I didn't think of this until now when you asked for my review earlier. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org