Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 20 Nov 2011 16:33:09 +0000 (UTC)
From:      Attilio Rao <attilio@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r227758 - in head/sys: amd64/conf dev/ppbus kern sys vm
Message-ID:  <201111201633.pAKGX9fs000402@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: attilio
Date: Sun Nov 20 16:33:09 2011
New Revision: 227758
URL: http://svn.freebsd.org/changeset/base/227758

Log:
  Introduce macro stubs in the mutex implementation that will be always
  defined and will allow consumers, willing to provide options, file and
  line to locking requests, to not worry about options redefining the
  interfaces.
  This is typically useful when there is the need to build another
  locking interface on top of the mutex one.
  
  The introduced functions that consumers can use are:
  - mtx_lock_flags_
  - mtx_unlock_flags_
  - mtx_lock_spin_flags_
  - mtx_unlock_spin_flags_
  - mtx_assert_
  - thread_lock_flags_
  
  Spare notes:
  - Likely we can get rid of all the 'INVARIANTS' specification in the
    ppbus code by using the same macro as done in this patch (but this is
    left to the ppbus maintainer)
  - all the other locking interfaces may require a similar cleanup, where
    the most notable case is sx which will allow a further cleanup of
    vm_map locking facilities
  - The patch should be fully compatible with older branches, thus a MFC
    is previewed (infact it uses all the underlying mechanisms already
    present).
  
  Comments review by:	eadler, Ben Kaduk
  Discussed with:		kib, jhb
  MFC after:	1 month

Modified:
  head/sys/amd64/conf/GENERIC
  head/sys/dev/ppbus/ppb_base.c
  head/sys/kern/kern_mutex.c
  head/sys/sys/mutex.h
  head/sys/vm/vm_map.c

Modified: head/sys/amd64/conf/GENERIC
==============================================================================
--- head/sys/amd64/conf/GENERIC	Sun Nov 20 15:18:49 2011	(r227757)
+++ head/sys/amd64/conf/GENERIC	Sun Nov 20 16:33:09 2011	(r227758)
@@ -73,11 +73,11 @@ options 	KDB			# Enable kernel debugger 
 options 	DDB			# Support DDB.
 options 	GDB			# Support remote GDB.
 options 	DEADLKRES		# Enable the deadlock resolver
-options 	INVARIANTS		# Enable calls of extra sanity checking
+#options 	INVARIANTS		# Enable calls of extra sanity checking
 options 	INVARIANT_SUPPORT	# Extra sanity checks of internal structures, required by INVARIANTS
-options 	WITNESS			# Enable checks to detect deadlocks and cycles
-options 	WITNESS_SKIPSPIN	# Don't run witness on spinlocks for speed
-options 	MALLOC_DEBUG_MAXZONES=8	# Separate malloc(9) zones
+#options 	WITNESS			# Enable checks to detect deadlocks and cycles
+#options 	WITNESS_SKIPSPIN	# Don't run witness on spinlocks for speed
+#options 	MALLOC_DEBUG_MAXZONES=8	# Separate malloc(9) zones
 
 # Make an SMP-capable kernel by default
 options 	SMP			# Symmetric MultiProcessor Kernel

Modified: head/sys/dev/ppbus/ppb_base.c
==============================================================================
--- head/sys/dev/ppbus/ppb_base.c	Sun Nov 20 15:18:49 2011	(r227757)
+++ head/sys/dev/ppbus/ppb_base.c	Sun Nov 20 16:33:09 2011	(r227758)
@@ -236,11 +236,8 @@ ppb_unlock(device_t bus)
 void
 _ppb_assert_locked(device_t bus, const char *file, int line)
 {
-#ifdef INVARIANTS
-	struct ppb_data *ppb = DEVTOSOFTC(bus);
 
-	_mtx_assert(ppb->ppc_lock, MA_OWNED, file, line);
-#endif
+	mtx_assert_(DEVTOSOFTC(bus)->ppc_lock, MA_OWNED, file, line);
 }
 
 void

