From c4d4f4147d11045012750f4e2fafad6de9a4dc5f Mon Sep 17 00:00:00 2001 From: Greg Lehey Date: Tue, 22 May 2001 02:34:30 +0000 Subject: [PATCH] Add a new debug flag, DEBUG_LOCKREQS, which logs only lock requests. Use this instead of DEBUG_LASTREQS to decide whether to log lock requests. MFS: vinumlock: Catch a potential race condition where one process is waiting for a lock, and between the time it is woken and it retries the lock, another process gets it and places it in the first entry in the table. This problem has not been observed, but it's possible, and it's easy enough to fix. Submitted by: tegge vinumunlock: Catch a real bug capable of hanging a system. When releasing a lock, vinumunlock() called wakeup_one. This caused wakeups to sometimes get lost. After due consideration, we think that this is due to the fact that you can't guarantee that some other process is also waiting on the same address. This makes wakeup_one a very dangerous function to use. --- sys/dev/vinum/vinumlock.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/sys/dev/vinum/vinumlock.c b/sys/dev/vinum/vinumlock.c index 801f13f50dd..f7d3f78c921 100644 --- a/sys/dev/vinum/vinumlock.c +++ b/sys/dev/vinum/vinumlock.c @@ -154,17 +154,18 @@ lockrange(daddr_t stripe, struct buf *bp, struct plex *plex) if ((lock->stripe == stripe) /* it's our stripe */ &&(lock->bp != bp)) { /* but not our request */ #ifdef VINUMDEBUG - if (debug & DEBUG_LASTREQS) { - struct rangelock info; + if (debug & DEBUG_LOCKREQS) { + struct rangelockinfo lockinfo; - info.stripe = stripe; - info.bp = bp; - logrq(loginfo_lockwait, (union rqinfou) &info, bp); + lockinfo.stripe = stripe; + lockinfo.bp = bp; + lockinfo.plexno = plex->plexno; + logrq(loginfo_lockwait, (union rqinfou) &lockinfo, bp); } #endif plex->lockwaits++; /* waited one more time */ msleep(lock, &plex->lockmtx, PRIBIO, "vrlock", 0); - lock = plex->lock; /* start again */ + lock = &plex->lock[-1]; /* start again */ foundlocks = 0; pos = NULL; } @@ -189,8 +190,14 @@ lockrange(daddr_t stripe, struct buf *bp, struct plex *plex) plex->usedlocks++; /* one more lock */ mtx_unlock(&plex->lockmtx); #ifdef VINUMDEBUG - if (debug & DEBUG_LASTREQS) - logrq(loginfo_lock, (union rqinfou) pos, bp); + if (debug & DEBUG_LOCKREQS) { + struct rangelockinfo lockinfo; + + lockinfo.stripe = stripe; + lockinfo.bp = bp; + lockinfo.plexno = plex->plexno; + logrq(loginfo_lock, (union rqinfou) &lockinfo, bp); + } #endif return pos; } @@ -211,14 +218,20 @@ unlockrange(int plexno, struct rangelock *lock) &plex->lock[PLEX_LOCKS]); #endif #ifdef VINUMDEBUG - if (debug & DEBUG_LASTREQS) - logrq(loginfo_unlock, (union rqinfou) lock, lock->bp); + if (debug & DEBUG_LOCKREQS) { + struct rangelockinfo lockinfo; + + lockinfo.stripe = lock->stripe; + lockinfo.bp = lock->bp; + lockinfo.plexno = plex->plexno; + logrq(loginfo_lockwait, (union rqinfou) &lockinfo, lock->bp); + } #endif lock->stripe = 0; /* no longer used */ plex->usedlocks--; /* one less lock */ if (plex->usedlocks == PLEX_LOCKS - 1) /* we were full, */ - wakeup_one(&plex->usedlocks); /* get a waiter if one's there */ - wakeup_one((void *) lock); + wakeup(&plex->usedlocks); /* get a waiter if one's there */ + wakeup((void *) lock); } /* Get a lock for the global config, wait if it's not available */