Add kern.racct.enable tunable and RACCT_DISABLED config option.
ClosedPublic
Actions

Authored by trasz on Apr 25 2015, 5:35 PM.

Details

Reviewers

kib
emaste

Group Reviewers

manpages

Commits

rS282213: Add kern.racct.enable tunable and RACCT_DISABLED config option.

Summary

Add kern.racct.enable tunable and RACCT_DISABLED config option. The point of this is to be able to add RACCT (with RACCT_DISABLED) to GENERIC, to avoid having to rebuild the kernel to use rctl(8).

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Event Timeline

trasz updated this revision to Diff 5002.Apr 25 2015, 5:35 PM

trasz retitled this revision from to Add kern.racct.enable tunable and RACCT_DISABLED config option..

trasz updated this object.

trasz edited the test plan for this revision. (Show Details)

Herald added a reviewer: manpages. · View Herald TranscriptApr 25 2015, 5:35 PM

Herald added a subscriber: imp. · View Herald Transcript

trasz updated this object.Apr 25 2015, 5:36 PM

trasz added reviewers: kib, emaste.

kib added inline comments.Apr 25 2015, 6:26 PM

sys/kern/kern_racct.c
323	(Random place). I do not see much value in these asserts, are you trying to catch missed #ifdef RACCT braces ? If keeping them around, I suggest making the whole statement a macros, like ASSERT_RACCT_ENABLED();
sys/vm/vm_pageout.c
1792	Better put only the last argument under the #ifdef, otherwise it is quite confusing.

Add ASSERT_RACCT_ENABLED() and reword the conditional in vm_pageout.c.

kib accepted this revision.Apr 27 2015, 3:06 PM

kib edited edge metadata.

This revision is now accepted and ready to land.Apr 27 2015, 3:06 PM

Closed by commit rS282213: Add kern.racct.enable tunable and RACCT_DISABLED config option. (authored by trasz). · Explain WhyApr 29 2015, 10:23 AM

This revision was automatically updated to reflect the committed changes.

op added a subscriber: op.Apr 29 2015, 12:55 PM

op added inline comments.

head/sys/kern/kern_racct.c
461 ↗	(On Diff #5078)	This is useless, if my theory not wrong.
489 ↗	(On Diff #5078)	Same here.
984 ↗	(On Diff #5078)	This does not cause memory leak? The steps in theory: enable the resource accounting start a random process disable resource accounting before the process exit wait to the process exit or kill the process Before this change, the racct_destroy() was always gets running.

op added inline comments.Apr 29 2015, 1:03 PM

head/sys/kern/kern_racct.c
489 ↗	(On Diff #5078)	This expression should be against "*racct == NULL" instead of "racct_enable".

trasz added inline comments.Apr 29 2015, 4:47 PM

head/sys/kern/kern_racct.c
984 ↗	(On Diff #5078)	The step #3 cannot ever happen; racct_enable is a tunable, not a sysctl - for this and several others, quite fundamental, reasons.

op added inline comments.Apr 29 2015, 6:13 PM

head/sys/kern/kern_racct.c
81 ↗	(On Diff #5078)	Ahh, I see now. That's a RO sysctl. Could you please add a comment here, to explain, that this never should be RW?
984 ↗	(On Diff #5078)	Fine. I missed that the sysctl is RO only.