Create a dedicated lock for zombproc to ligthen the load on allproc_lock
AbandonedPublic
Actions

Authored by mjg on Oct 31 2018, 2:31 AM.

Details

Reviewers

Summary

Currently both allproc and proctree are heavily intertwined and overloaded e.g. during -j 128 package builds. This patch removes one common proctree -> allproc consumer (in the reaping code).

I did not bother benchmarking this patch in isolation.

With this take care of there are 2 proctree -> allproc places remaining. One (in the exit path) can be trivially fixed with moving the op outside of proctree, another one on fork can be fixed with bitmaps for ID maintenance (which removes the need to walk the session and group ids, in turn not requiring proctree).

Test Plan

poudriere

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mjg created this revision.Oct 31 2018, 2:31 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 31 2018, 2:31 AM

Harbormaster completed remote builds in B20529: Diff 49816.Oct 31 2018, 2:31 AM

mjg edited the summary of this revision. (Show Details)Oct 31 2018, 2:32 AM

kib added inline comments.Nov 1 2018, 12:16 AM

sys/kern/kern_fork.c
306	So can this 'goto retry' cause the code to re-enter the block with doingzomb = 0 but zombproc_lock owned ?

mjg marked an inline comment as done.Nov 1 2018, 7:16 AM

mjg added inline comments.

sys/kern/kern_fork.c
306	right, should not have missed that. There is a not trivial to fix and pesky problem elsewhere: currently the process traversal for ps/top is done with the allproc lock held the entire time. With this change there is a time window where a previously process can become a zombie and will later by found in the zombproc list, thus will end up reported twice. I'm abandoning the change for now. Changing the traversal to use the proc hash instead deals with the problem and may be good enough to not warrant introduction of zombproc lock in the first place.