svn commit: r290221 - head/sys/powerpc/powerpc

[Justin Hibbits]

On Oct 30, 2015, at 9:27 PM, Conrad Meyer wrote:

> Comments inline.
>
> On Fri, Oct 30, 2015 at 7:08 PM, Justin Hibbits  
> <jhibbits at freebsd.org> wrote:
>> Author: jhibbits
>> Date: Sat Oct 31 02:08:39 2015
>> New Revision: 290221
>> URL: https://svnweb.freebsd.org/changeset/base/290221
>>
>> Log:
>>  Print unsigned memory sizes, to handle >2GB RAM on 32-bit powerpc.
>> ...
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- head/sys/powerpc/powerpc/machdep.c  Sat Oct 31 02:07:30  
>> 2015        (r290220)
>> +++ head/sys/powerpc/powerpc/machdep.c  Sat Oct 31 02:08:39  
>> 2015        (r290221)
>> @@ -176,12 +176,12 @@ cpu_startup(void *dummy)
>> #ifdef PERFMON
>>        perfmon_init();
>> #endif
>> -       printf("real memory  = %ld (%ld MB)\n", ptoa(physmem),
>> +       printf("real memory  = %lu (%lu MB)\n", ptoa(physmem),
>>            ptoa(physmem) / 1048576);
>
> Shouldn't this be "real memory = %ju (%lu MB)\n" and
> (uintmax_t)ptoa(physmem), or it may overflow on >4GB RAM systems?

Yes, it should, and it will.  However, currently ptoa() casts to  
unsigned long, and I didn't want to change that yet (I will  
eventually).  In fact, the machine I'm testing on has 8GB, which I've  
had to artificially limit to <4GB because of the various memory  
accounting size limits.

>
>>        realmem = physmem;
>>
>>        if (bootverbose)
>> -               printf("available KVA = %zd (%zd MB)\n",
>> +               printf("available KVA = %zu (%zu MB)\n",
>>                    virtual_end - virtual_avail,
>>                    (virtual_end - virtual_avail) / 1048576);
>>
>> @@ -199,7 +199,7 @@ cpu_startup(void *dummy)
>>                        #ifdef __powerpc64__
>>                        printf("0x%016lx - 0x%016lx, %ld bytes (%ld  
>> pages)\n",
>>                        #else
>> -                       printf("0x%08x - 0x%08x, %d bytes (%ld  
>> pages)\n",
>> +                       printf("0x%08x - 0x%08x, %u bytes (%lu  
>> pages)\n",
>
> It seems wrong that bytes is only %u here and pages is a %lu.  I think
> bytes should probably be %ju too?  What is the type of size1?

You're right.  It should be long.  However, it's 6-of-one in this  
case, since on powerpc (32-bit) sizeof(long) == sizeof(int), so it  
doesn't matter too much yet.  size1 is a vm_offset_t, which is 32-bits  
in this case.  phys_avail[] is also an array of vm_offset_t's, and I  
intend to change it to vm_paddr_t, and cast to uintmax_t for printf  
purposes, but again, that's longer term (a lot of the powerpc bootup  
needs a cleanup, anyway).

>
>>                        #endif
>>                            phys_avail[indx], phys_avail[indx + 1] -  
>> 1, size1,
>>                            size1 / PAGE_SIZE);
>> @@ -208,7 +208,7 @@ cpu_startup(void *dummy)
>>
>>        vm_ksubmap_init(&kmi);
>>
>> -       printf("avail memory = %ld (%ld MB)\n",  
>> ptoa(vm_cnt.v_free_count),
>> +       printf("avail memory = %lu (%lu MB)\n",  
>> ptoa(vm_cnt.v_free_count),
>>            ptoa(vm_cnt.v_free_count) / 1048576);
>
> Same as with the first printf.

See above.  It's on my list, I just wanted something cleaner for now.

>
> Best,
> Conrad


- Justin