- For any lock list we hold the head in order to reduce allocation from

  the free list and in this way avoid contention on the w_mtx.
  In order to make the code simple, we rely on the rule that when the head
  has not a child it also doesn't have other subsequent entries.
  Actually this assertion is broken because we can free all the head
  children and quit witness_unlock() with the head still allocated, with no
  children and subsequent entries present.
  Fix this by shifting the head if other entries are present and still
  freeing the object, but leaving always an head.
- Fix witness_thread_has_locks() in order to report, correctly, if the
  lock list linked to a specific thread has children or not based on the
  above explained rule.
- Fix a printout into DDB's "show alllocks" command in order to show,
  correctly, the process name that is really what we want.
- Fix style(9) for a comment.

Tested by:	Giovanni Trematerra <giovanni dot trematerra at gmail dot com>
Reported by:	Marko Kiiskila <marko dot kiiskila at nokia dot com>
Sponsored by:	Nokia

1 files changed, 18 insertions, 5 deletions

diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 15f960e..0e00107 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -1099,6 +1099,7 @@ witness_checkorder(struct lock_object *lock, int flags, const char *file,
 	plock = &(*lock_list)->ll_children[(*lock_list)->ll_count - 1];
 	if (interlock != NULL && plock->li_lock == interlock) {
 		if ((*lock_list)->ll_count == 1) {
+
 			/*
 			 * The interlock is the only lock we hold, so
 			 * nothing to do.
@@ -1523,10 +1524,20 @@ found:
 	intr_restore(s);
 
 	/*
-	 * If this lock list entry is not the first and is now empty, free it.
+	 * In order to reduce contention on w_mtx, we want to keep always an
+	 * head object into lists so that frequent allocation from the 
+	 * free witness pool (and subsequent locking) is avoided.
+	 * In order to maintain the current code simple, when the head
+	 * object is totally unloaded it means also that we do not have
+	 * further objects in the list, so the list ownership needs to be
+	 * hand over to another object if the current head needs to be freed.
 	 */
-	if (*lock_list != lle && (*lock_list)->ll_count == 0) {
-		lle = *lock_list;
+	if ((*lock_list)->ll_count == 0) {
+		if (*lock_list == lle) {
+			if (lle->ll_next == NULL)
+				return;
+		} else
+			lle = *lock_list;
 		*lock_list = lle->ll_next;
 		CTR3(KTR_WITNESS, "%s: pid %d removed lle %p", __func__,
 		    td->td_proc->p_pid, lle);
@@ -2025,7 +2036,9 @@ static int
 witness_thread_has_locks(struct thread *td)
 {
 
-	return (td->td_sleeplocks != NULL);
+	if (td->td_sleeplocks == NULL)
+		return (0);
+	return (td->td_sleeplocks->ll_count != 0);
 }
 
 static int
@@ -2252,7 +2265,7 @@ DB_SHOW_COMMAND(alllocks, db_witness_list_all)
 			if (!witness_thread_has_locks(td))
 				continue;
 			db_printf("Process %d (%s) thread %p (%d)\n", p->p_pid,
-			    td->td_name, td, td->td_tid);
+			    p->p_comm, td, td->td_tid);
 			witness_ddb_list(td);
 		}
 	}