Remove the arbitrary limit on DTrace buffers
AbandonedPublic
Actions

Authored by gnn on Feb 17 2017, 10:50 PM.

Details

Reviewers

Group Reviewers

DTrace

Summary

Turn the limit into a sysctl (kern.dtrace.buffer_maxsize)

Test Plan

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7548
Build 7701: arc lint + arc unit

gnn retitled this revision from to Remove the arbitrary limit on DTrace buffers.

gnn updated this object.

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

rstone added inline comments.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12122	I like that this is tunable but isn't this allocated per-cpu? As I understand it the original heuristic was attempting to prevent us from allocating too much on large-cpu count machines. Maybe that's irrelevant because for us "large cpu count" implies "huge amounts of memory"

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12122	I don't know of any systems with 40 cores that don't also have 128G or more of RAM. Also,note that the tunable is 16m, so even with 40 cores that's "only" 640M, and no one needs more than 640...M

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12125	I don't really understand what this comment means. We don't sleep here if there's a memory shortage, so an allocation failure should just cause us to release any already-allocated buffers and bail. Am I missing something?
sys/cddl/dev/dtrace/dtrace_sysctl.c
99	SYSCTL_QUAD is wrong for this: dtrace_strsize_default is a size_t which is 32 bits in ILP32. I guess SYSCTL_ULONG is better.
100	typo: "szie"

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12125	Hmm. libdtrace also needs to allocate buffers of this size for when it copies the kernel buffers, so one can, for instance, trigger the OOM killer by using a large enough buffer size. In that case, we should probably set buffer_maxsize based on physmem/ncpu. See r261122.

gnn marked 2 inline comments as done.Feb 17 2017, 11:19 PM

gnn added inline comments.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12125	If you like I can also remove the comment. That's not original to me.

Updated

Address two of markj's suggestions.

gnn marked 2 inline comments as done.Mar 21 2017, 4:55 AM

gnn added inline comments.

sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
12125	In 261122 all the limits were removed in libdtrace. Are you suggesting that due to that change I should at least do a protection setting of: if (size > physmem * PAGE_SIZE) at this point?