Modified: head/sys/kern/kern_mutex.c
==============================================================================
--- head/sys/kern/kern_mutex.c	Sun Nov 20 15:18:49 2011	(r227757)
+++ head/sys/kern/kern_mutex.c	Sun Nov 20 16:33:09 2011	(r227758)
@@ -274,7 +274,7 @@ _mtx_unlock_spin_flags(struct mtx *m, in
  * is already owned, it will recursively acquire the lock.
  */
 int
-_mtx_trylock(struct mtx *m, int opts, const char *file, int line)
+mtx_trylock_flags_(struct mtx *m, int opts, const char *file, int line)
 {
 #ifdef LOCK_PROFILING
 	uint64_t waittime = 0;
@@ -540,7 +540,7 @@ _mtx_lock_spin(struct mtx *m, uintptr_t 
 #endif /* SMP */
 
 void
-_thread_lock_flags(struct thread *td, int opts, const char *file, int line)
+thread_lock_flags_(struct thread *td, int opts, const char *file, int line)
 {
 	struct mtx *m;
 	uintptr_t tid;

Modified: head/sys/sys/mutex.h
==============================================================================
--- head/sys/sys/mutex.h	Sun Nov 20 15:18:49 2011	(r227757)
+++ head/sys/sys/mutex.h	Sun Nov 20 16:33:09 2011	(r227758)
@@ -81,6 +81,10 @@
  *	 of the kernel via macros, thus allowing us to use the cpp LOCK_FILE
  *	 and LOCK_LINE. These functions should not be called directly by any
  *	 code using the API. Their macros cover their functionality.
+ *	 Functions with a `_' suffix are the entrypoint for the common
+ *	 KPI covering both compat shims and fast path case.  These can be
+ *	 used by consumers willing to pass options, file and line
+ *	 informations, in an option-independent way.
  *
  * [See below for descriptions]
  *
@@ -88,6 +92,8 @@
 void	mtx_init(struct mtx *m, const char *name, const char *type, int opts);
 void	mtx_destroy(struct mtx *m);
 void	mtx_sysinit(void *arg);
+int	mtx_trylock_flags_(struct mtx *m, int opts, const char *file,
+	    int line);
 void	mutex_init(void);
 void	_mtx_lock_sleep(struct mtx *m, uintptr_t tid, int opts,
 	    const char *file, int line);
@@ -97,7 +103,6 @@ void	_mtx_lock_spin(struct mtx *m, uintp
 	    const char *file, int line);
 #endif
 void	_mtx_unlock_spin(struct mtx *m, int opts, const char *file, int line);
-int	_mtx_trylock(struct mtx *m, int opts, const char *file, int line);
 void	_mtx_lock_flags(struct mtx *m, int opts, const char *file, int line);
 void	_mtx_unlock_flags(struct mtx *m, int opts, const char *file, int line);
 void	_mtx_lock_spin_flags(struct mtx *m, int opts, const char *file,
@@ -107,12 +112,12 @@ void	_mtx_unlock_spin_flags(struct mtx *
 #if defined(INVARIANTS) || defined(INVARIANT_SUPPORT)
 void	_mtx_assert(const struct mtx *m, int what, const char *file, int line);
 #endif
-void	_thread_lock_flags(struct thread *, int, const char *, int);
+void	thread_lock_flags_(struct thread *, int, const char *, int);
 
 #define	thread_lock(tdp)						\
-    _thread_lock_flags((tdp), 0, __FILE__, __LINE__)
+	thread_lock_flags_((tdp), 0, __FILE__, __LINE__)
 #define	thread_lock_flags(tdp, opt)					\
-    _thread_lock_flags((tdp), (opt), __FILE__, __LINE__)
+	thread_lock_flags_((tdp), (opt), __FILE__, __LINE__)
 #define	thread_unlock(tdp)						\
        mtx_unlock_spin((tdp)->td_lock)
 
@@ -290,27 +295,48 @@ extern struct mtx_pool *mtxpool_sleep;
 #error LOCK_DEBUG not defined, include <sys/lock.h> before <sys/mutex.h>
 #endif
 #if LOCK_DEBUG > 0 || defined(MUTEX_NOINLINE)
-#define	mtx_lock_flags(m, opts)						\
-	_mtx_lock_flags((m), (opts), LOCK_FILE, LOCK_LINE)
-#define	mtx_unlock_flags(m, opts)					\
-	_mtx_unlock_flags((m), (opts), LOCK_FILE, LOCK_LINE)
-#define	mtx_lock_spin_flags(m, opts)					\
-	_mtx_lock_spin_flags((m), (opts), LOCK_FILE, LOCK_LINE)
-#define	mtx_unlock_spin_flags(m, opts)					\
-	_mtx_unlock_spin_flags((m), (opts), LOCK_FILE, LOCK_LINE)
+#define	mtx_lock_flags_(m, opts, file, line)				\
+	_mtx_lock_flags((m), (opts), (file), (line))
+#define	mtx_unlock_flags_(m, opts, file, line)				\
+	_mtx_unlock_flags((m), (opts), (file), (line))
+#define	mtx_lock_spin_flags_(m, opts, file, line)			\
+	_mtx_lock_spin_flags((m), (opts), (file), (line))
+#define	mtx_unlock_spin_flags_(m, opts, file, line)			\
+	_mtx_unlock_spin_flags((m), (opts), (file), (line))
 #else	/* LOCK_DEBUG == 0 && !MUTEX_NOINLINE */
+#define	mtx_lock_flags_(m, opts, file, line)				\
+	__mtx_lock((m), curthread, (opts), (file), (line))
+#define	mtx_unlock_flags_(m, opts, file, line)				\
+	__mtx_unlock((m), curthread, (opts), (file), (line))
+#define	mtx_lock_spin_flags_(m, opts, file, line)			\
+	__mtx_lock_spin((m), curthread, (opts), (file), (line))
+#define	mtx_unlock_spin_flags_(m, opts, file, line)			\
+	__mtx_unlock_spin((m))
+#endif	/* LOCK_DEBUG > 0 || MUTEX_NOINLINE */
+
+#ifdef INVARIANTS
+#define	mtx_assert_(m, what, file, line)				\
+	_mtx_assert((m), (what), (file), (line))
+
+#define GIANT_REQUIRED	mtx_assert_(&Giant, MA_OWNED, __FILE__, __LINE__)
+
+#else	/* INVARIANTS */
+#define mtx_assert_(m, what, file, line)	(void)0
+#define GIANT_REQUIRED
+#endif	/* INVARIANTS */
+
 #define	mtx_lock_flags(m, opts)						\
-	__mtx_lock((m), curthread, (opts), LOCK_FILE, LOCK_LINE)
+	mtx_lock_flags_((m), (opts), LOCK_FILE, LOCK_LINE)
 #define	mtx_unlock_flags(m, opts)					\
-	__mtx_unlock((m), curthread, (opts), LOCK_FILE, LOCK_LINE)
+	mtx_unlock_flags_((m), (opts), LOCK_FILE, LOCK_LINE)
 #define	mtx_lock_spin_flags(m, opts)					\
-	__mtx_lock_spin((m), curthread, (opts), LOCK_FILE, LOCK_LINE)
+	mtx_lock_spin_flags_((m), (opts), LOCK_FILE, LOCK_LINE)
 #define	mtx_unlock_spin_flags(m, opts)					\
-	__mtx_unlock_spin((m))
-#endif	/* LOCK_DEBUG > 0 || MUTEX_NOINLINE */
-
+	mtx_unlock_spin_flags_((m), (opts), LOCK_FILE, LOCK_LINE)
 #define mtx_trylock_flags(m, opts)					\
-	_mtx_trylock((m), (opts), LOCK_FILE, LOCK_LINE)
+	mtx_trylock_flags_((m), (opts), LOCK_FILE, LOCK_LINE)
+#define	mtx_assert(m, what)						\
+	mtx_assert_((m), (what), __FILE__, __LINE__)
 
 #define	mtx_sleep(chan, mtx, pri, wmesg, timo)				\
 	_sleep((chan), &(mtx)->lock_object, (pri), (wmesg), (timo))
@@ -398,17 +424,6 @@ struct mtx_args {
 #define MA_NOTRECURSED	LA_NOTRECURSED
 #endif
 
-#ifdef INVARIANTS
-#define	mtx_assert(m, what)						\
-	_mtx_assert((m), (what), __FILE__, __LINE__)
-
-#define GIANT_REQUIRED	mtx_assert(&Giant, MA_OWNED)
-
-#else	/* INVARIANTS */
-#define mtx_assert(m, what)	(void)0
-#define GIANT_REQUIRED
-#endif	/* INVARIANTS */
-
 /*
  * Common lock type names.
  */

Modified: head/sys/vm/vm_map.c
==============================================================================
--- head/sys/vm/vm_map.c	Sun Nov 20 15:18:49 2011	(r227757)
+++ head/sys/vm/vm_map.c	Sun Nov 20 16:33:09 2011	(r227758)
@@ -464,7 +464,7 @@ _vm_map_lock(vm_map_t map, const char *f
 {
 
 	if (map->system_map)
-		_mtx_lock_flags(&map->system_mtx, 0, file, line);
+		mtx_lock_flags_(&map->system_mtx, 0, file, line);
 	else
 		(void)_sx_xlock(&map->lock, 0, file, line);
 	map->timestamp++;
@@ -489,7 +489,7 @@ _vm_map_unlock(vm_map_t map, const char 
 {
 
 	if (map->system_map)
-		_mtx_unlock_flags(&map->system_mtx, 0, file, line);
+		mtx_unlock_flags_(&map->system_mtx, 0, file, line);
 	else {
 		_sx_xunlock(&map->lock, file, line);
 		vm_map_process_deferred();
@@ -501,7 +501,7 @@ _vm_map_lock_read(vm_map_t map, const ch
 {
 
 	if (map->system_map)
-		_mtx_lock_flags(&map->system_mtx, 0, file, line);
+		mtx_lock_flags_(&map->system_mtx, 0, file, line);
 	else
 		(void)_sx_slock(&map->lock, 0, file, line);
 }
@@ -511,7 +511,7 @@ _vm_map_unlock_read(vm_map_t map, const 
 {
 
 	if (map->system_map)
-		_mtx_unlock_flags(&map->system_mtx, 0, file, line);
+		mtx_unlock_flags_(&map->system_mtx, 0, file, line);
 	else {
 		_sx_sunlock(&map->lock, file, line);
 		vm_map_process_deferred();
@@ -524,7 +524,7 @@ _vm_map_trylock(vm_map_t map, const char
 	int error;
 
 	error = map->system_map ?
-	    !_mtx_trylock(&map->system_mtx, 0, file, line) :
+	    !mtx_trylock_flags_(&map->system_mtx, 0, file, line) :
 	    !_sx_try_xlock(&map->lock, file, line);
 	if (error == 0)
 		map->timestamp++;
@@ -537,7 +537,7 @@ _vm_map_trylock_read(vm_map_t map, const
 	int error;
 
 	error = map->system_map ?
-	    !_mtx_trylock(&map->system_mtx, 0, file, line) :
+	    !mtx_trylock_flags_(&map->system_mtx, 0, file, line) :
 	    !_sx_try_slock(&map->lock, file, line);
 	return (error == 0);
 }
@@ -558,9 +558,7 @@ _vm_map_lock_upgrade(vm_map_t map, const
 	unsigned int last_timestamp;
 
 	if (map->system_map) {
-#ifdef INVARIANTS
-		_mtx_assert(&map->system_mtx, MA_OWNED, file, line);
-#endif
+		mtx_assert_(&map->system_mtx, MA_OWNED, file, line);
 	} else {
 		if (!_sx_try_upgrade(&map->lock, file, line)) {
 			last_timestamp = map->timestamp;
@@ -586,9 +584,7 @@ _vm_map_lock_downgrade(vm_map_t map, con
 {
 
 	if (map->system_map) {
-#ifdef INVARIANTS
-		_mtx_assert(&map->system_mtx, MA_OWNED, file, line);
-#endif
+		mtx_assert_(&map->system_mtx, MA_OWNED, file, line);
 	} else
 		_sx_downgrade(&map->lock, file, line);
 }
@@ -609,13 +605,14 @@ vm_map_locked(vm_map_t map)
 		return (sx_xlocked(&map->lock));
 }
 
+/* XXX: INVARIANTS here is still necessary because of sx support. */
 #ifdef INVARIANTS
 static void
 _vm_map_assert_locked(vm_map_t map, const char *file, int line)
 {
 
 	if (map->system_map)
-		_mtx_assert(&map->system_mtx, MA_OWNED, file, line);
+		mtx_assert_(&map->system_mtx, MA_OWNED, file, line);
 	else
 		_sx_assert(&map->lock, SA_XLOCKED, file, line);
 }
@@ -626,7 +623,7 @@ _vm_map_assert_locked_read(vm_map_t map,
 {
 
 	if (map->system_map)
-		_mtx_assert(&map->system_mtx, MA_OWNED, file, line);
+		mtx_assert_(&map->system_mtx, MA_OWNED, file, line);
 	else
 		_sx_assert(&map->lock, SA_SLOCKED, file, line);
 }
@@ -661,7 +658,7 @@ _vm_map_unlock_and_wait(vm_map_t map, in
 
 	mtx_lock(&map_sleep_mtx);
 	if (map->system_map)
-		_mtx_unlock_flags(&map->system_mtx, 0, file, line);
+		mtx_unlock_flags_(&map->system_mtx, 0, file, line);
 	else
 		_sx_xunlock(&map->lock, file, line);
 	return (msleep(&map->root, &map_sleep_mtx, PDROP | PVM, "vmmaps",



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201111201633.pAKGX9fs000402>