On 16/11/21(Tue) 13:55, Visa Hankala wrote:
> Currently, dopselect() and doppoll() call tsleep_nsec() without retry.
> cheloha@ asked if the functions should handle spurious wakeups. I guess
> such wakeups are unlikely with the nowake wait channel, but I am not
> sure if that is a safe guess.
I'm not sure to understand, are we afraid a thread sleeping on `nowake'
can be awaken? Is it the assumption here?
> The following diff adds the retrying. The code is a bit arduous, so the
> retry loop is put in a separate function that both poll and select use.
Using a separate function makes sense anyway.
> Index: kern/sys_generic.c
> ===================================================================
> RCS file: src/sys/kern/sys_generic.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 sys_generic.c
> --- kern/sys_generic.c 16 Nov 2021 13:48:23 -0000 1.141
> +++ kern/sys_generic.c 16 Nov 2021 13:50:08 -0000
> @@ -90,6 +90,7 @@ int dopselect(struct proc *, int, fd_set
> int doppoll(struct proc *, struct pollfd *, u_int, struct timespec *,
> const sigset_t *, register_t *);
> void doselwakeup(struct selinfo *);
> +int selsleep(struct timespec *);
>
> int
> iovec_copyin(const struct iovec *uiov, struct iovec **iovp, struct iovec
> *aiov,
> @@ -664,19 +665,7 @@ dopselect(struct proc *p, int nd, fd_set
> * there's nothing to wait for.
> */
> if (nevents == 0 && ncollected == 0) {
> - uint64_t nsecs = INFSLP;
> -
> - if (timeout != NULL) {
> - if (!timespecisset(timeout))
> - goto done;
> - nsecs = MAX(1, MIN(TIMESPEC_TO_NSEC(timeout), MAXTSLP));
> - }
> - error = tsleep_nsec(&nowake, PSOCK | PCATCH, "kqsel", nsecs);
> - /* select is not restarted after signals... */
> - if (error == ERESTART)
> - error = EINTR;
> - if (error == EWOULDBLOCK)
> - error = 0;
> + error = selsleep(timeout);
> goto done;
> }
>
> @@ -849,6 +838,46 @@ selfalse(dev_t dev, int events, struct p
> }
>
> /*
> + * Sleep until a signal arrives or the optional timeout expires.
> + */
> +int
> +selsleep(struct timespec *timeout)
> +{
> + uint64_t end, now, nsecs;
> + int error;
> +
> + if (timeout != NULL) {
> + if (!timespecisset(timeout))
> + return (0);
> + now = getnsecuptime();
> + end = MIN(now + TIMESPEC_TO_NSEC(timeout), MAXTSLP);
> + if (end < now)
> + end = MAXTSLP;
> + }
> +
> + do {
> + if (timeout != NULL)
> + nsecs = MAX(1, end - now);
> + else
> + nsecs = INFSLP;
> + error = tsleep_nsec(&nowake, PSOCK | PCATCH, "selslp", nsecs);
> + if (timeout != NULL) {
> + now = getnsecuptime();
> + if (now >= end)
> + break;
> + }
> + } while (error == 0);
> +
> + /* poll/select is not restarted after signals... */
> + if (error == ERESTART)
> + error = EINTR;
> + if (error == EWOULDBLOCK)
> + error = 0;
> +
> + return (error);
> +}
> +
> +/*
> * Record a select request.
> */
> void
> @@ -1158,19 +1187,7 @@ doppoll(struct proc *p, struct pollfd *f
> * there's nothing to wait for.
> */
> if (nevents == 0 && ncollected == 0) {
> - uint64_t nsecs = INFSLP;
> -
> - if (timeout != NULL) {
> - if (!timespecisset(timeout))
> - goto done;
> - nsecs = MAX(1, MIN(TIMESPEC_TO_NSEC(timeout), MAXTSLP));
> - }
> -
> - error = tsleep_nsec(&nowake, PSOCK | PCATCH, "kqpoll", nsecs);
> - if (error == ERESTART)
> - error = EINTR;
> - if (error == EWOULDBLOCK)
> - error = 0;
> + error = selsleep(timeout);
> goto done;
> }
>
>
