Page MenuHomeFreeBSD

est tool, so that 's' writes data about free space distribution
ClosedPublic

Authored by dougm on Aug 7 2017, 7:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 12:29 PM
Unknown Object (File)
Wed, Mar 20, 2:46 AM
Unknown Object (File)
Tue, Mar 19, 4:51 AM
Unknown Object (File)
Tue, Mar 19, 3:23 AM
Unknown Object (File)
Wed, Mar 13, 4:42 PM
Unknown Object (File)
Fri, Mar 1, 9:34 AM
Unknown Object (File)
Thu, Feb 29, 7:34 PM
Unknown Object (File)
Thu, Feb 29, 6:37 PM
Subscribers
None

Details

Summary

To help analyze the allocation of memory blocks by blist functions, add a method for analyzing the radix tree structures and reporting on the number, and sizes, of maximal intervals of free blocks. The report includes the number of maximal intervals, and also the number of them in each of several size ranges, from small (size 1, or 3 to 4) to large (28657 to 46367) with size boundaries defined by Fibonacci numbers. The report is written in the test tool with the 's' command, or in a running kernel by sysctl.

The analysis of the radix tree frequently computes the position of the lone bit set in a daddr_t, a computation that also appears in leaf allocation. That computation has been moved into a function of its own, and optimized for cases where an inlined machine instruction can replace the usual binary search.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Use sbufs. Insert some sysctl hooks.

Widen fields to u_addr_t for computing stats. Eliminate some redundant state.

sys/kern/subr_blist.c
376 ↗(On Diff #31798)

Insert blank line.

383 ↗(On Diff #31798)

Incorrect indentation.

385 ↗(On Diff #31798)

Insert blank line.

391 ↗(On Diff #31798)

Insert blank line.

400 ↗(On Diff #31798)

Insert blank line.

407–408 ↗(On Diff #31798)

The u_daddr_t definition should come first because it is a larger data type.

422–423 ↗(On Diff #31798)

This will probably need to change, but it can wait for a future revision.

426 ↗(On Diff #31798)

Use postfix instead.

432 ↗(On Diff #31798)

Use postfix.

443 ↗(On Diff #31798)

Insert blank line.

452 ↗(On Diff #31798)

The outermost parens around the expression are not needed. Spaces are required around binary operators.

463 ↗(On Diff #31798)

This needs to be declared in blist.h.

467 ↗(On Diff #31798)

The struct should come first.

524 ↗(On Diff #31798)

This needs to be implemented in swap_pager.c. See swp_pager_getswapspace(), swp_pager_strategy(), or swp_pager_freeswapspace().

Endeavor to apply reviewer suggestions.

sys/kern/subr_blist.c
391–398 ↗(On Diff #32009)

It is probably better to bzero() the entire structure and then initialize "start".

sys/vm/swap_pager.c
804–805 ↗(On Diff #32009)

This function should be "static".

817 ↗(On Diff #32009)

The braces can be eliminated.

825 ↗(On Diff #32009)

The below SYSCTL_OID() definition is an example of how to hook this function into the sysctl framework.

static int sysctl_vm_phys_free(SYSCTL_HANDLER_ARGS);
SYSCTL_OID(_vm, OID_AUTO, phys_free, CTLTYPE_STRING | CTLFLAG_RD,
    NULL, 0, sysctl_vm_phys_free, "A", "Phys Free Info");
dougm marked 8 inline comments as done.

Use bzero. Make sysctl handler static.

sys/kern/subr_blist.c
95 ↗(On Diff #32411)

This should be <sys/sbuf.h>

442–443 ↗(On Diff #32411)

You need to use "%ju" and uintmax_t.

sys/vm/swap_pager.c
169 ↗(On Diff #32411)

Get rid of this stray blank line.

sys/kern/subr_blist.c
393 ↗(On Diff #32411)

Insert a blank line before the bzero().

swap_pager.c needs #include <sys/sbuf.h>

Fix histograms to use boundaries at power of 2 and 3x powers of 2. Fix types in output. Fix overflows in computing averages. Drop debruijn method for finding bit location. Drop use of flsll for finding hi bit position.

Factor out some histogram calculation.

Eliminate redundant variable.

Eliminate (another) redundant variable.

sys/kern/subr_blist.c
376 ↗(On Diff #32573)

If every other block is allocated, this field can overflow.

Widen the field for counting the number of gaps.

Widen the histogram count fields.

Adopt reviewer comments on code formatting, etc.

Allocate the gap_stats struct with malloc, before acquiring any locks, use the allocated struct for all calculations, and free it after releasing any locks.

Add requisite blank lines to new functions.

Change histogram buckets to have fibonacci boundaries. Improve presentation of histogram output.

Don't print 0 histogram entries.

Fix test on fib bucket search.

Handle the case of only 1 bucket in the histogram.

Store stats struct on stack. Change output presentation. Add device names to sysctl output. Clean up comments.

Stop using 'gap' in output. Stop writing ranges of only one value.

Address comments on signedness, argument ordering, comment formatting, definition ordering, function naming, parameter naming, sequence length, function return typing, indentation.

Constify two gap parameters. Reformat fib initialization.

Fix line-continuation indentation on some sbuf_prinf lines.

dougm retitled this revision from Add a stats option to the test tool, so that 's' writes data about free space distribution to est tool, so that 's' writes data about free space distribution.Sep 10 2017, 5:03 PM
dougm edited the summary of this revision. (Show Details)

Change sizeof() argument from type name to variable name.

This revision was automatically updated to reflect the committed changes.