Fix cv_timedwait timeout

Perform the already past expiration time check before updating
cvp->cv_mutex with the provided mutex.  This check only depends
on local state.  Doing it first ensures that cvp->cv_mutex will not
be updated in the timeout case or if it's ever called with an
expire_time <= now.

Reviewed-by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #616

diff --git a/module/spl/spl-condvar.c b/module/spl/spl-condvar.c
index 479bbfd1234896a63b714398105c8b187dbcf1d3..80c2ef09051fd79129ae20d14bf33ae46f8852ea 100644 (file)
--- a/module/spl/spl-condvar.c
+++ b/module/spl/spl-condvar.c
@@ -166,22 +166,19 @@ __cv_timedwait_common(kcondvar_t *cvp, kmutex_t *mp, clock_t expire_time,
        ASSERT(mp);
        ASSERT(cvp->cv_magic == CV_MAGIC);
        ASSERT(mutex_owned(mp));
        ASSERT(mp);
        ASSERT(cvp->cv_magic == CV_MAGIC);
        ASSERT(mutex_owned(mp));

-       atomic_inc(&cvp->cv_refs);

 
 

+       /* XXX - Does not handle jiffie wrap properly */
+       time_left = expire_time - jiffies;
+       if (time_left <= 0)
+               return (-1);
+
+       atomic_inc(&cvp->cv_refs);

        m = ACCESS_ONCE(cvp->cv_mutex);
        if (!m)
                m = xchg(&cvp->cv_mutex, mp);
        /* Ensure the same mutex is used by all callers */
        ASSERT(m == NULL || m == mp);
 
        m = ACCESS_ONCE(cvp->cv_mutex);
        if (!m)
                m = xchg(&cvp->cv_mutex, mp);
        /* Ensure the same mutex is used by all callers */
        ASSERT(m == NULL || m == mp);
 

-       /* XXX - Does not handle jiffie wrap properly */
-       time_left = expire_time - jiffies;
-       if (time_left <= 0) {
-               /* XXX - doesn't reset cv_mutex */
-               atomic_dec(&cvp->cv_refs);
-               return (-1);
-       }
-

        prepare_to_wait_exclusive(&cvp->cv_event, &wait, state);
        atomic_inc(&cvp->cv_waiters);
 
        prepare_to_wait_exclusive(&cvp->cv_event, &wait, state);
        atomic_inc(&cvp->cv_waiters);
 


@@ -238,28 +235,25 @@ __cv_timedwait_hires(kcondvar_t *cvp, kmutex_t *mp, hrtime_t expire_time,
 {
        DEFINE_WAIT(wait);
        kmutex_t *m;
 {
        DEFINE_WAIT(wait);
        kmutex_t *m;

-       hrtime_t time_left, now;
+       hrtime_t time_left;

        ktime_t ktime_left;
 
        ASSERT(cvp);
        ASSERT(mp);
        ASSERT(cvp->cv_magic == CV_MAGIC);
        ASSERT(mutex_owned(mp));
        ktime_t ktime_left;
 
        ASSERT(cvp);
        ASSERT(mp);
        ASSERT(cvp->cv_magic == CV_MAGIC);
        ASSERT(mutex_owned(mp));

-       atomic_inc(&cvp->cv_refs);

 
 

+       time_left = expire_time - gethrtime();
+       if (time_left <= 0)
+               return (-1);
+
+       atomic_inc(&cvp->cv_refs);

        m = ACCESS_ONCE(cvp->cv_mutex);
        if (!m)
                m = xchg(&cvp->cv_mutex, mp);
        /* Ensure the same mutex is used by all callers */
        ASSERT(m == NULL || m == mp);
 
        m = ACCESS_ONCE(cvp->cv_mutex);
        if (!m)
                m = xchg(&cvp->cv_mutex, mp);
        /* Ensure the same mutex is used by all callers */
        ASSERT(m == NULL || m == mp);
 

-       now = gethrtime();
-       time_left = expire_time - now;
-       if (time_left <= 0) {
-               atomic_dec(&cvp->cv_refs);
-               return (-1);
-       }
-

        prepare_to_wait_exclusive(&cvp->cv_event, &wait, state);
        atomic_inc(&cvp->cv_waiters);
 
        prepare_to_wait_exclusive(&cvp->cv_event, &wait, state);
        atomic_inc(&cvp->cv_waiters);