Date: Tue, 13 Jan 2015 13:53:19 -0500 From: John Baldwin <jhb@freebsd.org> To: Eric van Gyzen <eric@vangyzen.net>, Lars Engels <lars.engels@0x20.net>, Allan Jude <allanjude@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] Display progress during getmemsize() so the kernel doesn't look like it hanged Message-ID: <3221643.2Qu3vC2WD5@ralph.baldwin.cx> In-Reply-To: <54B53E40.1030903@vangyzen.net> References: <D0D89A8E.129518%rpokala@panasas.com> <54B35B36.4040504@freebsd.org> <20150113091122.GK67556@e-new.0x20.net> <54B53E40.1030903@vangyzen.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3221643.2Qu3vC2WD5>