PERFORCE change 100123 for review
Kip Macy
kmacy at FreeBSD.org
Tue Jun 27 07:50:34 UTC 2006
http://perforce.freebsd.org/chv.cgi?CH=100123
Change 100123 by kmacy at kmacy_storage:sun4v_work_sleepq on 2006/06/27 07:49:30
add lock profiling (MUTEX_PROFILING is now a misnomer) to lockmgr
don't call wakeup while we're holding the interlock mutex
I could go into whats wrong with lockmgr:
- priority inversion
- excessive wakeups
- convoluted control flow
- waking up all waiters (including exclusive) on a lock downgrade
but life is too short
I wish I could say that I'm surprised that NFS is one of the top 3 most contended locks
BLAH BLAH BLAH
Affected files ...
.. //depot/projects/kmacy_sun4v/src/sys/kern/kern_lock.c#3 edit
.. //depot/projects/kmacy_sun4v/src/sys/sys/lock_profile.h#8 edit
.. //depot/projects/kmacy_sun4v/src/sys/sys/lockmgr.h#3 edit
Differences ...
==== //depot/projects/kmacy_sun4v/src/sys/kern/kern_lock.c#3 (text+ko) ====
@@ -43,6 +43,7 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: src/sys/kern/kern_lock.c,v 1.96 2005/12/23 21:32:40 jeff Exp $");
+#include "opt_global.h"
#include <sys/param.h>
#include <sys/kdb.h>
#include <sys/kernel.h>
@@ -52,6 +53,7 @@
#include <sys/mutex.h>
#include <sys/proc.h>
#include <sys/systm.h>
+#include <sys/lock_profile.h>
#ifdef DEBUG_LOCKS
#include <sys/stack.h>
#endif
@@ -143,22 +145,21 @@
* accepted shared locks and shared-to-exclusive upgrades to go away.
*/
int
-lockmgr(lkp, flags, interlkp, td)
- struct lock *lkp;
- u_int flags;
- struct mtx *interlkp;
- struct thread *td;
+_lockmgr(struct lock *lkp, u_int flags, struct mtx *interlkp, struct thread *td, char *file, int line)
{
int error;
struct thread *thr;
- int extflags, lockflags;
+ int extflags, lockflags, needwakeup;
+ uint64_t waitstart;
error = 0;
+ needwakeup = 0;
if (td == NULL)
thr = LK_KERNPROC;
else
thr = td;
+ lock_profile_waitstart(&waitstart);
if ((flags & LK_INTERNAL) == 0)
mtx_lock(lkp->lk_interlock);
CTR6(KTR_LOCK,
@@ -210,10 +211,13 @@
lockflags = LK_HAVE_EXCL;
if (td != NULL && !(td->td_pflags & TDP_DEADLKTREAT))
lockflags |= LK_WANT_EXCL | LK_WANT_UPGRADE;
+
error = acquire(&lkp, extflags, lockflags);
if (error)
break;
sharelock(td, lkp, 1);
+ if (lkp->lk_sharecount == 1)
+ lock_profile_obtain_lock_success(&lkp->lk_object, waitstart, file, line);
#if defined(DEBUG_LOCKS)
stack_save(&lkp->lk_stack);
#endif
@@ -224,6 +228,9 @@
* An alternative would be to fail with EDEADLK.
*/
sharelock(td, lkp, 1);
+ if (lkp->lk_sharecount == 1)
+ lock_profile_obtain_lock_success(&lkp->lk_object, waitstart, file, line);
+
/* FALLTHROUGH downgrade */
case LK_DOWNGRADE:
@@ -237,7 +244,7 @@
lkp->lk_flags &= ~LK_HAVE_EXCL;
lkp->lk_lockholder = LK_NOPROC;
if (lkp->lk_waitcount)
- wakeup((void *)lkp);
+ needwakeup = 1;
break;
case LK_EXCLUPGRADE:
@@ -267,6 +274,9 @@
if (lkp->lk_sharecount <= 0)
panic("lockmgr: upgrade without shared");
shareunlock(td, lkp, 1);
+ if (lkp->lk_sharecount == 0)
+ lock_profile_release_lock(&lkp->lk_object);
+
/*
* If we are just polling, check to see if we will block.
*/
@@ -288,7 +298,7 @@
if (error) {
if ((lkp->lk_flags & ( LK_WANT_EXCL | LK_WAIT_NONZERO)) == (LK_WANT_EXCL | LK_WAIT_NONZERO))
- wakeup((void *)lkp);
+ needwakeup = 1;
break;
}
if (lkp->lk_exclusivecount != 0)
@@ -297,6 +307,7 @@
lkp->lk_lockholder = thr;
lkp->lk_exclusivecount = 1;
COUNT(td, 1);
+ lock_profile_obtain_lock_success(&lkp->lk_object, waitstart, file, line);
#if defined(DEBUG_LOCKS)
stack_save(&lkp->lk_stack);
#endif
@@ -309,7 +320,7 @@
*/
if ( (lkp->lk_flags & (LK_SHARE_NONZERO|LK_WAIT_NONZERO)) ==
LK_WAIT_NONZERO)
- wakeup((void *)lkp);
+ needwakeup = 1;
/* FALLTHROUGH exclusive request */
case LK_EXCLUSIVE:
@@ -347,7 +358,7 @@
lkp->lk_flags &= ~LK_WANT_EXCL;
if (error) {
if (lkp->lk_flags & LK_WAIT_NONZERO)
- wakeup((void *)lkp);
+ needwakeup = 1;
break;
}
lkp->lk_flags |= LK_HAVE_EXCL;
@@ -356,6 +367,7 @@
panic("lockmgr: non-zero exclusive count");
lkp->lk_exclusivecount = 1;
COUNT(td, 1);
+ lock_profile_obtain_lock_success(&lkp->lk_object, waitstart, file, line);
#if defined(DEBUG_LOCKS)
stack_save(&lkp->lk_stack);
#endif
@@ -375,19 +387,24 @@
lkp->lk_flags &= ~LK_HAVE_EXCL;
lkp->lk_lockholder = LK_NOPROC;
lkp->lk_exclusivecount = 0;
+ lock_profile_release_lock(&lkp->lk_object);
+ if (lkp->lk_flags & LK_WAIT_NONZERO)
+ needwakeup = 1;
} else {
lkp->lk_exclusivecount--;
}
- } else if (lkp->lk_flags & LK_SHARE_NONZERO)
+ } else if (lkp->lk_flags & LK_SHARE_NONZERO) {
shareunlock(td, lkp, 1);
- else {
+ if (!(lkp->lk_flags & LK_SHARE_NONZERO)) {
+ lock_profile_release_lock(&lkp->lk_object);
+ if (lkp->lk_flags & LK_WAIT_NONZERO)
+ needwakeup = 1;
+ }
+ } else {
printf("lockmgr: thread %p unlocking unheld lock\n",
thr);
kdb_backtrace();
}
-
- if (lkp->lk_flags & LK_WAIT_NONZERO)
- wakeup((void *)lkp);
break;
case LK_DRAIN:
@@ -422,9 +439,11 @@
(lkp->lk_flags & (LK_HAVE_EXCL | LK_WANT_EXCL | LK_WANT_UPGRADE |
LK_SHARE_NONZERO | LK_WAIT_NONZERO)) == 0) {
lkp->lk_flags &= ~LK_WAITDRAIN;
- wakeup((void *)&lkp->lk_flags);
+ needwakeup = 1;
}
mtx_unlock(lkp->lk_interlock);
+ if (needwakeup)
+ wakeup((void *)lkp);
return (error);
}
@@ -504,17 +523,18 @@
#ifdef DEBUG_LOCKS
stack_zero(&lkp->lk_stack);
#endif
+ lock_profile_init(&lkp->lk_object, wmesg);
}
/*
* Destroy a lock.
*/
void
-lockdestroy(lkp)
- struct lock *lkp;
+lockdestroy(struct lock *lkp)
{
CTR2(KTR_LOCK, "lockdestroy(): lkp == %p (lk_wmesg == \"%s\")",
lkp, lkp->lk_wmesg);
+ lock_profile_destroy(&lkp->lk_object);
}
/*
==== //depot/projects/kmacy_sun4v/src/sys/sys/lock_profile.h#8 (text+ko) ====
@@ -50,6 +50,8 @@
u_int hash = 0;
struct lock_profile_object *l = &lo->lo_profile_obj;
+ lo->lo_flags = 0;
+ lo->lo_name = name;
l->lpo_acqtime = 0;
l->lpo_waittime = 0;
l->lpo_filename = NULL;
@@ -73,7 +75,7 @@
{
#if 0
struct lock_profile_object *l = &lo->lo_profile_obj;
- if (m->mtx_object.lo_flags & LO_PROFILE)
+ if (lo->lo_flags & LO_PROFILE)
stack_destroy(l->lpo_stack);
#endif
}
==== //depot/projects/kmacy_sun4v/src/sys/sys/lockmgr.h#3 (text+ko) ====
@@ -40,6 +40,7 @@
#ifdef DEBUG_LOCKS
#include <sys/stack.h> /* XXX */
#endif
+#include <sys/_lock.h>
struct mtx;
@@ -59,6 +60,9 @@
int lk_timo; /* maximum sleep time (for tsleep) */
struct thread *lk_lockholder; /* thread of exclusive lock holder */
struct lock *lk_newlock; /* lock taking over this lock */
+#ifdef MUTEX_PROFILING
+ struct lock_object lk_object;
+#endif
#ifdef DEBUG_LOCKS
struct stack lk_stack;
#endif
@@ -197,11 +201,13 @@
int timo, int flags);
void lockdestroy(struct lock *);
-int lockmgr(struct lock *, u_int flags,
- struct mtx *, struct thread *p);
+int _lockmgr(struct lock *, u_int flags, struct mtx *, struct thread *p, char *file, int line);
void transferlockers(struct lock *, struct lock *);
void lockmgr_printinfo(struct lock *);
int lockstatus(struct lock *, struct thread *);
int lockcount(struct lock *);
+#define lockmgr(lock, flags, mtx, td) _lockmgr((lock), (flags), (mtx), (td), __FILE__, __LINE__)
+
+
#endif /* !_SYS_LOCKMGR_H_ */
More information about the p4-projects
mailing list