witness performance improvements

Attilio Rao attilio at freebsd.org
Fri Jul 25 22:17:41 UTC 2008


2008/7/25, Jeff Roberson <jroberson at jroberson.net>:
> 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.

Please go on and commit the code, delaying any further improvement.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein


More information about the freebsd-arch mailing list