Page MenuHomeFreeBSD
Differential D20579

Remove limit on blist_alloc maxsize, allow bigger getswapspace allocations
ClosedPublic
Actions

Authored by dougm on Jun 9 2019, 7:19 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 18, 1:25 PM
Unknown Object (File)
Tue, Apr 16, 9:24 AM
Unknown Object (File)
Dec 22 2023, 10:19 PM
Unknown Object (File)
Sep 2 2023, 5:16 AM
Unknown Object (File)
Jul 3 2023, 9:45 AM
Unknown Object (File)
Jul 3 2023, 9:44 AM
Unknown Object (File)
Jul 3 2023, 9:43 AM
Unknown Object (File)
Jul 3 2023, 9:42 AM
View All 17 Files
Subscribers
imp
pho

Details

Reviewers
kib
markj
alc
Commits
rS349777: Change blist_next_leaf_alloc so that it can examine more than one leaf
Summary

Change blist_next_leaf_alloc so that it can examine more than one leaf after the one where the possible block allocation begins, and allocate a larger number of blocks than the current limit. This does not affect the limit on minimum allocation size, which still cannot exceed BLIST_MAX_ALLOC.

Use this change to modify swp_pager_getswapspace and its callers, so that it can allocate more than BLIST_MAX_ALLOC blocks if they are available, to avoid a later call that would allocate those blocks anyway.

The change to blist_next_leaf_alloc could also be used to support TRIM, to grab a large number of blocks at a time to get trimmed. That would require other changes, including a new "trimming cursor" that stays ahead of the regular cursor, but none of that is implemented yet.

Test Plan

