tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: New diagnostic routine - mutex_ownable()



On Mon, 1 May 2017, Paul Goyette wrote:

even when using expensive checks, it's best not to make them
more expensive than is really necessary.

I understand and agree.

From my reading of sys/lockdebug.h, kern/subr_lockdebug.c and kern/kern_mutex.c it would appear that mutex_ownable() can become

	int
	mutex_ownable(kmutex_t mtx)
	{

		#ifdef DEBUG
		MUTEX_WANTLOCK(mtx)
		#endif
		return 1;
	}

Taylor pointed out privately that this won't quite work, since lockdebug_wantlock() currently updates ld->ld_{ex,sh}want counters.

The attached diffs get around this by making the "shared" argument to lockdebug_wantlock() have three values instead of two. A positive value now refers to shwant, a zero value to exwant, and a negative value means don't touch either counter! The new MUTEX_TESTLOCK() macro uses -1 for the shared argument.

Diffs attached (including man-page and sets-lists updates) - please let me know if this is acceptable.



+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+
Index: share/man/man9/Makefile
===================================================================
RCS file: /cvsroot/src/share/man/man9/Makefile,v
retrieving revision 1.409
retrieving revision 1.409.2.3
diff -u -p -r1.409 -r1.409.2.3
--- share/man/man9/Makefile	15 Apr 2017 13:52:51 -0000	1.409
+++ share/man/man9/Makefile	30 Apr 2017 04:56:55 -0000	1.409.2.3
@@ -1,4 +1,4 @@
-#       $NetBSD: Makefile,v 1.409 2017/04/15 13:52:51 kamil Exp $
+#       $NetBSD: Makefile,v 1.409.2.3 2017/04/30 04:56:55 pgoyette Exp $
 
 #	Makefile for section 9 (kernel function and variable) manual pages.
 
@@ -32,7 +32,7 @@ MAN=	accept_filter.9 accf_data.9 accf_ht
 	kcpuset.9 kernhist.9 klua_lock.9 klua_mod_register.9 kmem.9 kpause.9 \
 	kfilter_register.9 knote.9 \
 	kprintf.9 kthread.9 linedisc.9 lock.9 locking.9 log.9 ltsleep.9 \
-	LWP_CACHE_CREDS.9 \
+	localcount.9 LWP_CACHE_CREDS.9 \
 	makeiplcookie.9 \
 	malloc.9 mb.9 mbuf.9 mca.9 memcmp.9 memcpy.9 memoryallocators.9 \
 	memmove.9 memset.9 \
@@ -119,6 +119,7 @@ MLINKS+=autoconf.9 config_search_loc.9 \
 	autoconf.9 config_attach_pseudo.9 \
 	autoconf.9 config_detach.9 \
 	autoconf.9 config_detach_children.9 \
+	autoconf.9 config_detach_release.9 \
 	autoconf.9 config_deactivate.9 \
 	autoconf.9 config_defer.9 \
 	autoconf.9 config_interrupts.9 \
@@ -515,6 +516,7 @@ MLINKS+=module.9 module_autoload.9 \
 MLINKS+=mstohz.9 hztoms.9
 MLINKS+=mutex.9 mutex_init.9 mutex.9 mutex_destroy.9 mutex.9 mutex_enter.9 \
 	mutex.9 mutex_exit.9 mutex.9 mutex_tryenter.9 mutex.9 mutex_owned.9 \
+	mutex.9 mutex_ownable.9 \
 	mutex.9 mutex_spin_enter.9 mutex.9 mutex_spin_exit.9
 MLINKS+=m_tag.9 m_tag_copy.9 \
 	m_tag.9 m_tag_copy_chain.9 \
Index: share/man/man9/mutex.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/mutex.9,v
retrieving revision 1.26
diff -u -p -r1.26 mutex.9
--- share/man/man9/mutex.9	4 Sep 2013 10:17:58 -0000	1.26
+++ share/man/man9/mutex.9	1 May 2017 10:53:44 -0000
@@ -1,4 +1,4 @@
-.\"	$NetBSD: mutex.9,v 1.26 2013/09/04 10:17:58 skrll Exp $
+.\"	$NetBSD: mutex.9,v 1.26.16.1 2017/04/30 04:56:55 pgoyette Exp $
 .\"
 .\" Copyright (c) 2007, 2009 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -36,6 +36,7 @@
 .Nm mutex_destroy ,
 .Nm mutex_enter ,
 .Nm mutex_exit ,
+.Nm mutex_ownable ,
 .Nm mutex_owned ,
 .Nm mutex_spin_enter ,
 .Nm mutex_spin_exit ,
