From owner-freebsd-fs@FreeBSD.ORG Wed Dec 9 20:32:08 2009 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 30FAB106566B for ; Wed, 9 Dec 2009 20:32:08 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 003F28FC16 for ; Wed, 9 Dec 2009 20:32:07 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id A5A2D46B2A; Wed, 9 Dec 2009 15:32:07 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id D51B58A020; Wed, 9 Dec 2009 15:32:06 -0500 (EST) From: John Baldwin To: Matt Reimer Date: Wed, 9 Dec 2009 15:31:42 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091103; KDE/4.3.1; amd64; ; ) References: <200912091015.15450.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <200912091531.42421.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Wed, 09 Dec 2009 15:32:06 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-fs@freebsd.org Subject: Re: PATCH: increase heap size for (gpt)zfsboot X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Dec 2009 20:32:08 -0000 On Wednesday 09 December 2009 1:00:47 pm Matt Reimer wrote: > On Wed, Dec 9, 2009 at 7:15 AM, John Baldwin wrote: > > On Tuesday 08 December 2009 6:06:14 pm Matt Reimer wrote: > >> On Tue, Dec 8, 2009 at 5:13 AM, John Baldwin wrote: > >> > On Monday 07 December 2009 7:01:03 pm Matt Reimer wrote: > >> >> Enlarge the heap size for gptzfsboot and zfsboot. This is necessary so > >> >> the ZFS code has enough memory to handle decompression and error > >> >> recovery. > >> >> > >> >> Before this patch the heap grew from the end of the (gpt)zfsboot code > >> >> and static data up to 640KB, possibly overrunning the stack. Now the > >> >> heap is located at 16MB-64MB. > >> >> > >> >> Note that this requires machines with at least 64MB RAM, but this is > >> >> not likely to be a problem on any machine running ZFS. > >> >> > >> >> Sponsored by: VPOP Technologies, Inc. > >> >> > >> >> Matt Reimer > >> > > >> > Unfortunately the 16M - 64M range may not all be useable RAM. It may contain > >> > ACPI tables, etc. A robust approach would involve walking the SMAP, etc. I > >> > just committed some changes to sys/boot/i386/loader/biosmem.c that make it > >> > smarter about choosing a heap region. I suggest adopting that algorithm for > >> > figuring out a safe heap range. zfsboot and gptzfsboot should have enough > >> > free space to take the bios_getmem() function. You can increase the minimum > >> > heap size beyond 3M if desired (though 3M is the minimum the loader will > >> > guarantee currently, and if ZFS needs more than that it likely needs to be > >> > changed in both places). > >> > >> Thanks John. I dropped the bios_getmem() into zfsboot.c and it works > >> for me. How's the attached patch look to you? > > > > I tweaked it slightly (memtop* aren't needed for gptboot, and memsize() isn't > > used anymore). Can you test the updated version below? > > It works. But making bios_getmem() inline causes a compiler warning: > > /usr/src/sys/boot/i386/gptzfsboot/../zfsboot/zfsboot.c: In function 'main': > /usr/src/sys/boot/i386/gptzfsboot/../zfsboot/zfsboot.c:353: warning: > inlining failed in call to 'bios_getmem': --param > max-inline-insns-single limit reached > /usr/src/sys/boot/i386/gptzfsboot/../zfsboot/zfsboot.c:620: warning: > called from here Ok, the inline is probably not needed for zfsboot since it isn't quite as space constrained. I will commit it w/o the inline. -- John Baldwin