Page MenuHomeFreeBSD
Differential D58313

kqueue: Add NOTE_REAP
ClosedPublic
Actions

Authored by des on Fri, Jul 17, 8:10 PM.
Tags
None
Referenced Files
F164483253: D58313.id182768.diff
Sat, Aug 1, 7:56 AM
F164460575: D58313.id182433.diff
Sat, Aug 1, 5:12 AM
Unknown Object (File)
Fri, Jul 31, 7:36 PM
Unknown Object (File)
Fri, Jul 31, 10:38 AM
Unknown Object (File)
Fri, Jul 31, 6:40 AM
Unknown Object (File)
Fri, Jul 31, 5:49 AM
Unknown Object (File)
Thu, Jul 30, 5:41 PM
Unknown Object (File)
Thu, Jul 30, 5:54 AM
View All 41 Files
Subscribers
dch
imp
kevans
olce
ziaee

Details

Reviewers
kib
markj
kevans
Group Reviewers
Klara
Commits
rG2bacbbecb165: kqueue: Add NOTE_REAP
Summary

Add a NOTE_REAP event for EVFILTER_PROC which provides a notification
when the process is reaped.

MFC after: 1 week
Sponsored by: Klara, Inc.
Sponsored by: NetApp, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

des created this revision.Fri, Jul 17, 8:10 PM
des held this revision as a draft.
Herald added subscribers: ziaee, olce, imp. · View Herald TranscriptFri, Jul 17, 8:10 PM
Harbormaster completed remote builds in B74899: Diff 182154.Fri, Jul 17, 8:10 PM
des added a child revision: D58314: pwait: Optionally wait until process is reaped.Fri, Jul 17, 8:10 PM
kib mentioned this in D58314: pwait: Optionally wait until process is reaped.Sun, Jul 19, 5:54 PM
dch added a subscriber: dch.Mon, Jul 20, 9:40 AM
des published this revision for review.Tue, Jul 21, 7:51 PM
des added reviewers: kib, markj.

taking out of draft since I don't get notifications for replies to drafts

des added a reviewer: kevans.Tue, Jul 21, 8:11 PM
des added a subscriber: kevans.

I just tested this patch again for the first time since February and to my surprise, it now seems to work. Perhaps some of the work @kevans and @kib have done in this area since then fixed whatever issue was preventing it from working. Please review...

kevans added inline comments.Tue, Jul 21, 10:13 PM
sys/kern/kern_event.c
2249

Why move this out of the flux segment?

des added inline comments.Tue, Jul 21, 11:01 PM
sys/kern/kern_event.c
2249

We set the KN_SCAN bit before entering flux, we should clear it after leaving flux. I suppose it makes no difference in practice since in both cases kq is locked throughout, but this seemed cleaner to me.

des updated this revision to Diff 182433.Wed, Jul 22, 12:23 AM

update

Harbormaster completed remote builds in B74993: Diff 182433.Wed, Jul 22, 12:23 AM
des edited the summary of this revision. (Show Details)Wed, Jul 22, 8:16 AM
des updated this revision to Diff 182449.Wed, Jul 22, 10:36 AM
des edited the summary of this revision. (Show Details)

remove unnecessary change

Harbormaster completed remote builds in B75002: Diff 182449.Wed, Jul 22, 10:36 AM
des marked an inline comment as done.Wed, Jul 22, 10:36 AM
kib accepted this revision.Wed, Jul 22, 11:48 PM
kib added inline comments.
lib/libsys/kqueue.2
547

I think some explanation what 'reaped' means is useful.

sys/kern/kern_event.c
448
This revision is now accepted and ready to land.Wed, Jul 22, 11:48 PM
des added inline comments.Thu, Jul 23, 7:41 AM
lib/libsys/kqueue.2
547

Why? It's established terminology. I prefer “collected” but “reaped” is what it's been called for 40 or 50 years, cf. procctl(2), waitpid(2) etc.

sys/kern/kern_event.c
448

Better to do a sweep later.

markj accepted this revision.Thu, Jul 23, 3:37 PM
markj added inline comments.
lib/libsys/kqueue.2
547

You might at least refer to wait(2), e.g., "The process has been reaped by a call to the wait(2) family of functions."

des marked 2 inline comments as done.Thu, Jul 23, 4:36 PM
des updated this revision to Diff 182543.Thu, Jul 23, 5:46 PM

rf

This revision now requires review to proceed.Thu, Jul 23, 5:46 PM
Harbormaster completed remote builds in B75048: Diff 182543.Thu, Jul 23, 5:46 PM
des updated this revision to Diff 182544.Thu, Jul 23, 5:46 PM

rf

Harbormaster completed remote builds in B75049: Diff 182544.Thu, Jul 23, 5:46 PM
des added a reviewer: Klara.Thu, Jul 23, 5:49 PM
kib accepted this revision.Thu, Jul 23, 10:49 PM
kib added inline comments.
sys/kern/kern_event.c
473

I think that EVFILT_PROC and EVFILT_PROCDESC need a separate PCTRLMASKs, I will handle this afterward.

This revision is now accepted and ready to land.Thu, Jul 23, 10:49 PM
des planned changes to this revision.Fri, Jul 24, 1:33 PM

I just noticed that NOTE_REAP aliases NOTE_SIGNAL. Testing a fix.

des updated this revision to Diff 182620.Fri, Jul 24, 4:28 PM

don't alias NOTE_SIGNAL

Harbormaster completed remote builds in B75097: Diff 182620.Fri, Jul 24, 4:28 PM
markj accepted this revision.Fri, Jul 24, 7:51 PM
markj added inline comments.
sys/sys/event.h
212–215
This revision is now accepted and ready to land.Fri, Jul 24, 7:51 PM
kib accepted this revision.Fri, Jul 24, 7:55 PM
Closed by commit rG2bacbbecb165: kqueue: Add NOTE_REAP (authored by des). · Explain WhyMon, Jul 27, 10:17 AM
This revision was automatically updated to reflect the committed changes.
des added a commit: rG2bacbbecb165: kqueue: Add NOTE_REAP.

Revision Contents
Changeset List

PathSize
lib/
libsys/
10 lines
libsysdecode/
2 lines
sys/
kern/
13 lines
1 line
9 lines
sys/
11 lines

Diff 182768

View Options

lib/libsys/kqueue.2

Loading...
View Options

lib/libsysdecode/flags.c

Loading...
View Options

sys/kern/kern_event.c

Loading...
View Options

sys/kern/kern_exit.c

Loading...
View Options

sys/kern/sys_procdesc.c

Loading...
View Options

sys/sys/event.h

Loading...