Page MenuHomeFreeBSD
Differential D18058

Handle the possiblity that the cursor has already passed the free blocks of a meta-node.
ClosedPublic
Actions

Authored by dougm on Nov 20 2018, 9:12 PM.
Tags
None
Referenced Files
F103819545: D18058.diff
Fri, Nov 29, 8:58 PM
Unknown Object (File)
Wed, Nov 6, 9:31 AM
Unknown Object (File)
Sat, Nov 2, 5:30 PM
Unknown Object (File)
Oct 17 2024, 2:08 AM
Unknown Object (File)
Oct 15 2024, 6:41 AM
Unknown Object (File)
Oct 1 2024, 6:17 AM
Unknown Object (File)
Sep 26 2024, 5:19 AM
Unknown Object (File)
Sep 24 2024, 4:58 AM
View All 57 Files
Subscribers
cem
imp
pho

Details

Reviewers
alc
markj
Commits
rS340914: blist_meta_alloc assumes that mask=scan->bm_bitmap is nonzero. But if the
Summary

blist_meta_alloc assumes that mask=scan->bm_bitmap is nonzero. But if the cursor lies in the middle of the
space that the meta node represents, then blanking the low bits of mask may make it zero, and break later code that expects a nonzero value. Add a test that returns failure if the mask has been cleared.

Diff Detail

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

Event Timeline

dougm created this revision.Nov 20 2018, 9:12 PM
dougm updated this revision to Diff 50652.Nov 21 2018, 4:10 AM
dougm edited the summary of this revision. (Show Details)
dougm retitled this revision from Handle the possiblity that the cursor has already passed the free nodes of a meta-node. to Handle the possiblity that the cursor has already passed the free blocks of a meta-node..Nov 21 2018, 4:18 AM
dougm added reviewers: alc, markj.
dougm added a subscriber: pho.
alc accepted this revision.Nov 21 2018, 5:11 PM

Yes, this is clearly a bug. Hopefully, it is the one that Peter is tripping over.

This revision is now accepted and ready to land.Nov 21 2018, 5:11 PM
pho added a comment.Nov 24 2018, 6:58 AM

D18058.id50652.diff ran the stress2/misc/stealer.sh test scenario for 11 hours before freezing up: https://people.freebsd.org/~pho/stress/log/dougm015.txt

dougm updated this revision to Diff 51077.Nov 24 2018, 8:33 PM

Allow bighint to be updated for a meta-node where the cursor lies ahead of all nonempty children, or even when there are no nonempty children, since blist_alloc depends on a reduction of the root bighint to allow a failed allocation to finish.

Alternatively, we could change the test in blist_alloc to depend on something other than bighint. This is the only case where having a too-large bighint breaks things.

This revision now requires review to proceed.Nov 24 2018, 8:33 PM
alc added a comment.Nov 24 2018, 9:39 PM
In D18058#388822, @pho wrote:

D18058.id50652.diff ran the stress2/misc/stealer.sh test scenario for 11 hours before freezing up: https://people.freebsd.org/~pho/stress/log/dougm015.txt

This is a different problem that will require a different change.

alc added a comment.Nov 24 2018, 9:42 PM
In D18058#389007, @dougm_rice.edu wrote:

Allow bighint to be updated for a meta-node where the cursor lies ahead of all nonempty children, or even when there are no nonempty children, since blist_alloc depends on a reduction of the root bighint to allow a failed allocation to finish.

Alternatively, we could change the test in blist_alloc to depend on something other than bighint. This is the only case where having a too-large bighint breaks things.

Umm, you just deleted the patch here that fixes the original panic, and the current patch has nothing to do with the summary. Please create a new review for the current patch.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2018, 9:52 PM
Closed by commit rS340914: blist_meta_alloc assumes that mask=scan->bm_bitmap is nonzero. But if the (authored by alc). · Explain Why
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptNov 24 2018, 9:52 PM
markj added a comment.EditedNov 27 2018, 4:41 PM

My desktop panicked overnight with panic "freeing freed block" on line 828. I have a kernel dump if there's anything you'd like to see:

Edit: meant to add that this is an INVARIANTS kernel at r340969 I was using a pre-r340914 kernel by accident.

