On 1/22/14, 1:22 PM, John Baldwin wrote:
On Wednesday, January 22, 2014 3:59:37 pm Alfred Perlstein wrote:
On 1/22/14, 12:27 PM, John Baldwin wrote:
On Wednesday, January 22, 2014 2:06:39 pm Alfred Perlstein wrote:
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.
Where would the memory for the char* array come from?
That is a good question. I suspect it would be up to the subsystem to
allocate it.
Wouldn't it be trivial for *most* of the subsystems to simply have this
either as a static global or static function variable:
static char *mutex_typename = { "kqueue", "foo" };
Under kern I see this:
grep mtx_init * | grep -v NULL
...
kern_rmlock.c: mtx_init(&rm->rm_lock_mtx, name, "rmlock_mtx",
MTX_NOWITNESS);
subr_bus.c: mtx_init(&devsoftc.mtx, "dev mtx", "devd", MTX_DEF);
Those are solved with statics.
Another example:
sys/dev/ae/if_ae.c
mtx_init(&sc->mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
MTX_DEF);
I think the array could be in the softc here? sc->mutex_name_type[0] =
device_get_nameunit(dev); sc->mutex_name_type[1] = MTX_NETWORK_LOCK;
Do we want to do that? It moves "wasting space" to another variable.
I'm not sure where there isn't the possibility of using either static
(for a global mutex) or space inside the equiv of the softc (or proc or
whatever) for this?
I'm not sure this is a good idea, just an idea. Are there places where
it's not as simple as doing this?
To be honest, the whole name vs type thing isn't widely used, and it makes
the mtx_init() function kind of fugly. I think what I would actually prefer
is to just kill it, changing the various places that pass a separate name to
just pass the type instead. Note that none of the other lock APIs even allow
setting a separate type. This would let us remove the static pending list
array as well.
(And yes, I added the name vs type thing, but at this point I think it did
not turn out nearly as useful as I had thought it would be.)
The original issue of picking useful-to-witness lock names (i.e. not just
using device_get_nameunit()) still remains of course.
I really want to agree, but anything that reduces the immediate ability
for people to diagnose problems really makes me worry.
This would mean that you would see "network device lock" or some "type"
but not know the actual owner.
I would say that maybe given this it's just better to grow
WITNESS_PENDING based on maxcpu like the PR I pointed out, that way we
do not introduce churn AND we maintain the debug-ability.
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/185831
-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"
