From owner-p4-projects@FreeBSD.ORG Tue Jun 27 07:50:34 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 1736516A40D; Tue, 27 Jun 2006 07:50:34 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CCC2516A408 for ; Tue, 27 Jun 2006 07:50:33 +0000 (UTC) (envelope-from kmacy@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6C1C143D79 for ; Tue, 27 Jun 2006 07:50:24 +0000 (GMT) (envelope-from kmacy@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k5R7oOIX001906 for ; Tue, 27 Jun 2006 07:50:24 GMT (envelope-from kmacy@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k5R7oOOi001903 for perforce@freebsd.org; Tue, 27 Jun 2006 07:50:24 GMT (envelope-from kmacy@freebsd.org) Date: Tue, 27 Jun 2006 07:50:24 GMT Message-Id: <200606270750.k5R7oOOi001903@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to kmacy@freebsd.org using -f From: Kip Macy To: Perforce Change Reviews Cc: Subject: PERFORCE change 100123 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Jun 2006 07:50:34 -0000 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 __FBSDID("$FreeBSD: src/sys/kern/kern_lock.c,v 1.96 2005/12/23 21:32:40 jeff Exp $"); +#include "opt_global.h" #include #include #include @@ -52,6 +53,7 @@ #include #include #include +#include #ifdef DEBUG_LOCKS #include #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 /* XXX */ #endif +#include 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_ */