witness performance improvements

John Baldwin jhb at freebsd.org
Fri Jul 25 16:27:36 UTC 2008


On Thursday 24 July 2008 10:30:36 pm Jeff Roberson wrote:
> On Mon, 21 Jul 2008, John Baldwin wrote:
> 
> > On Friday 18 July 2008 10:41:58 pm Jeff Roberson wrote:
> >> Hello,
> >>
> >> I have a patch that improves witness performance available at:
> >>
> >> http://people.freebsd.org/~jeff/witness.diff
> >>
> >> This improvement comes at the cost of some significant space overhead.  
It
> >> changes the witness graph from a linked tree to a matrix based approach.
> >> Relationships can be quickly resolved with a table lookup.  The table 
size
> >> is WITNESS_COUNT^2, or 1MB with the current count of 1024.
> >
> > Woo!  Thanks for polishing this.
> >
> >> This patch also makes struct witness objects persistent even after the
> >> last lock using this name has been removed.  This is helpful for short
> >> lived objects which may be created frequently.
> >
> > Originally, the idea was that if one had a LOR bug in a driver, one could
> > kldunload the driver and have WITNESS forget about any orders for the
> > driver's lock, fix the bug, and try again, but the short-lived names 
problem
> > is much more common in practice, and trying to remove info about a 
specific
> > lock class from the graph is a bit tenuous, so I think this is the better
> > approach going forward.
> >
> >> To reduce lock contention on SMP witness_checkorder() now runs without 
the
> >> w_mtx when there are no lock violations.  I also cache a lock_list_entry
> >> in each thread as allocating these requires the w_mtx.  The entry is
> >> disposed of at thread_exit().
> >
> > Neat.
> >
> >> I'm mostly interested in hearing what people have to say about the space
> >> bloat.  I believe it is in a commit ready state.
> >
> > I think the space usage is perfectly fine.  Also, now that you malloc the
> > actual witness objects instead of putting them in the BSS (something that
> > should have been done anyway I think), I would make the number of witness
> > objects a loader tunable.
> 
> Well, I'm glad there is a consensus that this is the right way forward. 
> The state of the code is that there may be a bug in the dot output but 
> I've not had any problems with the regular witness operation.
> 
> There are still some style bugs in it.  Attilio has expressed some 
> interest in a full review and style clean-up.  I'd like to get what I have 
> now, minus the dot output, into svn.  And then do a set of follow on 
> commits to add back dot or Attilio's comma separated graph output that can 
> be parsed to dot.
> 
> Any objections to commiting this knowing it has some style bugs and a 
> little work left?  I'd like to get people testing the core functionality 
> more.  We've sat on this patch for a couple of years now as well.

I think you can commit and work on the other stuff afterwards.  I had noticed 
a few style things too but don't want those to hold this up.

-- 
John Baldwin


More information about the freebsd-arch mailing list