kevent behavior

[Jilles Tjoelker]

On Fri, Mar 27, 2015 at 03:26:54PM +0200, Konstantin Belousov wrote:
> On Thu, Mar 26, 2015 at 11:58:27PM +0100, Jilles Tjoelker wrote:
> > On Thu, Mar 26, 2015 at 11:45:51PM +0200, Konstantin Belousov wrote:
> > > On Wed, Mar 25, 2015 at 11:35:30PM +0100, Jilles Tjoelker wrote:
> > 
> > > > POSIX does not say anything about kevent(), including whether it should
> > > > be a cancellation point or not. Given that kevent() may block for a long
> > > > time, making it a cancellation point seems to make sense.
> > 
> > > But POSIX language is very explicit for its own set of interfaces, and
> > > that makes sense.  This is why I think that cancellability, after the
> > > 15+ years of kqueue existence, should be opt in.

> > Hmm, I think most code written is cancel-unsafe. It is unlikely to have
> > cancel-safe code using kevent() and relying on it not being a
> > cancellation point, but still possible.
> Ok, I re-considered my opinion after re-reading your responses.

> > This also means we shouldn't wait too long with adding missing
> > cancellation points like ppoll().

> > > > The kevent() cancellation point implementation would be much like most
> > > > other cancellation points such as pselect(), with the difference that
> > > > the call may have committed even if it fails, in the case that
> > > > nchanges > nevents and in the case that nchanges > 0 && errno == EINTR.
> > > > If cancellation is requested while blocking in the latter case, libthr
> > > > will have to return -1/EINTR to indicate to the application that the
> > > > changes were applied successfully while allowing the thread to be
> > > > cancelled at the next cancellation point, even though there may not be
> > > > any signal handler.

> I do not quite understand this.

> If any error (except things like EFAULT) occured while processing the
> changelist, kevent(2) returns error count, and not the -1, regarless of
> the length of eventlist.  In this case, we do not cancel.

kevent(2) has two ways of reporting an error while processing the
changelist. If there is enough space in eventlist, it reports the error
there and continues processing changelist; if there is no space, it
reports the error via errno and stops processing changelist.

> Syscall cannot return n/EINTR, where n >= 0.

> If -1/EINTR is returned, this means that the call blocked and sleep
> was interrupted.  The changes from the changelist were applied, 
> do you suggested that we must not cancel if nchanges > 0 ?

Yes, so that the application knows the kqueue's state when the thread is
cancelled.

> > > > If nevents == 0, kevent() does not block and need not be a cancellation
> > > > point. This special case may simplify things slightly for application
> > > > programmers.

> > > > A kqueue() flag for cancellation does not seem very useful: since such a
> > > > flag would be a kernel thing and pthread cancellation is a libthr thing,
> > > > requesting the flag from the kernel would slow down all kevent() calls.

> > > Yes, I thought about adding some (small) userspace table which would
> > > track cancellable kqueues.

> > I think the interaction with dup() and similar calls and with exec makes
> > this too nasty.

> Hm, yes.

> > > But if we make the cancellation state per-call, and not a state
> > > for kqueue descriptor, we might indicate the cancellability by
> > > some event type, which is not passed to the kernel at all.  The
> > > libthr wrapper for kevent(2) would need to scan the changelist and
> > > zero-out the cancellation indicator.

> > This seems a bit hackish. However, enabling cancellation per-call seems
> > better than per-kqueue.

> Patch below always considers kevent(2) as cancellable, unless nevents == 0.
> I will correct the call to thr_cancel_leave() if I misunderstood you.

> [snip]
> +.Sh CANCELLATION BEHAVIOUR
> +If
> +.Fa nevents
> +is non-zero, i.e. the function is potentially blocking, the call
> +is the cancellation point.

"is a cancellation point."

> +Otherwise, i.e. if
> +.Fa nevents
> +is zero, the call is not cancellable.

> +Cancellation can only occur when the call was blocked and no changes
> +to the queue were requested.

This is not how the code works, but I think the code is correct. The
code also allows cancellation before anything happens. As such, if
cancellation occurs at kevent(2), the kqueue's state is unchanged (from
that call).

A new error should be added to the man page:

.It Bq Er EINTR
A cancellation request was delivered to the thread, but not yet handled.

Both [EINTR] errors should have this sentence added to them:

All changes in the
.Fa changelist
have been applied.

-- 
Jilles Tjoelker