Date: Sat, 16 Mar 2002 19:08:53 +0900 From: Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> To: Alfred Perlstein <bright@mu.org> Cc: "Brian F. Feldman" <green@FreeBSD.org>, jhb@FreeBSD.org, current@FreeBSD.org, Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> Subject: sx_upgrade() (was: Re: cvs commit: src/sys/kern kern_mtxpool.c src/sys/sys kernel.h src/sys/vm vm_fault.c vm_glue.c vm_map.c vm_map.h vm_pageout.c vm_zone.c) Message-ID: <200203161009.g2GA8rA5092735@silver.carrots.uucp.r.dl.itc.u-tokyo.ac.jp> In-Reply-To: <20020315185320.GJ4857@elvis.mu.org> References: <20020315054113.GC4857@elvis.mu.org> <200203151121.g2FBLnj36094@green.bikeshed.org> <20020315185320.GJ4857@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Multipart_Sat_Mar_16_19:08:53_2002-1
Content-Type: text/plain; charset=US-ASCII
[Add jhb and move to -current]
On Fri, 15 Mar 2002 10:53:20 -0800,
Alfred Perlstein <bright@mu.org> said:
Alfred> * Brian F. Feldman <green@FreeBSD.org> [020315 03:22] wrote:
>> Alfred Perlstein <bright@mu.org> wrote:
>> >
>> > What is the problem?
>>
>> Damn good question. Are the tracebacks related? If not, what are you
>> supposed to be telling me it's deadlocking on? I don't see the system being
>> deadlocked. What is it actually supposed to be blocked on?
>>
>> > Well basically you changed:
>> >
>> > ! static __inline__ int
>> > ! _vm_map_lock_upgrade(vm_map_t map, struct thread *td) {
>> > ! int error;
>> > !
>> > ! vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
>> > ! error = lockmgr(&map->lock, LK_EXCLUPGRADE, NULL, td);
>> > ! if (error == 0)
>> > ! map->timestamp++;
>> > ! return error;
>> > }
>> >
>> > into:
>> >
>> > ! _vm_map_lock_upgrade(vm_map_t map, const char *file, int line)
>> > {
>> > ! vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
>> > ! if (_sx_try_upgrade(&map->lock, file, line)) {
>> > ! map->timestamp++;
>> > ! return (0);
>> > ! }
>> > ! return (EWOULDBLOCK);
>> > }
>>
>> It doesn't need LK_EXCLUPGRADE semantics, only LK_UPGRADE, if it's not
>> blocking. It backs out completely and unlocks the shared reference and
>> tries for an exclusive lock.
Alfred> Sigh, you're making me do all the work here... :(
Alfred> lockmgr(&map->lock, LK_EXCLUPGRADE, NULL, td);
Alfred> means:
Alfred> Turn my shared lock into an exclusive lock,
Alfred> if it's shared then wait for all shared locks to drain,
Alfred> however if someone else is requesting an exclusive lock, then fail.
Alfred> _sx_try_upgrade(&map->lock, file, line)
Alfred> means:
Alfred> Give me an exclusive lock
Alfred> if anyone else has a shared lock then fail immediately.
Alfred> What happens in your case is that you get into a busy loop
Alfred> because you never yeild the processor.
Alfred> This happens in vm_map_lookup() because of this:
Alfred> if (fault_type & VM_PROT_WRITE) {
Alfred> /*
Alfred> * Make a new object, and place it in the object
Alfred> * chain. Note that no new references have appeared
Alfred> * -- one just moved from the map to the new
Alfred> * object.
Alfred> */
Alfred> if (vm_map_lock_upgrade(map))
Alfred> goto RetryLookup;
Alfred> So, you fail your sx_lock upgrade and cause the cpu to spin.
Alfred> You basically need to add logic to the sxlock subsystem to do what
Alfred> lockmgr does, actually wait for the others to go away or fail if
Alfred> someone else wants an upgrade.
Attached patch implements sx_upgrade() which should work as you said
above. This compiles fine, but is not tested yet.
sx_upgrader records the thread that wants to upgrade. If sx_upgrader
is non-NULL, sx_sunlock() wakes up the upgrader rather than other
exclusive lock waiters.
--Multipart_Sat_Mar_16_19:08:53_2002-1
Content-Type: application/octet-stream; type=patch
Content-Disposition: attachment; filename="sx_upgrade.diff"
Content-Transfer-Encoding: 7bit
==== //depot/user/tanimura/inodezone_uma/kern/kern_sx.c#1 - /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/kern/kern_sx.c ====
--- /tmp/tmp.89710.0 Sat Mar 16 18:50:40 2002
+++ /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/kern/kern_sx.c Sat Mar 16 17:56:00 2002
@@ -72,6 +72,7 @@
cv_init(&sx->sx_excl_cv, description);
sx->sx_excl_wcnt = 0;
sx->sx_xholder = NULL;
+ sx->sx_upgrader = NULL;
LOCK_LOG_INIT(lock, 0);
@@ -215,7 +216,15 @@
* there are exclusive lock waiters.
*/
if (sx->sx_excl_wcnt > 0) {
- if (sx->sx_cnt == 0)
+ /*
+ * The upgrader beats any other exclusive lockers.
+ * Note that the upgrader holds the last shared lock.
+ */
+ if (sx->sx_upgrader != NULL && sx->sx_cnt == 1) {
+ KASSERT(td->td_flags & TDF_CVWAITQ != 0,
+ ("_sx_sunlock: the upgrader is not waiting for sx_excl_cv"));
+ cv_waitq_remove(sx->sx_upgrader);
+ } else if (sx->sx_cnt == 0)
cv_signal(&sx->sx_excl_cv);
} else if (sx->sx_shrd_wcnt > 0)
cv_broadcast(&sx->sx_shrd_cv);
@@ -250,6 +259,45 @@
LOCK_LOG_LOCK("XUNLOCK", &sx->sx_object, 0, 0, file, line);
mtx_unlock(sx->sx_lock);
+}
+
+int
+_sx_upgrade(struct sx *sx, const char *file, int line)
+{
+
+ _sx_assert(sx, SX_SLOCKED, file, line);
+ mtx_lock(sx->sx_lock);
+
+ /*
+ * If another thread is waiting for lock upgrade,
+ * the curtherad must unlock this sx.
+ */
+ if (sx->sx_upgrader != NULL) {
+ mtx_unlock(sx->sx_lock);
+ return (0);
+ }
+ sx->sx_upgrader = curthread;
+
+ /* Loop in case we lose the race for lock acquisition. */
+ while (sx->sx_cnt != 1) {
+ sx->sx_excl_wcnt++;
+ cv_wait(&sx->sx_excl_cv, sx->sx_lock);
+ sx->sx_excl_wcnt--;
+ }
+
+ /* We must be the sole thread slocking this sx. */
+ MPASS(sx->sx_cnt == 1);
+
+ /* Acquire an exclusive lock. */
+ sx->sx_cnt = -1;
+ sx->sx_xholder = curthread;
+ sx->sx_upgrader = NULL;
+
+ LOCK_LOG_LOCK("XUPGRADE", &sx->sx_object, 0, 1, file, line);
+ WITNESS_UPGRADE(&sx->sx_object, LOP_EXCLUSIVE, file, line);
+
+ mtx_unlock(sx->sx_lock);
+ return (1);
}
int
==== //depot/user/tanimura/inodezone_uma/kern/subr_witness.c#1 - /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/kern/subr_witness.c ====
--- /tmp/tmp.89710.1 Sat Mar 16 18:50:41 2002
+++ /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/kern/subr_witness.c Sat Mar 16 18:02:38 2002
@@ -717,9 +717,9 @@
if ((lock->lo_flags & LO_UPGRADABLE) == 0)
panic("upgrade of non-upgradable lock (%s) %s @ %s:%d",
class->lc_name, lock->lo_name, file, line);
- if ((flags & LOP_TRYLOCK) == 0)
- panic("non-try upgrade of lock (%s) %s @ %s:%d", class->lc_name,
- lock->lo_name, file, line);
+ if ((flags & LOP_TRYLOCK) == 0) {
+ /* TODO: check for lock order reversal. */
+ }
if ((lock->lo_class->lc_flags & LC_SLEEPLOCK) == 0)
panic("upgrade of non-sleep lock (%s) %s @ %s:%d",
class->lc_name, lock->lo_name, file, line);
==== //depot/user/tanimura/inodezone_uma/sys/sx.h#1 - /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/sys/sx.h ====
--- /tmp/tmp.89710.2 Sat Mar 16 18:50:41 2002
+++ /home/carrots/tanimura/silver4/p4-freefall/inodezone_uma/sys/sx.h Sat Mar 16 17:33:09 2002
@@ -44,6 +44,7 @@
struct cv sx_excl_cv; /* xlock waiters. */
int sx_excl_wcnt; /* Number of xlock waiters. */
struct thread *sx_xholder; /* Thread presently holding xlock. */
+ struct thread *sx_upgrader; /* Thread presently waiting for lock upgrade. */
};
#ifdef _KERNEL
@@ -55,6 +56,7 @@
int _sx_try_xlock(struct sx *sx, const char *file, int line);
void _sx_sunlock(struct sx *sx, const char *file, int line);
void _sx_xunlock(struct sx *sx, const char *file, int line);
+int _sx_upgrade(struct sx *sx, const char *file, int line);
int _sx_try_upgrade(struct sx *sx, const char *file, int line);
void _sx_downgrade(struct sx *sx, const char *file, int line);
#ifdef INVARIANT_SUPPORT
@@ -67,6 +69,7 @@
#define sx_try_xlock(sx) _sx_try_xlock((sx), LOCK_FILE, LOCK_LINE)
#define sx_sunlock(sx) _sx_sunlock((sx), LOCK_FILE, LOCK_LINE)
#define sx_xunlock(sx) _sx_xunlock((sx), LOCK_FILE, LOCK_LINE)
+#define sx_upgrade(sx) _sx_upgrade((sx), LOCK_FILE, LOCK_LINE)
#define sx_try_upgrade(sx) _sx_try_upgrade((sx), LOCK_FILE, LOCK_LINE)
#define sx_downgrade(sx) _sx_downgrade((sx), LOCK_FILE, LOCK_LINE)
--Multipart_Sat_Mar_16_19:08:53_2002-1
Content-Type: text/plain; charset=US-ASCII
--
Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> <tanimura@FreeBSD.org>
--Multipart_Sat_Mar_16_19:08:53_2002-1--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203161009.g2GA8rA5092735>