On 23/12/20(Wed) 07:18, Visa Hankala wrote:
> This fixes a recent regression in kqueue_scan() where the function can
> mistakenly return EWOULDBLOCK.
>
> Currently, kqueue_scan() does one more scan attempt after a timeout.
> Usually, this gives no new events and the function bails out through
> the following code. Note that it clears `error'.
>
> if (kq->kq_count == 0) {
> /*
> * Successive loops are only necessary if there are more
> * ready events to gather, so they don't need to block.
> */
> if ((tsp != NULL && !timespecisset(tsp)) ||
> scan->kqs_nevent != 0) {
> splx(s);
> error = 0;
> goto done;
> }
>
> However, there can be a last-minute event activation, in which case the
> function processes the event queue. Unfortunately, the error variable
> preserves its value EWOULDBLOCK/EAGAIN that gets returned to the caller.
> kevent(2), or select(2) or poll(2), is not supposed to return this error.
>
> The issue emerged in r1.146 of kern_event.c when the final copyout() was
> moved outside kqueue_scan(). The copyout()'s return value used to
> override the EWOULDBLOCK.
>
> The following patch fixes the regression by clearing `error' at the
> start of each scan round. The clearing could be done conditionally after
> kqueue_sleep(). However, that does not seem as robust.
The value could be cleaned before "goto retry", that would be more
robust, no?
Ok mpi@ either way.
> Index: kern/kern_event.c
> ===
> RCS file: src/sys/kern/kern_event.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 kern_event.c
> --- kern/kern_event.c 20 Dec 2020 12:54:05 - 1.153
> +++ kern/kern_event.c 23 Dec 2020 07:10:24 -
> @@ -977,6 +977,8 @@ kqueue_scan(struct kqueue_scan_state *sc
> retry:
> KASSERT(nkev == 0);
>
> + error = 0;
> +
> if (kq->kq_state & KQ_DYING) {
> error = EBADF;
> goto done;
>
