On 1/22/14, 10:14 AM, John-Mark Gurney wrote:
Scott Long wrote this message on Tue, Jan 21, 2014 at 15:12 -0700:
On Jan 21, 2014, at 9:26 AM, John Baldwin <j...@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 <j...@freebsd.org> wrote:

On Sunday 19 January 2014 18:18:03 Rui Paulo wrote:
On 19 Jan 2014, at 17:59, Neel Natu <n...@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?
Locks have a name and a type.  The type is used for witness, but if it
is NULL, the name is used.  So you could if you wanted, create a common
type, and then put driver/device name in name, but the passed in strings
to both name and type have to be stable storage (only the pointer is
stored), so you can't use a stack variable to construct it.

Hmm, what if locks had a pointer to a 2 element char * array, the first being the name, the second the type. That would keep the size of the lock down and most locks could share a common tuple of name/type in each subsystem. This would allow us to get rid of the pending static list.

effectively:
struct lock_object {
        char *lo_name;          /* Individual lock name. */
        u_int   lo_flags;
        u_int   lo_data;                /* General class specific data. */
        struct  witness *lo_witness;    /* Data for witness. */
};

would change to:
struct lock_object {
char **lo_name_type; /* Individual lock name[0]/type[1]. */
        u_int   lo_flags;
        u_int   lo_data;                /* General class specific data. */
        struct  witness *lo_witness;    /* Data for witness. */
};

This may be somewhat disruptive, I haven't played with how it would actually change driver/etc/code.

-Alfred
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to