Page MenuHomeFreeBSD

proc: always store parent pid in p_oppid
ClosedPublic

Authored by mjg on Nov 3 2018, 5:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 5:59 PM
Unknown Object (File)
Thu, Nov 14, 4:07 PM
Unknown Object (File)
Wed, Nov 13, 10:59 AM
Unknown Object (File)
Wed, Nov 13, 10:21 AM
Unknown Object (File)
Wed, Nov 13, 5:23 AM
Unknown Object (File)
Mon, Nov 11, 2:27 PM
Unknown Object (File)
Sun, Nov 10, 2:01 PM
Unknown Object (File)
Sun, Nov 10, 1:15 PM
Subscribers

Details

Summary

Doing so removes the dependency on proctree lock from sysctl process list export which further reduces contention during poudriere -j 128 runs.

While here adjust getppid.

Test Plan

kyua, poudriere

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

The patch seems to be generated on top of D17817 and is not applicable.

I do not like proc_reparent_ptrace() addition, just expand the signature of proc_reparent() with the new arg.

sys/kern/kern_exit.c
115

Now that the condition is simplified, it can be written as

return (child->p_pptr->p_pid == child->p_oppid ? child->p_pptr : initproc);
sys/kern/kern_prot.c
126–127

ppid is not needed. Just

return (p->p_oppid);

is enough.

sys/sys/proc.h
598

The comment needs updating, it no longer has anything to do with ptrace.

  • address feedback
  • regen against head
  • i did not change the condition in proc_realparent as it goes way over the 80 char limit
sys/kern/kern_exit.c
111

I think it really useful to add MPASS(child->p_oppid != 0) asserts at all places where old chode checked for p_oppid == 0.

115

The line would just wrap, I still think that saving three lines is good.

117

Can this list walk replaced by lookup by p_oppid ?

mjg marked 3 inline comments as done.Nov 13 2018, 7:23 PM
mjg added inline comments.
sys/kern/kern_exit.c
111

we can't easily assert here. the original condition has ||. I don't think it adds much to the other place.

115

well i can put this if you insist:

+       if ((child->p_treeflag & P_TREE_ORPHANED) == 0)
+               return (child->p_pptr->p_pid == child->p_oppid ?
+                           child->p_pptr : initproc);

I'm temporarily on a very slow connection and can't upload the 300KB+ diff arc requires without timing out

117

it would require adding allproc here. (or the relevant hash lock after they get added later). i think this is a consideration unrelated to the patch.

sys/kern/kern_exit.c
111

I do not understand why. It should be non-conditional case now.

115

Assuming the continuation line uses +4 spaces, this looks fine.

This revision is now accepted and ready to land.Nov 16 2018, 2:03 PM
This revision was automatically updated to reflect the committed changes.