[src/trunk]: src/sys/kern Fix edge cases in cv_timedwaitbt, cv_timedwaitbt_sig.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/383ad2ed14ec
branches:  trunk
changeset: 932239:383ad2ed14ec
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun May 03 01:19:47 2020 +0000

description:
Fix edge cases in cv_timedwaitbt, cv_timedwaitbt_sig.

- If the timeout is exactly zero, fail immediately with EWOULDBLOCK.

- If the timeout is just so small it would be rounded to zero ticks,
  make sure to wait at least one tick.

- Make sure we never return with a negative timeout left.

diffstat:

 sys/kern/kern_condvar.c |  60 ++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 52 insertions(+), 8 deletions(-)

diffs (116 lines):

diff -r 42253ff10ca6 -r 383ad2ed14ec sys/kern/kern_condvar.c
--- a/sys/kern/kern_condvar.c   Sun May 03 01:06:55 2020 +0000
+++ b/sys/kern/kern_condvar.c   Sun May 03 01:19:47 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_condvar.c,v 1.47 2020/04/19 20:35:29 ad Exp $     */
+/*     $NetBSD: kern_condvar.c,v 1.48 2020/05/03 01:19:47 riastradh Exp $      */
 
 /*-
  * Copyright (c) 2006, 2007, 2008, 2019, 2020 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_condvar.c,v 1.47 2020/04/19 20:35:29 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_condvar.c,v 1.48 2020/05/03 01:19:47 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -334,23 +334,45 @@
 {
        struct bintime slept;
        unsigned start, end;
+       int timo;
        int error;
 
        KASSERTMSG(bt->sec >= 0, "negative timeout");
        KASSERTMSG(epsilon != NULL, "specify maximum requested delay");
 
+       /* If there's nothing left to wait, time out.  */
+       if (bt->sec == 0 && bt->frac == 0)
+               return EWOULDBLOCK;
+
+       /* Convert to ticks, but clamp to be >=1.  */
+       timo = bintime2timo(bt);
+       KASSERTMSG(timo >= 0, "negative ticks: %d", timo);
+       if (timo == 0)
+               timo = 1;
+
        /*
         * getticks() is technically int, but nothing special
         * happens instead of overflow, so we assume two's-complement
         * wraparound and just treat it as unsigned.
         */
        start = getticks();
-       error = cv_timedwait(cv, mtx, bintime2timo(bt));
+       error = cv_timedwait(cv, mtx, timo);
        end = getticks();
 
+       /*
+        * Set it to the time left, or zero, whichever is larger.  We
+        * do not fail with EWOULDBLOCK here because this may have been
+        * an explicit wakeup, so the caller needs to check before they
+        * give up or else cv_signal would be lost.
+        */
        slept = timo2bintime(end - start);
-       /* bt := bt - slept */
-       bintime_sub(bt, &slept);
+       if (bintimecmp(bt, &slept, <=)) {
+               bt->sec = 0;
+               bt->frac = 0;
+       } else {
+               /* bt := bt - slept */
+               bintime_sub(bt, &slept);
+       }
 
        return error;
 }
@@ -377,23 +399,45 @@
 {
        struct bintime slept;
        unsigned start, end;
+       int timo;
        int error;
 
        KASSERTMSG(bt->sec >= 0, "negative timeout");
        KASSERTMSG(epsilon != NULL, "specify maximum requested delay");
 
+       /* If there's nothing left to wait, time out.  */
+       if (bt->sec == 0 && bt->frac == 0)
+               return EWOULDBLOCK;
+
+       /* Convert to ticks, but clamp to be >=1.  */
+       timo = bintime2timo(bt);
+       KASSERTMSG(timo >= 0, "negative ticks: %d", timo);
+       if (timo == 0)
+               timo = 1;
+
        /*
         * getticks() is technically int, but nothing special
         * happens instead of overflow, so we assume two's-complement
         * wraparound and just treat it as unsigned.
         */
        start = getticks();
-       error = cv_timedwait_sig(cv, mtx, bintime2timo(bt));
+       error = cv_timedwait_sig(cv, mtx, timo);
        end = getticks();
 
+       /*
+        * Set it to the time left, or zero, whichever is larger.  We
+        * do not fail with EWOULDBLOCK here because this may have been
+        * an explicit wakeup, so the caller needs to check before they
+        * give up or else cv_signal would be lost.
+        */
        slept = timo2bintime(end - start);
-       /* bt := bt - slept */
-       bintime_sub(bt, &slept);
+       if (bintimecmp(bt, &slept, <=)) {
+               bt->sec = 0;
+               bt->frac = 0;
+       } else {
+               /* bt := bt - slept */
+               bintime_sub(bt, &slept);
+       }
 
        return error;
 }