From owner-freebsd-hackers@FreeBSD.ORG Tue Jan 13 18:53:31 2015 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 92BF4A56; Tue, 13 Jan 2015 18:53:31 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6970FB3E; Tue, 13 Jan 2015 18:53:31 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-70-85-31.nwrknj.fios.verizon.net [173.70.85.31]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3D882B924; Tue, 13 Jan 2015 13:53:30 -0500 (EST) From: John Baldwin To: Eric van Gyzen , Lars Engels , Allan Jude Subject: Re: [PATCH] Display progress during getmemsize() so the kernel doesn't look like it hanged Date: Tue, 13 Jan 2015 13:53:19 -0500 Message-ID: <3221643.2Qu3vC2WD5@ralph.baldwin.cx> User-Agent: KMail/4.14.2 (FreeBSD/10.1-STABLE; KDE/4.14.2; amd64; ; ) In-Reply-To: <54B53E40.1030903@vangyzen.net> References: <54B35B36.4040504@freebsd.org> <20150113091122.GK67556@e-new.0x20.net> <54B53E40.1030903@vangyzen.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 13 Jan 2015 13:53:30 -0500 (EST) Cc: freebsd-hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Jan 2015 18:53:31 -0000 On 1/13/15 10:48 AM, Eric van Gyzen wrote: > On 01/13/2015 04:11, Lars Engels wrote: >> On Mon, Jan 12, 2015 at 12:27:18AM -0500, Allan Jude wrote: >>> Is this feature still useful with memtest.tests=0? >> This feature is useful for everyone who has it set to 1, if they know >> about it or not. So it's a very useful feature. > > Agreed. > > Comments on the patch: > > The patch will divide by zero when PAGE_SIZE > 1 MiB. Maybe remove > PAGES_PER_MB, and just use PAGES_PER_GB. Make it const, too. > > The "total" line is mostly redundant with the later messages regarding > "real memory" and "avail memory". I suggest removing it. I agree with these. One other nit is that FreeBSD tends to use all caps for a macro rather than variables. Ravi, how about this variant, it also only displays the dots if the memory test is enabled. Also, I think the commit to disable the test should probably be merged to 10, yes. Index: machdep.c =================================================================== --- machdep.c (revision 277075) +++ machdep.c (working copy) @@ -1557,6 +1557,8 @@ native_parse_memmap(caddr_t kmdp, vm_paddr_t *phys } } +#define PAGES_PER_GB (1024 * 1024 * 1024 / PAGE_SIZE) + /* * Populate the (physmap) array with base/bound pairs describing the * available physical memory in the system, then test this memory and @@ -1575,6 +1577,7 @@ getmemsize(caddr_t kmdp, u_int64_t first) u_long physmem_start, physmem_tunable, memtest; pt_entry_t *pte; quad_t dcons_addr, dcons_size; + int page_counter; bzero(physmap, sizeof(physmap)); basemem = 0; @@ -1678,6 +1681,9 @@ getmemsize(caddr_t kmdp, u_int64_t first) * physmap is in bytes, so when converting to page boundaries, * round up the start address and round down the end address. */ + page_counter = 0; + if (memtest != 0) + printf("Testing system memory"); for (i = 0; i <= physmap_idx; i += 2) { vm_paddr_t end; @@ -1708,6 +1714,14 @@ getmemsize(caddr_t kmdp, u_int64_t first) goto skip_memtest; /* + * Print a "." every GB to show we're making + * progress. + */ + page_counter++; + if ((page_counter % PAGES_PER_GB) == 0) + printf("."); + + /* * map page into kernel: valid, read/write,non-cacheable */ *pte = pa | PG_V | PG_RW | PG_NC_PWT | PG_NC_PCD; @@ -1794,6 +1808,8 @@ do_next: } *pte = 0; invltlb(); + if (memtest != 0) + printf("\n"); /* * XXX -- John Baldwin