Page MenuHomeFreeBSD

ctld: Use kevent(2) for socket events rather than select(2)
Needs ReviewPublic

Authored by jhb on Wed, Jan 22, 7:53 PM.
Tags
None
Referenced Files
F108600824: D48597.id149721.diff
Sun, Jan 26, 7:31 PM
Unknown Object (File)
Thu, Jan 23, 9:12 PM
Unknown Object (File)
Wed, Jan 22, 9:03 PM
Unknown Object (File)
Wed, Jan 22, 8:17 PM
Subscribers

Details

Reviewers
trasz
asomers
mav

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 61890
Build 58774: arc lint + arc unit

Event Timeline

usr.sbin/ctld/ctld.c
2411

I considered replacing the signal handlers with EVFILT_SIGNAL. I might still do a later commit to do that for most cases, but the SIGALARM / setitimer case is used to interrupt a connect() call if it takes too long and EVFILT_TIMER wouldn't raise an interruption, so that one needs to stay as a signal at least. SIGHUP, SIGTERM, etc. would probably be fine though to be ignored but raise EVFILT_SIGNAL events that popped out here.

usr.sbin/ctld/ctld.c
2411

BTW, there's a problem with the way that the current logic handles SIGCHLD: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=284282 . I'll fix that one myself if I find the time.

2414

Why are you changing the "return" to "continue" here?

usr.sbin/ctld/ctld.c
2411

Huh, yeah, I noticed that the code looked really weird to me with how it handled pidfile. I guess switching to EVFILT_SIGNAL and handling the signal events in main_loop would "fix" that bug since main_loop wouldn't get reentered?

2414

If the signal is one of the caught signals it will notice at the top of the loop anyway? I think it really doesn't matter. Eventually if this used EVFILT_SIGNAL the outer loop logic would move into here anyway but it could switch to continue at that time? Either way gives the same result. (I did start on the EVFILT_SIGNAL thing before backing off, so maybe that was in my mind?)

usr.sbin/ctld/ctld.c
2387

I forgot to mention, this call is the main reason we can't easily use EVFILT_SIGNAL for signals other than SIGALRM. We would need a kevent we could register that would return when there is a pending connection to accept via the in-kernel proxy. There is some precedent for having the kernel create new events that aren't registered by userland directly, e.g. EVFILT_AIO is generated by the aio routines without having been added by a call to kevent(2) from userland. We could also just use a d_kqfilter method for /dev/ctl though as I think we'd have to have a new EVFILT_CTL regardless and perhaps use fflags (NOTE_*) to choose which CTL events we care about. For now we would just define a single flag for this event.

usr.sbin/ctld/ctld.c
2740–2743

Won't moving it after daemonisation hide possible error output?

usr.sbin/ctld/ctld.c
2740–2743

Yes it will. But I don't see a good alternative, given that a kq is not inherited after fork.

mav added inline comments.
usr.sbin/ctld/ctld.c
2740–2743

About that I can't comment, never used it, so I'll resign here.

usr.sbin/ctld/ctld.c
2740–2743

Yes, this was due to the requirement of kqueue. The errors do still get logged to syslog, so they aren't entirely invisible.