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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to