Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 16 Mar 2017 06:51:00 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r315379 - in stable/11/sys: kern sys
Message-ID:  <201703160651.v2G6p0Jl087518@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mjg
Date: Thu Mar 16 06:51:00 2017
New Revision: 315379
URL: https://svnweb.freebsd.org/changeset/base/315379

Log:
  MFC r313392,r313784:
  
  rwlock: implement RW_LOCK_WRITER_RECURSED bit
  
  This moves recursion handling out of the inlined wunlock path and in
  particular saves a read and a branch.
  
  ==
  
  rwlock: tidy up r313392
  
  While a new bit was added and thread alignment got shifted to accomodate it,
  RW_READERS_SHIFT was not modified accordingly and clashed with the new flag.
  
  This was surprisingly harmless. If the lock was taken for writing, other flags
  were tested. If the lock was taken for reading, it would correctly work for
  readers > 1 and this was the only relevant test performed.

Modified:
  stable/11/sys/kern/kern_rwlock.c
  stable/11/sys/sys/rwlock.h
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/kern/kern_rwlock.c
==============================================================================
--- stable/11/sys/kern/kern_rwlock.c	Thu Mar 16 06:45:36 2017	(r315378)
+++ stable/11/sys/kern/kern_rwlock.c	Thu Mar 16 06:51:00 2017	(r315379)
@@ -312,6 +312,7 @@ __rw_try_wlock(volatile uintptr_t *c, co
 	if (rw_wlocked(rw) &&
 	    (rw->lock_object.lo_flags & LO_RECURSABLE) != 0) {
 		rw->rw_recurse++;
+		atomic_set_ptr(&rw->rw_lock, RW_LOCK_WRITER_RECURSED);
 		rval = 1;
 	} else
 		rval = atomic_cmpset_acq_ptr(&rw->rw_lock, RW_UNLOCKED,
@@ -345,10 +346,8 @@ _rw_wunlock_cookie(volatile uintptr_t *c
 	WITNESS_UNLOCK(&rw->lock_object, LOP_EXCLUSIVE, file, line);
 	LOCK_LOG_LOCK("WUNLOCK", &rw->lock_object, 0, rw->rw_recurse, file,
 	    line);
-	if (rw->rw_recurse)
-		rw->rw_recurse--;
-	else
-		_rw_wunlock_hard(rw, (uintptr_t)curthread, file, line);
+
+	_rw_wunlock_hard(rw, (uintptr_t)curthread, file, line);
 
 	TD_LOCKS_DEC(curthread);
 }
@@ -802,6 +801,7 @@ __rw_wlock_hard(volatile uintptr_t *c, u
 		    ("%s: recursing but non-recursive rw %s @ %s:%d\n",
 		    __func__, rw->lock_object.lo_name, file, line));
 		rw->rw_recurse++;
+		atomic_set_ptr(&rw->rw_lock, RW_LOCK_WRITER_RECURSED);
 		if (LOCK_LOG_TEST(&rw->lock_object, 0))
 			CTR2(KTR_LOCK, "%s: %p recursing", __func__, rw);
 		return;
@@ -994,12 +994,17 @@ __rw_wunlock_hard(volatile uintptr_t *c,
 		return;
 
 	rw = rwlock2rw(c);
-	MPASS(!rw_recursed(rw));
 
-	LOCKSTAT_PROFILE_RELEASE_RWLOCK(rw__release, rw,
-	    LOCKSTAT_WRITER);
-	if (_rw_write_unlock(rw, tid))
+	if (!rw_recursed(rw)) {
+		LOCKSTAT_PROFILE_RELEASE_RWLOCK(rw__release, rw,
+		    LOCKSTAT_WRITER);
+		if (_rw_write_unlock(rw, tid))
+			return;
+	} else {
+		if (--(rw->rw_recurse) == 0)
+			atomic_clear_ptr(&rw->rw_lock, RW_LOCK_WRITER_RECURSED);
 		return;
+	}
 
 	KASSERT(rw->rw_lock & (RW_LOCK_READ_WAITERS | RW_LOCK_WRITE_WAITERS),
 	    ("%s: neither of the waiter flags are set", __func__));

Modified: stable/11/sys/sys/rwlock.h
==============================================================================
--- stable/11/sys/sys/rwlock.h	Thu Mar 16 06:45:36 2017	(r315378)
+++ stable/11/sys/sys/rwlock.h	Thu Mar 16 06:51:00 2017	(r315379)
@@ -58,13 +58,14 @@
 #define	RW_LOCK_READ_WAITERS	0x02
 #define	RW_LOCK_WRITE_WAITERS	0x04
 #define	RW_LOCK_WRITE_SPINNER	0x08
+#define	RW_LOCK_WRITER_RECURSED	0x10
 #define	RW_LOCK_FLAGMASK						\
 	(RW_LOCK_READ | RW_LOCK_READ_WAITERS | RW_LOCK_WRITE_WAITERS |	\
-	RW_LOCK_WRITE_SPINNER)
+	RW_LOCK_WRITE_SPINNER | RW_LOCK_WRITER_RECURSED)
 #define	RW_LOCK_WAITERS		(RW_LOCK_READ_WAITERS | RW_LOCK_WRITE_WAITERS)
 
 #define	RW_OWNER(x)		((x) & ~RW_LOCK_FLAGMASK)
-#define	RW_READERS_SHIFT	4
+#define	RW_READERS_SHIFT	5
 #define	RW_READERS(x)		(RW_OWNER((x)) >> RW_READERS_SHIFT)
 #define	RW_READERS_LOCK(x)	((x) << RW_READERS_SHIFT | RW_LOCK_READ)
 #define	RW_ONE_READER		(1 << RW_READERS_SHIFT)
@@ -111,13 +112,9 @@
 #define	__rw_wunlock(rw, tid, file, line) do {				\
 	uintptr_t _tid = (uintptr_t)(tid);				\
 									\
-	if ((rw)->rw_recurse)						\
-		(rw)->rw_recurse--;					\
-	else {								\
-		if (__predict_false(LOCKSTAT_PROFILE_ENABLED(rw__release) ||\
-		    !_rw_write_unlock((rw), _tid)))			\
-			_rw_wunlock_hard((rw), _tid, (file), (line));	\
-	}								\
+	if (__predict_false(LOCKSTAT_PROFILE_ENABLED(rw__release) ||	\
+	    !_rw_write_unlock((rw), _tid)))				\
+		_rw_wunlock_hard((rw), _tid, (file), (line));		\
 } while (0)
 
 /*

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