On 1/22/14, 2:52 PM, John-Mark Gurney wrote:
Alfred Perlstein wrote this message on Wed, Jan 22, 2014 at 14:15 -0800:
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.
isn't it usually apparent which lock it is from the backtrace?
Isn't the backtrace pretty obvious given IP/PC?
-Alfred
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
