Re: add clock_nanosleep(2)

2018-05-27 Thread Martin Pieuchot
On 26/05/18(Sat) 17:49, Scott Cheloha wrote:
> Index: sys/kern/kern_synch.c
> ===
> RCS file: /cvs/src/sys/kern/kern_synch.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 kern_synch.c
> --- sys/kern/kern_synch.c 24 Apr 2018 16:28:42 -  1.144
> +++ sys/kern/kern_synch.c 26 May 2018 22:42:21 -
> @@ -236,31 +236,32 @@ msleep(const volatile void *ident, struc
>   * entered the sleep queue we drop the it. After sleeping we re-lock.
>   */
>  int
> -rwsleep(const volatile void *ident, struct rwlock *wl, int priority,
> +rwsleep(const volatile void *ident, struct rwlock *rwl, int priority,
>  const char *wmesg, int timo)
>  {
>   struct sleep_state sls;
> - int error, error1;
> + int error, error1, status;
>   WITNESS_SAVE_DECL(lock_fl);
>  
>   KASSERT((priority & ~(PRIMASK | PCATCH | PNORELOCK)) == 0);
> - rw_assert_wrlock(wl);
> + rw_assert_anylock(rwl);
> + status = rw_status(rwl);
>  
>   sleep_setup(, ident, priority, wmesg);
>   sleep_setup_timeout(, timo);
>   sleep_setup_signal(, priority);
>  
> - WITNESS_SAVE(>rwl_lock_obj, lock_fl);
> + WITNESS_SAVE(>rwl_lock_obj, lock_fl);
>  
> - rw_exit_write(wl);
> + rw_exit(rwl);
>  
>   sleep_finish(, 1);
>   error1 = sleep_finish_timeout();
>   error = sleep_finish_signal();
>  
>   if ((priority & PNORELOCK) == 0) {
> - rw_enter_write(wl);
> - WITNESS_RESTORE(>rwl_lock_obj, lock_fl);
> + rw_enter(rwl, status);
> + WITNESS_RESTORE(>rwl_lock_obj, lock_fl);
>   }
>  
>   /* Signal errors are higher priority than timeouts. */

The rwsleep() and corresponding man page changes are ok with me and can
already be committed.



add clock_nanosleep(2)

2018-05-26 Thread Scott Cheloha
Hi,

Here's a first attempt at adding clock_nanosleep(2).

Its use cases are admittedly more niche than nanosleep(2),
and indeed there are just a handful of spots in base where
I think it would be an improvement over existing code, but
it's definitely an improvement, particularly in simplicity,
where applicable.

This implementation introduces a rwlock in tc_setrealtimeclock()
and wakes threads when it exits the critical section, so absolute
CLOCK_REALTIME timeouts never miss a clock jump.

I feel that this is more of a hack than adjusting timeouts from
hardclock(9), and something like that that will probably be the
better solution if other absolute CLOCK_REALTIME timeouts are made
sensitive to jumps in the future, but I wanted to start here with
something minimally invasive.  I'm still unsure if making absolute
timeouts sensitive to clock jumps is even worth it, particularly in
cases where there is no portable alternative clock choice, like with
pthread_mutex_timedlock(3).

(Maybe of note is that this was guenther@'s approach several years
ago [1], but that patch did not land.)

... but with clock_nanosleep(2) you can use any clock, so I think
it's a perfectly valid feature with no real downsides to inclusion.

The patch also generalizes rwsleep(9) to cover both write and
read locks.  This seems like an obvious extension preferable to
adding yet another *sleep() wrapper just for read locks.

The patch puts the sleeps in tick-loops to handle arbitrary
timeouts for both nanosleep(2) and clock_nanosleep(2), removing
nanosleep's 100-million second upper bound.  Starting here, I want
to remove the 100-million second upper bound from system calls with
timeouts because:

  (a) It's a lie: 100 million seconds at 100hz is way, way
  more ticks than a signed 32-bit int can represent.

  (b) It's one less platform-specific thing for userspace to
  check; particularly cross-platform high-level languages,
  effectively all of which have an interface to nanosleep(2).

  Lots of sleeps don't check the return code: eliminating
  the 100-million second gotcha case makes debugging and
  reasoning about code easier.

Looking for feedback that I can spruce things up with and then
ask for OKs later (assuming there's no objection to including
the interface itself).

Thoughts & feedback?

--
Scott Cheloha

[1] https://marc.info/?l=openbsd-tech=128245846500419=2

Index: include/time.h
===
RCS file: /cvs/src/include/time.h,v
retrieving revision 1.30
diff -u -p -r1.30 time.h
--- include/time.h  5 Sep 2017 03:16:13 -   1.30
+++ include/time.h  26 May 2018 22:42:20 -
@@ -165,6 +165,7 @@ int nanosleep(const struct timespec *, s
 
 #if __POSIX_VISIBLE >= 200112
 int clock_getcpuclockid(pid_t, clockid_t *);
+int clock_nanosleep(clockid_t, int, const struct timespec *, struct timespec 
*);
 #endif
 
 #if __POSIX_VISIBLE >= 200809
Index: lib/libc/Symbols.list
===
RCS file: /cvs/src/lib/libc/Symbols.list,v
retrieving revision 1.62
diff -u -p -r1.62 Symbols.list
--- lib/libc/Symbols.list   5 Dec 2017 13:45:31 -   1.62
+++ lib/libc/Symbols.list   26 May 2018 22:42:21 -
@@ -58,6 +58,7 @@ _thread_sys_chown
 _thread_sys_chroot
 _thread_sys_clock_getres
 _thread_sys_clock_gettime
+_thread_sys_clock_nanosleep
 _thread_sys_clock_settime
 _thread_sys_close
 _thread_sys_closefrom
@@ -254,6 +255,7 @@ chown
 chroot
 clock_getres
 clock_gettime
+clock_nanosleep
 clock_settime
 close
 closefrom
Index: lib/libc/hidden/time.h
===
RCS file: /cvs/src/lib/libc/hidden/time.h,v
retrieving revision 1.5
diff -u -p -r1.5 time.h
--- lib/libc/hidden/time.h  5 Sep 2017 03:16:13 -   1.5
+++ lib/libc/hidden/time.h  26 May 2018 22:42:21 -
@@ -30,6 +30,7 @@ PROTO_STD_DEPRECATED(clock);
 PROTO_DEPRECATED(clock_getcpuclockid);
 PROTO_NORMAL(clock_getres);
 PROTO_NORMAL(clock_gettime);
+PROTO_CANCEL(clock_nanosleep);
 PROTO_NORMAL(clock_settime);
 PROTO_STD_DEPRECATED(ctime);
 PROTO_DEPRECATED(ctime_r);
Index: lib/libc/sys/Makefile.inc
===
RCS file: /cvs/src/lib/libc/sys/Makefile.inc,v
retrieving revision 1.154
diff -u -p -r1.154 Makefile.inc
--- lib/libc/sys/Makefile.inc   12 Jan 2018 04:36:12 -  1.154
+++ lib/libc/sys/Makefile.inc   26 May 2018 22:42:21 -
@@ -27,7 +27,7 @@ SRCS+=canceled.c
 
 # syscalls that would be normal...except for cancellation or SIGTHR
 CANCEL=accept accept4 \
-   close closefrom connect \
+   clock_nanosleep close closefrom connect \
fcntl fsync \
msgrcv msgsnd msync \
nanosleep \
@@ -175,8 +175,8 @@ ${HIDDEN}:  ; @${GENERATE.rsyscall_hidde
 
 MAN+=  __get_tcb.2 __thrsigdivert.2 __thrsleep.2 _exit.2 accept.2 \
access.2 acct.2 adjfreq.2