Date: Tue, 27 Jun 2006 07:50:24 GMT From: Kip Macy <kmacy@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 100123 for review Message-ID: <200606270750.k5R7oOOi001903@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=100123 Change 100123 by kmacy@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_ */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606270750.k5R7oOOi001903>