Details

Reviewers

Commits

rS323391: To analyze the allocation of swap blocks by blist functions, add a method

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

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dougm created this revision.Aug 7 2017, 7:06 AM

Use sbufs. Insert some sysctl hooks.

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

alc added inline comments.Aug 13 2017, 4:08 PM

sys/kern/subr_blist.c
376	Insert blank line.
383	Incorrect indentation.
385	Insert blank line.
391	Insert blank line.
400	Insert blank line.
407–408	The u_daddr_t definition should come first because it is a larger data type.
422–423	This will probably need to change, but it can wait for a future revision.
426	Use postfix instead.
432	Use postfix.
443	Insert blank line.
452	The outermost parens around the expression are not needed. Spaces are required around binary operators.
463	This needs to be declared in blist.h.
467	The struct should come first.
524	This needs to be implemented in swap_pager.c. See swp_pager_getswapspace(), swp_pager_strategy(), or swp_pager_freeswapspace().

alc mentioned this in D11819: Allow blist allocations to span leaf boundaries, but not meta boundaries.Aug 13 2017, 4:11 PM

Endeavor to apply reviewer suggestions.

alc added inline comments.Aug 26 2017, 5:02 PM

sys/kern/subr_blist.c
391–398	It is probably better to bzero() the entire structure and then initialize "start".
sys/vm/swap_pager.c
804–805	This function should be "static".
817	The braces can be eliminated.
825	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");

Use bzero. Make sysctl handler static.

alc added inline comments.Sep 1 2017, 2:28 AM

sys/kern/subr_blist.c
95	This should be <sys/sbuf.h>
442–443	You need to use "%ju" and uintmax_t.
sys/vm/swap_pager.c
169	Get rid of this stray blank line.

alc added inline comments.Sep 1 2017, 2:30 AM

sys/kern/subr_blist.c
393	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.

Update comments.

alc added inline comments.Sep 4 2017, 4:52 PM

sys/kern/subr_blist.c
375	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.

Fix cut-n-paste error.

Make fib array const.

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.

Update the sysctl.

Constify two gap parameters. Reformat fib initialization.

Fix line-continuation indentation on some sbuf_prinf lines.

dougm edited the summary of this revision. (Show Details)Sep 10 2017, 4:47 PM

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.

Closed by commit rS323391: To analyze the allocation of swap blocks by blist functions, add a method (authored by alc). · Explain WhySep 10 2017, 5:46 PM

This revision was automatically updated to reflect the committed changes.

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 32780

sys/kern/subr_blist.c

sys/sys/blist.h

sys/vm/swap_pager.c

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 32780

sys/kern/subr_blist.c

sys/sys/blist.h

sys/vm/swap_pager.c

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

Revision Contents
Changeset List