From owner-svn-src-head@freebsd.org Mon Oct 9 18:22:20 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C6437E384C2; Mon, 9 Oct 2017 18:22:20 +0000 (UTC) (envelope-from alc@rice.edu) Received: from pp2.rice.edu (proofpoint2.mail.rice.edu [128.42.201.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9BC0A756D6; Mon, 9 Oct 2017 18:22:19 +0000 (UTC) (envelope-from alc@rice.edu) Received: from pps.filterd (pp2.rice.edu [127.0.0.1]) by pp2.rice.edu (8.16.0.17/8.16.0.17) with SMTP id v99CcA8l006667; Mon, 9 Oct 2017 13:22:12 -0500 Received: from mh1.mail.rice.edu (mh1.mail.rice.edu [128.42.201.20]) by pp2.rice.edu with ESMTP id 2desyj8kjc-1; Mon, 09 Oct 2017 13:22:12 -0500 Received-X: from mh1.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh1.mail.rice.edu (Postfix) with ESMTP id 2BF13460E19; Mon, 9 Oct 2017 13:22:12 -0500 (CDT) Received-X: from mh1.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh1.mail.rice.edu (Postfix) with ESMTP id 2A5F1460DE8; Mon, 9 Oct 2017 13:22:12 -0500 (CDT) X-Virus-Scanned: by amavis-2.7.0 at mh1.mail.rice.edu, auth channel Received-X: from mh1.mail.rice.edu ([127.0.0.1]) by mh1.mail.rice.edu (mh1.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id qezoSXrTPQVH; Mon, 9 Oct 2017 13:22:12 -0500 (CDT) Received: from 108-254-203-201.lightspeed.hstntx.sbcglobal.net (108-254-203-201.lightspeed.hstntx.sbcglobal.net [108.254.203.201]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh1.mail.rice.edu (Postfix) with ESMTPSA id B84AA460DE3; Mon, 9 Oct 2017 13:22:11 -0500 (CDT) Subject: Re: svn commit: r324420 - in head/sys: kern sys To: Cy Schubert , Alan Cox Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201710091554.v99FsAjK002950@slippy.cwsent.com> From: Alan Cox Message-ID: Date: Mon, 9 Oct 2017 13:22:09 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD i386; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <201710091554.v99FsAjK002950@slippy.cwsent.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=10 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611190142 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Oct 2017 18:22:20 -0000 On 10/09/2017 10:54, Cy Schubert wrote: > In message <201710082217.v98MHdNI012272@repo.freebsd.org>, Alan Cox writes: >> Author: alc >> Date: Sun Oct 8 22:17:39 2017 >> New Revision: 324420 >> URL: https://svnweb.freebsd.org/changeset/base/324420 >> >> Log: >> The blst_radix_init function has two purposes - to compute the number of >> nodes to allocate for the blist, and to initialize them. The computation >> can be done much more quickly by identifying the terminating node, if any, >> at every level of the tree and then summing the number of nodes at each >> level that precedes the topmost terminator. The initialization can also be >> done quickly, since settings at the root mark the tree as all-allocated, an >> d >> only a few terminator nodes need to be marked in the rest of the tree. >> Eliminate blst_radix_init, and perform its two functions more simply in >> blist_create. >> >> The allocation of the blist takes places in two pieces, but there's no good >> reason to do so, when a single allocation is sufficient, and simpler. >> Allocate the blist struct, and the array of nodes associated with it, with >> a >> single allocation. >> >> Submitted by: Doug Moore >> Reviewed by: markj (an earlier version) >> MFC after: 1 week >> Differential Revision: https://reviews.freebsd.org/D11968 >> >> Modified: >> head/sys/kern/subr_blist.c >> head/sys/sys/blist.h >> >> Modified: head/sys/kern/subr_blist.c >> ============================================================================= >> = >> --- head/sys/kern/subr_blist.c Sun Oct 8 21:20:25 2017 (r32441 >> 9) >> +++ head/sys/kern/subr_blist.c Sun Oct 8 22:17:39 2017 (r32442 >> 0) >> @@ -108,6 +108,7 @@ __FBSDID("$FreeBSD$"); >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -137,7 +138,6 @@ static void blst_copy(blmeta_t *scan, daddr_t blk, dad >> static daddr_t blst_leaf_fill(blmeta_t *scan, daddr_t blk, int count); >> static daddr_t blst_meta_fill(blmeta_t *scan, daddr_t allocBlk, daddr_t coun >> t, >> u_daddr_t radix); >> -static daddr_t blst_radix_init(blmeta_t *scan, daddr_t radix, daddr_t >> count); >> #ifndef _KERNEL >> static void blst_radix_print(blmeta_t *scan, daddr_t blk, daddr_t radix, >> int tab); >> @@ -218,30 +218,69 @@ blist_t >> blist_create(daddr_t blocks, int flags) >> { >> blist_t bl; >> - daddr_t nodes, radix; >> + daddr_t i, last_block; >> + u_daddr_t nodes, radix, skip; >> + int digit; >> >> /* >> - * Calculate the radix field used for scanning. >> + * Calculate the radix and node count used for scanning. Find the last >> + * block that is followed by a terminator. >> */ >> + last_block = blocks - 1; >> radix = BLIST_BMAP_RADIX; >> while (radix < blocks) { >> + if (((last_block / radix + 1) & BLIST_META_MASK) != 0) >> + /* >> + * A terminator will be added. Update last_block to th >> e >> + * position just before that terminator. >> + */ >> + last_block |= radix - 1; >> radix *= BLIST_META_RADIX; >> } >> - nodes = 1 + blst_radix_init(NULL, radix, blocks); >> >> - bl = malloc(sizeof(struct blist), M_SWAP, flags); >> + /* >> + * Count the meta-nodes in the expanded tree, including the final >> + * terminator, from the bottom level up to the root. >> + */ >> + nodes = (last_block >= blocks) ? 2 : 1; >> + last_block /= BLIST_BMAP_RADIX; >> + while (last_block > 0) { >> + nodes += last_block + 1; >> + last_block /= BLIST_META_RADIX; >> + } >> + bl = malloc(offsetof(struct blist, bl_root[nodes]), M_SWAP, flags); >> if (bl == NULL) >> return (NULL); >> >> bl->bl_blocks = blocks; >> bl->bl_radix = radix; >> bl->bl_cursor = 0; >> - bl->bl_root = malloc(nodes * sizeof(blmeta_t), M_SWAP, flags); >> - if (bl->bl_root == NULL) { >> - free(bl, M_SWAP); >> - return (NULL); >> + >> + /* >> + * Initialize the empty tree by filling in root values, then initialize >> + * just the terminators in the rest of the tree. >> + */ >> + bl->bl_root[0].bm_bighint = 0; >> + if (radix == BLIST_BMAP_RADIX) >> + bl->bl_root[0].u.bmu_bitmap = 0; >> + else >> + bl->bl_root[0].u.bmu_avail = 0; >> + last_block = blocks - 1; >> + i = 0; >> + while (radix > BLIST_BMAP_RADIX) { >> + radix /= BLIST_META_RADIX; >> + skip = radix_to_skip(radix); >> + digit = last_block / radix; >> + i += 1 + digit * skip; >> + if (digit != BLIST_META_MASK) { >> + /* >> + * Add a terminator. >> + */ >> + bl->bl_root[i + skip].bm_bighint = (daddr_t)-1; >> + bl->bl_root[i + skip].u.bmu_bitmap = 0; >> + } >> + last_block %= radix; >> } >> - blst_radix_init(bl->bl_root, radix, blocks); >> >> #if defined(BLIST_DEBUG) >> printf( >> @@ -261,7 +300,7 @@ blist_create(daddr_t blocks, int flags) >> void >> blist_destroy(blist_t bl) >> { >> - free(bl->bl_root, M_SWAP); >> + >> free(bl, M_SWAP); >> } >> >> @@ -1070,83 +1109,6 @@ blst_meta_fill(blmeta_t *scan, daddr_t allocBlk, daddr >> } >> scan->u.bmu_avail -= nblks; >> return (nblks); >> -} >> - >> -/* >> - * BLST_RADIX_INIT() - initialize radix tree >> - * >> - * Initialize our meta structures and bitmaps and calculate the exact >> - * amount of space required to manage 'count' blocks - this space may >> - * be considerably less than the calculated radix due to the large >> - * RADIX values we use. >> - */ >> -static daddr_t >> -blst_radix_init(blmeta_t *scan, daddr_t radix, daddr_t count) >> -{ >> - daddr_t i, memindex, next_skip, skip; >> - >> - memindex = 0; >> - >> - /* >> - * Leaf node >> - */ >> - >> - if (radix == BLIST_BMAP_RADIX) { >> - if (scan) { >> - scan->bm_bighint = 0; >> - scan->u.bmu_bitmap = 0; >> - } >> - return (memindex); >> - } >> - >> - /* >> - * Meta node. If allocating the entire object we can special >> - * case it. However, we need to figure out how much memory >> - * is required to manage 'count' blocks, so we continue on anyway. >> - */ >> - >> - if (scan) { >> - scan->bm_bighint = 0; >> - scan->u.bmu_avail = 0; >> - } >> - >> - skip = radix_to_skip(radix); >> - next_skip = skip / BLIST_META_RADIX; >> - radix /= BLIST_META_RADIX; >> - >> - for (i = 1; i < skip; i += next_skip) { >> - if (count >= radix) { >> - /* >> - * Allocate the entire object >> - */ >> - memindex = i + >> - blst_radix_init(((scan) ? &scan[i] : NULL), radix, >> - radix); >> - count -= radix; >> - } else if (count > 0) { >> - /* >> - * Allocate a partial object >> - */ >> - memindex = i + >> - blst_radix_init(((scan) ? &scan[i] : NULL), radix, >> - count); >> - count = 0; >> - } else { >> - /* >> - * Add terminator and break out. Make terminator bitma >> p >> - * zero to avoid a spanning leaf allocation that >> - * includes the terminator. >> - */ >> - if (scan) { >> - scan[i].bm_bighint = (daddr_t)-1; >> - scan[i].u.bmu_bitmap = 0; >> - } >> - break; >> - } >> - } >> - if (memindex < i) >> - memindex = i; >> - return (memindex); >> } >> >> #ifdef BLIST_DEBUG >> >> Modified: head/sys/sys/blist.h >> ============================================================================= >> = >> --- head/sys/sys/blist.h Sun Oct 8 21:20:25 2017 (r324419) >> +++ head/sys/sys/blist.h Sun Oct 8 22:17:39 2017 (r324420) >> @@ -82,7 +82,7 @@ typedef struct blist { >> daddr_t bl_blocks; /* area of coverage */ >> u_daddr_t bl_radix; /* coverage radix */ >> daddr_t bl_cursor; /* next-fit search starts at */ >> - blmeta_t *bl_root; /* root of radix tree */ >> + blmeta_t bl_root[1]; /* root of radix tree */ >> } *blist_t; >> >> #define BLIST_META_RADIX 16 >> > This commit is causing this: > > panic: freeing invalid range > > On my laptop (Core i3 chip): > > (kgdb) bt > #0 doadump (textdump=1) at pcpu.h:232 > #1 0xffffffff80583e16 in kern_reboot (howto=260) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:386 > #2 0xffffffff80584306 in vpanic (fmt=, > ap=) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:779 > #3 0xffffffff80584123 in panic (fmt=) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:710 > #4 0xffffffff805b9133 in blst_meta_free (scan=0xfffffe00045c3128, > freeBlk=, count=0, radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:869 > #5 0xffffffff805b90df in blst_meta_free (scan=0xfffffe00045c3048, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #6 0xffffffff805b90df in blst_meta_free (scan=0xfffffe00045c3038, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #7 0xffffffff805b90df in blst_meta_free (scan=0xfffffe00045c3028, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #8 0xffffffff805b90df in blst_meta_free (scan=0xfffffe00045c3018, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #9 0xffffffff808359af in swaponsomething (vp=, > id=0xfffff8000cb97200, nblks=3145727, > strategy=0xffffffff80835c60 , > close=0xffffffff80835ee0 , dev=132, flags=1) > at /opt/src/svn-current/sys/vm/swap_pager.c:2199 > #10 0xffffffff8083392c in sys_swapon (td=, > uap=) at /opt/src/svn-current/sys/vm/swap_pager.c:2 > 728 > #11 0xffffffff80887a11 in amd64_syscall (td=0xfffff8000c659560, traced=0) > at subr_syscall.c:132 > #12 0xffffffff8086afcb in Xfast_syscall () > at /opt/src/svn-current/sys/amd64/amd64/exception.S:419 > #13 0x000000002c689aea in ?? () > Previous frame inner to this frame (corrupt stack?) > Current language: auto; currently minimal > (kgdb) > > > On my gateway machine (AMD 4600+): > > (kgdb) bt > #0 doadump (textdump=) at pcpu.h:232 > #1 0xffffffff80573e11 in kern_reboot (howto=260) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:386 > #2 0xffffffff805742e6 in vpanic (fmt=, > ap=) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:779 > #3 0xffffffff80574103 in panic (fmt=) > at /opt/src/svn-current/sys/kern/kern_shutdown.c:710 > #4 0xffffffff805a8b63 in blst_meta_free (scan=0xfffffe00021c9038, > freeBlk=, count=0, radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:869 > #5 0xffffffff805a8b0f in blst_meta_free (scan=0xfffffe00021c9028, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #6 0xffffffff805a8b0f in blst_meta_free (scan=0xfffffe00021c9018, > freeBlk=, count=, > radix=) > at /opt/src/svn-current/sys/kern/subr_blist.c:926 > #7 0xffffffff8081becf in swaponsomething (vp=, > id=0xfffff800124d1080, nblks=262144, > strategy=0xffffffff8081c180 , > close=0xffffffff8081c400 , dev=101, flags=1) > at /opt/src/svn-current/sys/vm/swap_pager.c:2199 > #8 0xffffffff80819e4c in sys_swapon (td=, > uap=) at /opt/src/svn-current/sys/vm/swap_pager.c:2 > 728 > #9 0xffffffff80869fb1 in amd64_syscall (td=0xfffff80012558000, traced=0) > at subr_syscall.c:132 > #10 0xffffffff8084dd4b in Xfast_syscall () > at /opt/src/svn-current/sys/amd64/amd64/exception.S:419 > #11 0x0000000800a89aea in ?? () > Previous frame inner to this frame (corrupt stack?) > Current language: auto; currently minimal > (kgdb) > > In the case of my laptop, it panicked only once. My gateway machine > downstairs was in a panic reboot loop, panicking during swapon. > > Reverting this diff resolves my issue. > > Doug Moore has provided a fix, which I just committed.