(kgdb) bt
#0  __curthread () at ./machine/pcpu.h:230
#1  doadump (textdump=0) at /home/mark/src/freebsd-dev/sys/kern/kern_shutdown.c:371
#2  0xffffffff823961a8 in vt_kms_postswitch () from /boot/modules/drm.ko
#3  0xffffffff806451f8 in vt_window_switch (vw=0xffffffff80ea4990 <vt_conswindow>) at /home/mark/src/freebsd-dev/sys/dev/vt/vt_core.c:580
#4  0xffffffff806428c0 in vtterm_cngrab (tm=<optimized out>) at /home/mark/src/freebsd-dev/sys/dev/vt/vt_core.c:1572
#5  0xffffffff80731042 in cngrab () at /home/mark/src/freebsd-dev/sys/kern/kern_cons.c:370
#6  0xffffffff8079216b in vpanic (fmt=0xffffffff80bcbaa2 "freeing free block", ap=0xfffffe085221d670) at /home/mark/src/freebsd-dev/sys/kern/kern_shutdown.c:851
#7  0xffffffff80791fd3 in panic (fmt=<unavailable>) at /home/mark/src/freebsd-dev/sys/kern/kern_shutdown.c:804
#8  0xffffffff807c5c6b in blst_leaf_free (scan=0xfffffe0850b7efe0, blk=<optimized out>, count=<unavailable>) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:828
#9  blst_meta_free (scan=0xfffffe0850b7efe0, freeBlk=<optimized out>, count=<unavailable>, radix=64) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:857
#10 0xffffffff807c5c42 in blst_meta_free (scan=0xfffffe0850b7ebe0, freeBlk=499712, count=<unavailable>, radix=64) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:869
#11 0xffffffff807c5c42 in blst_meta_free (scan=0xfffffe0850b70440, freeBlk=499712, count=<unavailable>, radix=4096) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:869
#12 0xffffffff807c5c42 in blst_meta_free (scan=0xfffffe0850b60020, freeBlk=524288, count=<unavailable>, radix=262144) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:869
#13 0xffffffff807c5b1b in blist_free (bl=0xfffffe0850b60000, blkno=<unavailable>, count=1) at /home/mark/src/freebsd-dev/sys/kern/subr_blist.c:336
#14 0xffffffff80ac628a in swp_pager_freeswapspace (blk=<optimized out>, npages=1) at /home/mark/src/freebsd-dev/sys/vm/swap_pager.c:815
#15 0xffffffff80ad3611 in vm_fault_hold (map=0xfffff80094cb6000, vaddr=<optimized out>, fault_type=2 '\002', fault_flags=<optimized out>, m_hold=0x0) at /home/mark/src/freebsd-dev/sys/vm/vm_fault.c:1279
#16 0xffffffff80ad1c30 in vm_fault (map=0xfffff80094cb6000, vaddr=<optimized out>, fault_type=2 '\002', fault_flags=0) at /home/mark/src/freebsd-dev/sys/vm/vm_fault.c:536
#17 0xffffffff80b29e98 in trap_pfault (frame=0xfffffe085221dac0, usermode=<optimized out>) at /home/mark/src/freebsd-dev/sys/amd64/amd64/trap.c:830
#18 0xffffffff80b2953a in trap (frame=0xfffffe085221dac0) at /home/mark/src/freebsd-dev/sys/amd64/amd64/trap.c:351
dougm added a comment.Nov 27 2018, 6:13 PM

The stack trace shows a panic call from subr_blist.c:828. After this change was accepted with revision 340914, that panic line moved from line 828 to line 830. Can you double-check to make sure that you're testing a version that includes this change?

markj added a comment.Nov 27 2018, 6:18 PM
In D18058#390117, @dougm_rice.edu wrote:

The stack trace shows a panic call from subr_blist.c:828. After this change was accepted with revision 340914, that panic line moved from line 828 to line 830. Can you double-check to make sure that you're testing a version that includes this change?

You're right. I had installed a new kernel yesterday but didn't boot into it, so I got confused. Sorry for the noise.

cem added a subscriber: cem.EditedDec 13 2018, 5:16 PM
In D18058#390059, @markj wrote:

My desktop panicked overnight with panic "freeing freed block" on line 828. I have a kernel dump if there's anything you'd like to see:

Edit: meant to add that this is an INVARIANTS kernel at r340969

@markj Can you update this comment with the pre-340914 version you were actually using, if known? As-is, this is a little confusing :-). Thanks!

FWIW, I also hit the panic this revision fixes on a 340866+ (as in, SVN 340866 + local git modifications) non-INVARIANTS kernel and reopened the very similar-looking (but much older) PR 83761 for it. MarkJ pointed me at this review. @dougm_rice.edu Thanks for fixing the bug. I'm running 342026+ now.

markj added a comment.Dec 13 2018, 5:23 PM
In D18058#395282, @cem wrote:
In D18058#390059, @markj wrote:

My desktop panicked overnight with panic "freeing freed block" on line 828. I have a kernel dump if there's anything you'd like to see:

Edit: meant to add that this is an INVARIANTS kernel at r340969

@markj Can you update this comment with the pre-340914 version you were actually using, if known? As-is, this is a little confusing :-). Thanks!

I don't know what the revision was, just that it was before r340914.

cem added a comment.Dec 13 2018, 5:29 PM
In D18058#395284, @markj wrote:
In D18058#395282, @cem wrote:

@markj Can you update this comment with the pre-340914 version you were actually using, if known? As-is, this is a little confusing :-). Thanks!

I don't know what the revision was, just that it was before r340914.

If you've got the old one around, the what(1) command on the kernel file should show it. If not, that's ok; I'd still appreciate using markdown syntax to strike out the incorrect text (two tildes on either side of the text).

Revision Contents
Changeset List

PathSize
head/
sys/
kern/
2 lines

Diff 51078

View Options

head/sys/kern/subr_blist.c

Loading...