Re: memcpy of struct buf, or similar?

[Chris Gilbert <chris%dokein.co.uk@localhost>]

Chris Gilbert wrote:
> Attached is a slightly revised patch, to get rid of the changes to
> whitespace and sleeptab_lookup being moved.
> 
> I also added the KASSERT back to the cv_unsleep, as the lwp lock should
> be held at that point, and the lwp lock will be set to the sleepq lock,
> so it's safe to look at the sleepq.
> 
> Note there is an assumption that cv_has_waiters will be called with the
> mutex used to access the cv held, and so locking the sleepq lock will
> make sure we get a consistent state.

I've now checked that patch in, as it fixes the issue.

I've also produced that attached patch, which brings back cv_waiters.
However, I'm not seeing any performance gain with it using build.sh,
perhaps a specific benchmark would show a gain.  In theory there should
be some gain as we don't have to lock the sleepq if it's empty, but it's
probably lost in the noise of build.sh

Thanks,
Chris

Index: kern/kern_condvar.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_condvar.c,v
retrieving revision 1.23
diff -u -p -r1.23 kern_condvar.c
--- kern/kern_condvar.c 15 Jun 2008 09:56:18 -0000      1.23
+++ kern/kern_condvar.c 15 Jun 2008 13:40:34 -0000
@@ -96,6 +96,7 @@ cv_init(kcondvar_t *cv, const char *wmes
        KASSERT(wmesg != NULL);
        cv->cv_wmesg = wmesg;
        sleepq_init(CV_SLEEPQ(cv));
+       cv->cv_waiters = 0;
 }
 
 /*
@@ -111,6 +112,7 @@ cv_destroy(kcondvar_t *cv)
 #ifdef DIAGNOSTIC
        KASSERT(cv_is_valid(cv));
        cv->cv_wmesg = deadcv;
+       cv->cv_waiters = -3;
 #endif
 }
 
@@ -131,6 +133,7 @@ cv_enter(kcondvar_t *cv, kmutex_t *mtx, 
 
        LOCKDEBUG_LOCKED(CV_DEBUG_P(cv), cv, mtx, CV_RA, 0);
 
+       cv->cv_waiters++;
        l->l_kpriority = true;
        (void)sleeptab_lookup(&sleeptab, cv, &mp);
        sq = CV_SLEEPQ(cv);
@@ -278,8 +281,8 @@ cv_signal(kcondvar_t *cv)
 
        /* LOCKDEBUG_WAKEUP(CV_DEBUG_P(cv), cv, CV_RA); */
        KASSERT(cv_is_valid(cv));
-
-       cv_wakeup_one(cv);
+       if (__predict_false(cv->cv_waiters > 0))
+               cv_wakeup_one(cv);
 }
 
 static void __noinline
@@ -292,6 +295,8 @@ cv_wakeup_one(kcondvar_t *cv)
 
        KASSERT(cv_is_valid(cv));
 
+       cv->cv_waiters--;
+
        sq = CV_SLEEPQ(cv);
        (void)sleeptab_lookup(&sleeptab, cv, &mp);
        l = TAILQ_FIRST(sq);
@@ -328,7 +333,8 @@ cv_broadcast(kcondvar_t *cv)
        /* LOCKDEBUG_WAKEUP(CV_DEBUG_P(cv), cv, CV_RA); */
        KASSERT(cv_is_valid(cv));
 
-       cv_wakeup_all(cv);
+       if (__predict_false(cv->cv_waiters > 0))
+               cv_wakeup_all(cv);
 }
 
 static void __noinline
@@ -341,6 +347,8 @@ cv_wakeup_all(kcondvar_t *cv)
 
        KASSERT(cv_is_valid(cv));
 
+       cv->cv_waiters = 0;
+
        sq = CV_SLEEPQ(cv);
        (void)sleeptab_lookup(&sleeptab, cv, &mp);
        swapin = 0;
@@ -387,18 +395,7 @@ cv_wakeup(kcondvar_t *cv)
 bool
 cv_has_waiters(kcondvar_t *cv)
 {
-       bool result;
-       kmutex_t *mp;
-       sleepq_t *sq;
-
-       sq = CV_SLEEPQ(cv);
-       (void)sleeptab_lookup(&sleeptab, cv, &mp);
-
-       /* we can only get a valid result with the sleepq locked */
-       result = !TAILQ_EMPTY(sq);
-
-       mutex_spin_exit(mp);
-       return result;
+       return (cv->cv_waiters > 0);
 }
 
 /*
@@ -411,5 +408,11 @@ bool
 cv_is_valid(kcondvar_t *cv)
 {
 
-       return cv->cv_wmesg != deadcv && cv->cv_wmesg != NULL;
+       if (cv->cv_wmesg == deadcv || cv->cv_wmesg == NULL)
+               return false;
+
+       if (cv->cv_waiters < 0)
+               return false;
+
+       return true;
 }
Index: sys/condvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/condvar.h,v
retrieving revision 1.10
diff -u -p -r1.10 condvar.h
--- sys/condvar.h       31 May 2008 13:36:25 -0000      1.10
+++ sys/condvar.h       15 Jun 2008 13:40:35 -0000
@@ -44,6 +44,7 @@
 typedef struct kcondvar {
        void            *cv_opaque[2];  /* sleep queue */
        const char      *cv_wmesg;      /* description for /bin/ps */
+       int             cv_waiters;     /* number of waiters */
 } kcondvar_t;
 
 #ifdef _KERNEL