svn commit: r224516 - in head/sys: amd64/amd64 i386/i386 pc98/pc98

[Bruce Evans]

On Sun, 31 Jul 2011, Bruce Evans wrote:

> ...  In the
> whole kernel, 155 lines match TUNABLE.*FETCH and 14 of these match 'if ('.
> Some of the 14 are not wrong.  About 1/3 of them are for dubious use in
> memory sizing.  E.g., in amd64/machdep.c:
>
> % 	realmem = Maxmem;
>
> The unprobed Maxmem remains in realmem for printing a confusing value later
> in sysctls.

Oops.  This is actually the final (probed) value of Maxmem.  I got confused
by the file order being different from the dynamic order.

>
> % % 	/*
> % 	 * Display physical memory if SMBIOS reports reasonable amount.
> % 	 */
> % 	memsize = 0;
> % 	sysenv = getenv("smbios.memory.enabled");
> % 	if (sysenv != NULL) {
> % 		memsize = (uintmax_t)strtoul(sysenv, (char **)NULL, 10) << 
> 10;
> % 		freeenv(sysenv);
> % 	}
>
> First override for `memsize'.
>
> % 	if (memsize < ptoa((uintmax_t)cnt.v_free_count))
> % 		memsize = ptoa((uintmax_t)Maxmem);
>
> `memsize' is normally (?) `Maxmem' in bytes, but not if SMBIOS set it.
>
> % 	printf("real memory  = %ju (%ju MB)\n", memsize, memsize >> 20);
>
> Code doesn't match comment.  The display is unconditional, but the comment
> set that it is only done if SMBIOS reports a reasonable amount.
>
> `memsize' is not used after this point, so the only effect of the SMBIOS
> check is to possibly print a value that is different from all of the
> following:
> - the initial Maxmem in bytes
> - the final realmem in bytes (this equals the initial Maxmem in bytes)
> - the final Maxmem in bytes.
> The comment is also wrong in saying that SMBIOS is used.  Only an
> environment string is used.  Users can easily confuse themselves by putting
> garbage in this string.
>
> The above logic is also confused by the memory size being in cnt.v_free_cnt.
> If SMBIOS does't change memsize from 0, then we use Maxmem even if it is
> larger than cnt.v_free_cnt.

So realmem is the correct final value unless the SMBIOS non-call gives a
different value; in the latter case, we print this different value above
but the realmem sysctl can't recover it.

> % 	/*
> % 	 * Maxmem isn't the "maximum memory", it's one larger than the
> % 	 * highest page of the physical address space.  It should be
> % 	 * called something like "Maxphyspage".  We may adjust this
> % 	 * based on ``hw.physmem'' and the results of the memory test.
> % 	 */
> % 	Maxmem = atop(physmap[physmap_idx + 1]);
>
> Here we change Maxmem to another unprobed value.  I think this value is
> more closely related to cnt.v_free_cnt.
>
> % % #ifdef MAXMEM
> % 	Maxmem = MAXMEM / 4;
> % #endif
>
> Then we optionally change MaxMem to a hard-configured value.
>
> % % 	if (TUNABLE_ULONG_FETCH("hw.physmem", &physmem_tunable))
> % 		Maxmem = atop(physmem_tunable);
>
> Then we use a temporary variable, since the primary variable has
> units of pages while the tunable has units of bytes.  But this can
> be reorganized to:
>
> 	/*
> 	 * memsize was Maxmem in bytes, but was not maintained when Maxmem
> 	 * was changed about.  Maintain it now.
> 	 */
> 	memsize = ptoa(Maxmem).
> 	 *
> 	 * XXX we also have a physmem variable, but this can't be used to
> 	 * hole the result of the tunable any more than Maxmem can, since
> 	 * it is in pages.  memsize is not quite right either, since it
> 	 * is a uintmax_t while the following assumes that it is u_long.
> 	 * The u_long is too small for PAE on i386, but works for amd64.
> 	 * The uintmax_t is needed on i386, but is not needed for amd64.
> 	 */
> 	TUNABLE_ULONG_FETCH("hw.physmem", &memsize);	/* XXX sic */
> 	Maxmem = atop(memsize);
>
> % 	/*
> % 	 * Don't allow MAXMEM or hw.physmem to extend the amount of memory
> % 	 * in the system.
> % 	 */
> % 	if (Maxmem > atop(physmap[physmap_idx + 1]))
> % 		Maxmem = atop(physmap[physmap_idx + 1]);

Now we're in the getmemsize() function, which doesn't have a memsize
variable.  It should have one instead of the physmem_tunable variable,
if only because the physmem tunable is actually for memsize in bytes
and not for physmem in pages.

Bruce