Date: Wed, 2 Mar 2005 12:09:06 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Wes Peters <wes@softweyr.com> Cc: obrien@FreeBSD.org Subject: Re: Review request (Re: cvs commit: src/sys/i386/i386 machdep.c et al) Message-ID: <20050302111650.Q7969@delplex.bde.org> In-Reply-To: <200502282244.38877@zaphod.softweyr.com> References: <200502282142.j1SLgvhh067909@repoman.freebsd.org> <200502282244.38877@zaphod.softweyr.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 28 Feb 2005, Wes Peters wrote:
> On Monday 28 February 2005 20:19, David O'Brien wrote:
>> Uh, I hope you will fix the build on all non-i386 platforms before you
>> MFC. Where were they suppose to get the 'realmem' definition from?
>
> I've tested the attached patch on sparc64. Does this look OK for
> the rest of the arches? I left amd64 out since you have (apparently)
> already fixed that.
% Index: alpha/alpha/machdep.c
% ===================================================================
% RCS file: /ncvs/src/sys/alpha/alpha/machdep.c,v
% retrieving revision 1.232
% diff -u -r1.232 machdep.c
% --- alpha/alpha/machdep.c 6 Feb 2005 01:55:06 -0000 1.232
% +++ alpha/alpha/machdep.c 1 Mar 2005 14:35:38 -0000
% @@ -194,6 +194,7 @@
% struct msgbuf *msgbufp=0;
%
% long Maxmem = 0;
% +long realmem = 0;
realmem is MI, so its declaration shouldn't be N-tuplicated for N arches.
It should be next to the declaration of physmem. Maxmem is the MD variant
of this variable. Maxmem is close to being MI too -- you could probably
have just used it.
%
% long totalphysmem; /* total amount of physical memory in system */
% long resvmem; /* amount of memory reserved for PROM */
% @@ -250,6 +251,7 @@
% #endif
% printf("real memory = %ld (%ld MB)\n", alpha_ptob(Maxmem),
% alpha_ptob(Maxmem) / 1048576);
% + realmem = (long)alpha_ptob(Maxmem);
There too much duplication here too, but it is harder to untangle.
The duplication is almost 2*N times (previously only N times for the
printfs). Just one arch (sparc64) has the size not in pages and/or
not in a variable "long Maxmem". Otherwise, Maxmem is just as MI
as physmem.
The patch highlights other gratuitous machine dependencies in
exisiting code. ctob() should always be used to convert pages to
bytes, except it should have been renamed ptob() when clusters went
away 10+ years ago. Some arches use ptoa() which is bogus becaue a
size and not an address is wanted here...
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050302111650.Q7969>