From owner-svn-src-head@freebsd.org Mon Oct 9 15:54:16 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 8BFEDE33F0D; Mon, 9 Oct 2017 15:54:16 +0000 (UTC) (envelope-from cy.schubert@komquats.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id CF1C182844; Mon, 9 Oct 2017 15:54:15 +0000 (UTC) (envelope-from cy.schubert@komquats.com) Received: from spqr.komquats.com ([96.50.22.10]) by shaw.ca with SMTP id 1aNfer01hM9gt1aNgeQZtz; Mon, 09 Oct 2017 09:54:13 -0600 X-Authority-Analysis: v=2.2 cv=a+JAzQaF c=1 sm=1 tr=0 a=jvE2nwUzI0ECrNeyr98KWA==:117 a=jvE2nwUzI0ECrNeyr98KWA==:17 a=kj9zAlcOel0A:10 a=02M-m0pO-4AA:10 a=6I5d2MoRAAAA:8 a=YxBL1-UpAAAA:8 a=a4Aw6F95ZvH9RvhgbvMA:9 a=K0OK_Eu0TMvzUq_5:21 a=0CyyuH9aOhxaPQ_5:21 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=Ia-lj3WSrqcvXOmTRaiG:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTPS id B02F0D0; Mon, 9 Oct 2017 08:54:10 -0700 (PDT) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.15.2/8.15.2) with ESMTP id v99FsAjK002950; Mon, 9 Oct 2017 08:54:10 -0700 (PDT) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <201710091554.v99FsAjK002950@slippy.cwsent.com> X-Mailer: exmh version 2.8.0 04/21/2012 with nmh-1.6 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Alan Cox cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r324420 - in head/sys: kern sys In-Reply-To: Message from Alan Cox of "Sun, 08 Oct 2017 22:17:39 -0000." <201710082217.v98MHdNI012272@repo.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Mon, 09 Oct 2017 08:54:10 -0700 X-CMAE-Envelope: MS4wfDMxvRywROYAUhDwqywor3BDK/16EEP3jKuzwG86D9pgTB5QjUmIinMeTcptv/YsNlQbE4CdtV2R4/cZgefmzxiywC8rZWd62oA/n9yOnJ/38IP/lNyh FxTAaZ3vKLtjXafQsVncvnBJjLZMrspa1vdVonj45Fvel0fwcOst9YOKRtrvyVaQ5HowV6TOycISVzgn5t877qZ39PKHe/g6EkNuEPUmuwOvyn/a7smAjGGZ qeX6XrmgbBvyIZf/kiHnXt1y1G8N8VoJ3XWbh+sLpfg/cwHSb0UFDUStICztvnB9 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 15:54:16 -0000 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. -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few.