svn commit: r260898 - head/sys/kern

[John Baldwin]

On Tuesday, January 21, 2014 5:12:30 pm Scott Long wrote:
> 
> On Jan 21, 2014, at 9:26 AM, John Baldwin <jhb at freebsd.org> wrote:
> 
> > On Monday, January 20, 2014 5:18:44 pm Alexander Kabaev wrote:
> >> On Mon, 20 Jan 2014 11:32:29 -0500
> >> John Baldwin <jhb at freebsd.org> wrote:
> >> 
> >>> On Sunday 19 January 2014 18:18:03 Rui Paulo wrote:
> >>>> On 19 Jan 2014, at 17:59, Neel Natu <neel at FreeBSD.org> wrote:
> >>>>> Author: neel
> >>>>> Date: Mon Jan 20 01:59:35 2014
> >>>>> New Revision: 260898
> >>>>> URL: http://svnweb.freebsd.org/changeset/base/260898
> >>>>> 
> >>>>> Log:
> >>>>> Bump up WITNESS_COUNT from 1024 to 1536 so there are sufficient
> >>>>> entries for
> >>>>> WITNESS to actually work.
> >>>> 
> >>>> This value should be automatically tuned...
> >>> 
> >>> How do you propose to do so?  This is the count of locks initialized
> >>> before witness' own SYSINIT is executed and the array it sizes is
> >>> allocated statically at compile time.  This used to not be a static
> >>> array, but an intrusive list embedded in locks themselves, but we
> >>> decided to shave a pointer off of each lock that was only used for
> >>> that and to use a statically sized table instead.
> >>> 
> >>> -- 
> >>> John Baldwin
> >> 
> >> As <CONSTANT1> + <CONSTANT2> * MAXCPU, as evidently most recent
> >> overflows reported were caused by jacking MAXCPU up from its default
> >> value? 
> > 
> > If raising MAXCPU changes the number of unique lock names used, then the
> > locks are named incorrectly.  We don't use the 'pid' in the name for
> > PROC_LOCK precisely so that WITNESS will treat them all the same so
> > that if if it learns a lock order for pid 37 it enforces the same lock
> > order for pid 38.  Device locks should follow a similar rule.  They
> > should generally not include the device name (and in some cases they
> > really shouldn't even have the driver name).
> 
> Why shouldn’t they have a driver and device name?  Wouldn’t it help identify
> possible deadlocks from driver instances calling into each other?

It prevents that.  Let's say you have twe0 and twe1 and you use 'twe0 I/O'
and 'twe1 I/O' for the locks.  If you lock 'twe1 I/O' after 'twe0 I/O'
WITNESS will happily just create a new known lock order and not complain.
It will only complain if later during the same uptime you later acquire
'twe0 I/O' after 'twe1 I/O'.  If instead you name the lock 'twe I/O', then
the first time you try to acquire a second 'twe I/O' lock, WITNESS will
complain (and in general drivers shouldn't be calling into each other, so
we want them to complain the first time).

This is why we have the MTX_NETWORK_LOCK hack for driver transmit ring
locks, so that WITNESS uses a single set of lock order relationships
between transmit ring locks and other locks.

-- 
John Baldwin