]> granicus.if.org Git - zfs/commitdiff
Fix cv_timedwait timeout
authorBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 25 May 2017 17:01:44 +0000 (10:01 -0700)
committerGitHub <noreply@github.com>
Thu, 25 May 2017 17:01:44 +0000 (10:01 -0700)
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

module/spl/spl-condvar.c

index 479bbfd1234896a63b714398105c8b187dbcf1d3..80c2ef09051fd79129ae20d14bf33ae46f8852ea 100644 (file)
@@ -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));
-       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);
 
-       /* 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);
 
@@ -238,28 +235,25 @@ __cv_timedwait_hires(kcondvar_t *cvp, kmutex_t *mp, hrtime_t expire_time,
 {
        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));
-       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);
 
-       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);