@@ -52,6 +53,8 @@
 .Ft void
 .Fn mutex_exit "kmutex_t *mtx"
 .Ft int
+.Fn mutex_ownable "kmutex_t *mtx"
+.Ft int
 .Fn mutex_owned "kmutex_t *mtx"
 .Ft void
 .Fn mutex_spin_enter "kmutex_t *mtx"
@@ -172,6 +175,22 @@ if it is not already equal or higher.
 Release a mutex.
 The mutex must have been previously acquired by the caller.
 Mutexes may be released out of order as needed.
+.It Fn mutex_ownable "mtx"
+.Pp
+When compiled with LOCKDEBUG (see
+.Xr options 4 ) ,
+ensure that the current process can successfully acquire mtx.
+If mtx is already owned by the current process, the system will panic
+with a "locking against myself" error.
+.Pp
+This function is needed because
+.Fn mutex_owned
+does not differentiate if a spin mutex is owned by the current process
+vs owned by another process.
+.Fn mutex_ownable
+is reasonably heavy-weight (since it acquires the mutex, possibly
+waiting for it), so should only be used with
+.Xr KDASSERT 9 .
 .It Fn mutex_owned "mtx"
 .Pp
 For adaptive mutexes, return non-zero if the current LWP holds the mutex.
Index: distrib/sets/lists/comp/mi
===================================================================
RCS file: /cvsroot/src/distrib/sets/lists/comp/mi,v
retrieving revision 1.2125
retrieving revision 1.2125.2.3
diff -u -p -r1.2125 -r1.2125.2.3
--- distrib/sets/lists/comp/mi	20 Apr 2017 13:11:03 -0000	1.2125
+++ distrib/sets/lists/comp/mi	30 Apr 2017 04:56:55 -0000	1.2125.2.3
@@ -1,4 +1,4 @@
-#	$NetBSD: mi,v 1.2125 2017/04/20 13:11:03 joerg Exp $
+#	$NetBSD: mi,v 1.2125.2.3 2017/04/30 04:56:55 pgoyette Exp $
 #
 # Note: don't delete entries from here - mark them as "obsolete" instead.
 ./etc/mtree/set.comp				comp-sys-root
@@ -2873,6 +2873,7 @@
 ./usr/include/sys/ksyms.h			comp-c-include
 ./usr/include/sys/ktrace.h			comp-c-include
 ./usr/include/sys/lkm.h				comp-obsolete		obsolete
+./usr/include/sys/localcount.h			comp-c-include
 ./usr/include/sys/localedef.h			comp-c-include
 ./usr/include/sys/lock.h			comp-c-include
 ./usr/include/sys/lockf.h			comp-c-include
@@ -10312,6 +10313,7 @@
 ./usr/share/man/cat9/config_defer.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/config_detach.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/config_detach_children.0	comp-sys-catman		.cat
+./usr/share/man/cat9/config_detach_release.0	comp-sys-catman		.cat
 ./usr/share/man/cat9/config_finalize_register.0 comp-sys-catman		.cat
 ./usr/share/man/cat9/config_found.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/config_found_ia.0		comp-sys-catman		.cat
@@ -10740,6 +10742,7 @@
 ./usr/share/man/cat9/le64enc.0			comp-sys-catman		.cat
 ./usr/share/man/cat9/le64toh.0			comp-sys-catman		.cat
 ./usr/share/man/cat9/linedisc.0			comp-sys-catman		.cat
+./usr/share/man/cat9/localcount.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/lock.0			comp-sys-catman		.cat
 ./usr/share/man/cat9/locking.0			comp-sys-catman		.cat
 ./usr/share/man/cat9/lockinit.0			comp-sys-catman		.cat
@@ -10833,6 +10836,7 @@
 ./usr/share/man/cat9/mutex_enter.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/mutex_exit.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/mutex_init.0		comp-sys-catman		.cat
+./usr/share/man/cat9/mutex_ownable.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/mutex_owned.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/mutex_spin_enter.0		comp-sys-catman		.cat
 ./usr/share/man/cat9/mutex_spin_exit.0		comp-sys-catman		.cat
@@ -17774,6 +17778,7 @@
 ./usr/share/man/html9/config_defer.html		comp-sys-htmlman	html
 ./usr/share/man/html9/config_detach.html	comp-sys-htmlman	html
 ./usr/share/man/html9/config_detach_children.html	comp-sys-htmlman	html