I can boot a kernel with this patch. I'm not sure how to test to guarantee that swp_pager_getswapspace calls with npages > BLIST_MAX_ALLOC will be exercised.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dougm created this revision.Jun 9 2019, 7:19 PM
markj added inline comments.Jun 21 2019, 8:14 PM
kern/subr_blist.c
621 ↗(On Diff #58456)

As a matter of style I would prefer to avoid introducing new camel case names.

627 ↗(On Diff #58456)

We are assuming that startBlk is 0 mod BLIST_BMAP_RADIX, is that right?

633 ↗(On Diff #58456)

This line should be indented by four spaces.

637 ↗(On Diff #58456)

I don't quite understand this. How are we sure that scan is a leaf node?

664 ↗(On Diff #58456)

This line should be indented by four spaces.

vm/swap_pager.c
733 ↗(On Diff #58456)

Should this be imin()?

1387 ↗(On Diff #58456)

Now that number is one. :)

dougm updated this revision to Diff 58876.Jun 21 2019, 10:19 PM
dougm marked 7 inline comments as done.

By renaming, re-indenting, and updating stale comments, seek to answer questions and address reviewer concerns.

markj added a comment.Jul 4 2019, 6:34 PM

I apologize for the delay in getting back to this.

I can boot a kernel with this patch. I'm not sure how to test to guarantee that swp_pager_getswapspace calls with npages > BLIST_MAX_ALLOC will be exercised.

You'd need to at least bump the value of vm_pageout_page_count.

kern/subr_blist.c
654 ↗(On Diff #58876)

Here we're clearing bits from the next leaf. Isn't it possible that we are allocating blocks from multiple leaf nodes starting at start if maxcount is sufficiently large?

655 ↗(On Diff #58876)

Why can't this be written as maxcount % BLIST_BMAP_RADIX?

vm/swap_pager.c
1386 ↗(On Diff #58876)

"limited by"

dougm updated this revision to Diff 59404.Jul 4 2019, 6:56 PM
dougm marked 3 inline comments as done.

Add/alter comments and slightly restructure a computation in response to reviewer comments.

pho added a comment.Jul 5 2019, 11:20 AM

Typo?

cc -c -O2 -pipe -fno-strict-aliasing  -g -nostdinc  -I. -I../../.. -I../../../contrib/ck/include -I../../../contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h    -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -MD  -MF.depend.subr_blist.o -MTsubr_blist.o -fdebug-prefix-map=./machine=/usr/src/sys/amd64/include -fdebug-prefix-map=./x86=/usr/src/sys/x86/include -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse -msoft-float  -fno-asynchronous-unwind-tables -ffreestanding -fwrapv -fstack-protector -gdwarf-2 -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error-tautological-compare -Wno-error-empty-body -Wno-error-parentheses-equality -Wno-error-unused-function -Wno-error-pointer-sign -Wno-error-shift-negative-value -Wno-address-of-packed-member  -mno-aes -mno-avx  -std=iso9899:1999 -Werror  ../../../kern/subr_blist.c                                                                                                                                                
../../../kern/subr_blist.c:659:17: error: use of undeclared identifier 'BLIST_MAP_RADIX'
        if (maxcount % BLIST_MAP_RADIX != 0)
                       ^
1 error generated.
*** Error code 1
dougm updated this revision to Diff 59444.Jul 5 2019, 3:09 PM

Fix typo.

markj accepted this revision.Jul 5 2019, 3:14 PM
This revision is now accepted and ready to land.Jul 5 2019, 3:14 PM
pho added a comment.Jul 5 2019, 8:15 PM

I have run tests on D20579.59404.diff (with the typo fixed) for 8 hours. This included a buildworld / installworld.

Closed by commit rS349777: Change blist_next_leaf_alloc so that it can examine more than one leaf (authored by dougm). · Explain WhyJul 6 2019, 6:15 AM
This revision was automatically updated to reflect the committed changes.
dougm added a commit: rS349777: Change blist_next_leaf_alloc so that it can examine more than one leaf.
Herald added a subscriber: imp. · View Herald TranscriptJul 6 2019, 6:15 AM
pho added a comment.Jul 9 2019, 10:24 AM

Looks like this patch is responsible for the panics I see.

20190709 11:56:55 all (1/1): sort.sh
swap_pager: out of swap space
swp_pager_getswapspace(32): failed
Jul  9 12:03:41 mercat1 kernel: pid 3581 (sort), jid 0, uid 0, was killed: out of swap space
Jul  9 12:03:42 mercat1 kernel: pid 3583 (sort), jid 0, uid 0, was killed: out of swap space
panic: freeing free block: ffffc0, size 16, mask 1
cpuid = 7
time = 1562666624
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe102d95b270
vpanic() at vpanic+0x19d/frame 0xfffffe102d95b2c0
panic() at panic+0x43/frame 0xfffffe102d95b320
blst_meta_free() at blst_meta_free+0x12b/frame 0xfffffe102d95b360
blst_meta_free() at blst_meta_free+0x102/frame 0xfffffe102d95b3a0
blst_meta_free() at blst_meta_free+0x102/frame 0xfffffe102d95b3e0
blst_meta_free() at blst_meta_free+0x102/frame 0xfffffe102d95b420
blst_meta_free() at blst_meta_free+0x102/frame 0xfffffe102d95b460
blist_free() at blist_free+0x2e/frame 0xfffffe102d95b480
swp_pager_freeswapspace() at swp_pager_freeswapspace+0x8a/frame 0xfffffe102d95b4a0
swp_pager_meta_free_all() at swp_pager_meta_free_all+0xbb/frame 0xfffffe102d95b4f0
swap_pager_dealloc() at swap_pager_dealloc+0x115/frame 0xfffffe102d95b510
vm_object_terminate() at vm_object_terminate+0x27b/frame 0xfffffe102d95b560
vm_object_deallocate() at vm_object_deallocate+0x412/frame 0xfffffe102d95b5c0
vm_map_process_deferred() at vm_map_process_deferred+0x7f/frame 0xfffffe102d95b5e0
vm_map_remove() at vm_map_remove+0xc6/frame 0xfffffe102d95b610
vmspace_exit() at vmspace_exit+0xd3/frame 0xfffffe102d95b650
exit1() at exit1+0x5ad/frame 0xfffffe102d95b6c0
sigexit() at sigexit+0xdaf/frame 0xfffffe102d95b9a0
postsig() at postsig+0x336/frame 0xfffffe102d95ba70
ast() at ast+0x4c7/frame 0xfffffe102d95bab0
doreti_ast() at doreti_ast+0x1f/frame 0x7fffffffded0
KDB: enter: panic
[ thread pid 3581 tid 100238 ]
Stopped at      kdb_enter+0x3b: movq    $0,kdb_why
db> x/s version
version:        FreeBSD 13.0-CURRENT #5 r349777: Tue Jul  9 11:37:16 CEST 2019\012    pho@mercat1.netperf.freebsd.org:/usr/src/sys/amd64/compile/MEMGUARD\012
db>

The same test works fine on r349776.

Revision Contents
Changeset List

PathSize
sys/
kern/
104 lines
vm/
12 lines

Diff 59404

View Options

sys/kern/subr_blist.c

Loading...
View Options

sys/vm/swap_pager.c

Loading...