Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 Jul 2008 16:30:36 -1000 (HST)
From:      Jeff Roberson <jroberson@jroberson.net>
To:        John Baldwin <jhb@freebsd.org>
Cc:        attilio@freebsd.org, arch@freebsd.org, ivmaykov@gmail.com
Subject:   Re: witness performance improvements
Message-ID:  <20080724162733.B954@desktop>
In-Reply-To: <200807211141.09387.jhb@freebsd.org>
References:  <20080718163231.B954@desktop> <200807211141.09387.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

Thanks,
Jeff

>
> -- 
> John Baldwin
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080724162733.B954>