+./usr/share/man/html9/config_detach_release.html	comp-sys-htmlman	html
 ./usr/share/man/html9/config_finalize_register.html	comp-sys-htmlman	html
 ./usr/share/man/html9/config_found.html		comp-sys-htmlman	html
 ./usr/share/man/html9/config_found_ia.html	comp-sys-htmlman	html
@@ -18181,6 +18186,7 @@
 ./usr/share/man/html9/le64enc.html		comp-sys-htmlman	html
 ./usr/share/man/html9/le64toh.html		comp-sys-htmlman	html
 ./usr/share/man/html9/linedisc.html		comp-sys-htmlman	html
+./usr/share/man/html9/localcount.html		comp-sys-htmlman	html
 ./usr/share/man/html9/lock.html			comp-sys-htmlman	html
 ./usr/share/man/html9/locking.html		comp-sys-htmlman	html
 ./usr/share/man/html9/lockinit.html		comp-sys-htmlman	html
@@ -18273,6 +18279,7 @@
 ./usr/share/man/html9/mutex_enter.html		comp-sys-htmlman	html
 ./usr/share/man/html9/mutex_exit.html		comp-sys-htmlman	html
 ./usr/share/man/html9/mutex_init.html		comp-sys-htmlman	html
+./usr/share/man/html9/mutex_ownable.html	comp-sys-htmlman	html
 ./usr/share/man/html9/mutex_owned.html		comp-sys-htmlman	html
 ./usr/share/man/html9/mutex_spin_enter.html	comp-sys-htmlman	html
 ./usr/share/man/html9/mutex_spin_exit.html	comp-sys-htmlman	html
@@ -25348,6 +25355,7 @@
 ./usr/share/man/man9/config_defer.9		comp-sys-man		.man
 ./usr/share/man/man9/config_detach.9		comp-sys-man		.man
 ./usr/share/man/man9/config_detach_children.9	comp-sys-man		.man
+./usr/share/man/man9/config_detach_release.9	comp-sys-man		.man
 ./usr/share/man/man9/config_finalize_register.9 comp-sys-man		.man
 ./usr/share/man/man9/config_found.9		comp-sys-man		.man
 ./usr/share/man/man9/config_found_ia.9		comp-sys-man		.man
@@ -25776,6 +25784,7 @@
 ./usr/share/man/man9/le64enc.9			comp-sys-man		.man
 ./usr/share/man/man9/le64toh.9			comp-sys-man		.man
 ./usr/share/man/man9/linedisc.9			comp-sys-man		.man
+./usr/share/man/man9/localcount.9		comp-sys-man		.man
 ./usr/share/man/man9/lock.9			comp-sys-man		.man
 ./usr/share/man/man9/locking.9			comp-sys-man		.man
 ./usr/share/man/man9/lockinit.9			comp-sys-man		.man
@@ -25869,6 +25878,7 @@
 ./usr/share/man/man9/mutex_enter.9		comp-sys-man		.man
 ./usr/share/man/man9/mutex_exit.9		comp-sys-man		.man
 ./usr/share/man/man9/mutex_init.9		comp-sys-man		.man
+./usr/share/man/man9/mutex_ownable.9		comp-sys-man		.man
 ./usr/share/man/man9/mutex_owned.9		comp-sys-man		.man
 ./usr/share/man/man9/mutex_spin_enter.9		comp-sys-man		.man
 ./usr/share/man/man9/mutex_spin_exit.9		comp-sys-man		.man
