Date: Mon, 29 Aug 2005 11:13:58 -0400 From: John Baldwin <jhb@FreeBSD.org> To: Don Lewis <truckman@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 Message-ID: <200508291114.00148.jhb@FreeBSD.org> In-Reply-To: <200508250347.j7P3lbux099751@repoman.freebsd.org> References: <200508250347.j7P3lbux099751@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508291114.00148.jhb>