Index: sys/sys/mutex.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mutex.h,v
retrieving revision 1.20
retrieving revision 1.20.52.1
diff -u -p -r1.20 -r1.20.52.1
--- sys/sys/mutex.h	8 Feb 2010 09:54:27 -0000	1.20
+++ sys/sys/mutex.h	30 Apr 2017 04:56:55 -0000	1.20.52.1
@@ -1,4 +1,4 @@
-/*	$NetBSD: mutex.h,v 1.20 2010/02/08 09:54:27 skrll Exp $	*/
+/*	$NetBSD: mutex.h,v 1.20.52.1 2017/04/30 04:56:55 pgoyette Exp $	*/
 
 /*-
  * Copyright (c) 2002, 2006, 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -204,6 +204,7 @@ void	mutex_spin_exit(kmutex_t *);
 int	mutex_tryenter(kmutex_t *);
 
 int	mutex_owned(kmutex_t *);
+int	mutex_ownable(kmutex_t *);
 lwp_t	*mutex_owner(kmutex_t *);
 
 void	mutex_obj_init(void);
Index: sys/rump/librump/rumpkern/locks.c
===================================================================
RCS file: /cvsroot/src/sys/rump/librump/rumpkern/locks.c,v
retrieving revision 1.73
diff -u -p -r1.73 locks.c
--- sys/rump/librump/rumpkern/locks.c	27 Jan 2017 09:50:47 -0000	1.73
+++ sys/rump/librump/rumpkern/locks.c	1 May 2017 10:53:55 -0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: locks.c,v 1.73 2017/01/27 09:50:47 ozaki-r Exp $	*/
+/*	$NetBSD: locks.c,v 1.73.4.2 2017/04/30 07:07:56 pgoyette Exp $	*/
 
 /*
  * Copyright (c) 2007-2011 Antti Kantee.  All Rights Reserved.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: locks.c,v 1.73 2017/01/27 09:50:47 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: locks.c,v 1.73.4.2 2017/04/30 07:07:56 pgoyette Exp $");
 
 #include <sys/param.h>
 #include <sys/kmem.h>
@@ -183,6 +183,16 @@ mutex_exit(kmutex_t *mtx)
 __strong_alias(mutex_spin_exit,mutex_exit);
 
 int
+mutex_ownable(kmutex_t *mtx)
+{
+
+#ifdef LOCKDEBUG
+	WANTLOCK(mtx, -1);
+#endif
+	return 1;
+}
+
+int
 mutex_owned(kmutex_t *mtx)
 {
 
Index: sys/kern/kern_mutex.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_mutex.c,v
retrieving revision 1.64
diff -u -p -r1.64 kern_mutex.c
--- sys/kern/kern_mutex.c	26 Jan 2017 04:11:56 -0000	1.64
+++ sys/kern/kern_mutex.c	1 May 2017 10:53:59 -0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: kern_mutex.c,v 1.64 2017/01/26 04:11:56 christos Exp $	*/
+/*	$NetBSD: kern_mutex.c,v 1.64.4.1 2017/04/30 04:56:55 pgoyette Exp $	*/
 
 /*-
  * Copyright (c) 2002, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
 #define	__MUTEX_PRIVATE
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_mutex.c,v 1.64 2017/01/26 04:11:56 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_mutex.c,v 1.64.4.1 2017/04/30 04:56:55 pgoyette Exp $");
 
 #include <sys/param.h>
 #include <sys/atomic.h>
@@ -75,6 +75,9 @@ __KERNEL_RCSID(0, "$NetBSD: kern_mutex.c
 #define	MUTEX_WANTLOCK(mtx)					\
     LOCKDEBUG_WANTLOCK(MUTEX_DEBUG_P(mtx), (mtx),		\
         (uintptr_t)__builtin_return_address(0), 0)
+#define	MUTEX_TESTLOCK(mtx)					\
+    LOCKDEBUG_WANTLOCK(MUTEX_DEBUG_P(mtx), (mtx),		\
+        (uintptr_t)__builtin_return_address(0), -1)
 #define	MUTEX_LOCKED(mtx)					\
     LOCKDEBUG_LOCKED(MUTEX_DEBUG_P(mtx), (mtx), NULL,		\
         (uintptr_t)__builtin_return_address(0), 0)
@@ -831,6 +834,23 @@ mutex_owner(kmutex_t *mtx)
 }
 
 /*
+ * mutex_ownable:
+ *
+ *	When compiled with DEBUG and LOCKDEBUG defined, ensure that
+ *	the mutex is available.  We cannot use !mutex_owned() since
+ *	that won't work correctly for spin mutexes.
+ */
+int
+mutex_ownable(kmutex_t *mtx)
+{
+
+#ifdef LOCKDEBUG
+	MUTEX_TESTLOCK(mtx);
+#endif
+	return 1;
+}
+
+/*
  * mutex_tryenter:
  *
  *	Try to acquire the mutex; return non-zero if we did.
Index: sys/kern/subr_lockdebug.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_lockdebug.c,v
retrieving revision 1.55
diff -u -p -r1.55 subr_lockdebug.c
--- sys/kern/subr_lockdebug.c	26 Jan 2017 04:11:56 -0000	1.55
+++ sys/kern/subr_lockdebug.c	1 May 2017 10:54:03 -0000
@@ -454,9 +454,9 @@ lockdebug_wantlock(const char *func, siz
 			return;
 		}
 	}
-	if (shared)
+	if (shared > 0)
 		ld->ld_shwant++;
-	else
+	else if (shared == 0)
 		ld->ld_exwant++;
 	if (recurse) {
 		lockdebug_abort1(func, line, ld, s, "locking against myself",


Home | Main Index | Thread Index | Old Index