svn commit: r308817 - head/sys/powerpc/include [Still have pmap_t and struct pmap ppowerpc64 problems as of -r308860]

Sun Nov 20 04:06:42 UTC 2016

*sigh* okay, thanks.  I just tested, and vm/vm_page.h, and vm/vm.h can  
both be removed from memstat_uma.c for it to compile.  I'm kicking off  
a buildworld myself now, too, and hope to have it ready to commit  
tomorrow (takes a couple hours to buildworld on my G5).

- Justin

On Nov 19, 2016, at 9:47 PM, Mark Millard wrote:

> [Top post of bad news.]
>
> With the patch I get a different incomplete type used in libmemstat:
>
> struct md_page
>
> --- all_subdir_lib/libmemstat ---
> In file included from /usr/src/lib/libmemstat/memstat_uma.c:34:0:
> /usr/src/sys/vm/vm_page.h:144:17: error: field 'md' has incomplete  
> type
>  struct md_page md;  /* machine dependent stuff */
>                 ^
> *** [memstat_uma.o] Error code 1
>
> make[5]: stopped in /usr/src/lib/libmemstat
>
>
> ===
> Mark Millard
> markmi at dsl-only.net
>
> On 2016-Nov-19, at 7:42 PM, Mark Millard <markmi at dsl-only.net>  
> wrote:
>
>> On 2016-Nov-19, at 7:36 PM, Mark Millard <markmi at dsl-only.net>  
>> wrote:
>>
>>> On 2016-Nov-19, at 7:32 PM, Justin Hibbits <jhibbits at  
>>> freebsd.org> wrote:
>>>
>>>> Sorry, I generated the diff from a different tree that wasn't  
>>>> synced to head (had the same change in both trees originally). If  
>>>> that is the only problem, you can ignore it and try the rest. I  
>>>> can generate another diff later too.
>>>> - Justin
>>>
>>> Yep: I manually did the move of the pm_stats line and am building.
>>
>> If it builds and I install it on a PowerMac G5 and it boots, what  
>> do I
>> do to test if pm_stats and pm_mtx seems to be working well/right for
>> the out of kernel code? Do you know of a reasonable test?
>>
>> ===
>> Mark Millard
>> markmi at  dsl-only.net
>>
> On Nov 19, 2016 21:27, "Mark Millard" <markmi at dsl-only.net> wrote:
>> [Top post about patch issues.]
>>
>> Looking at the patch it seems to be designed for when #else was in  
>> use:
>>
>>> -#else
>>> +#elif defined(BOOKE)
>>
>> but -r308817 already has the 2nd line (BOOKE). Your patch shows:
>>
>>> Index: sys/powerpc/include/pmap.h
>>> ===================================================================
>>> --- sys/powerpc/include/pmap.h  (revision 308718)
>>> +++ sys/powerpc/include/pmap.h  (working copy)
>>
>> So it looks like you started from before -r308817 .
>>
>> Trying it (I'm at -r308860):
>>
>>> Patching file sys/powerpc/include/pmap.h using Plan A...
>>> Hunk #1 succeeded at 74.
>>> Hunk #2 succeeded at 84.
>>> Hunk #3 succeeded at 132.
>>> Hunk #4 succeeded at 145.
>>> Hunk #5 failed at 180.
>>> Hunk #6 succeeded at 194.
>>> Hunk #7 succeeded at 210.
>>> 1 out of 7 hunks failed--saving rejects to sys/powerpc/include/ 
>>> pmap.h.rej
>>
>>> # more sys/powerpc/include/pmap.h.rej
>>> @@ -179,13 +180,13 @@
>>> struct slb **slb_alloc_user_cache(void);
>>> void   slb_free_user_cache(struct slb **);
>>>
>>> -#else
>>> +#elif defined(BOOKE)
>>>
>>> struct pmap {
>>> +       struct pmap_statistics  pm_stats;       /* pmap statistics  
>>> */
>>>      struct mtx              pm_mtx;         /* pmap mutex */
>>>      tlbtid_t                pm_tid[MAXCPU]; /* TID to identify  
>>> this pmap entries in TLB */
>>>      cpuset_t                pm_active;      /* active on cpus */
>>> -       struct pmap_statistics  pm_stats;       /* pmap statistics  
>>> */
>>>
>>>      /* Page table directory, array of pointers to page tables. */
>>>      pte_t                   *pm_pdir[PDIR_NENTRIES];
>>
>>
>> ===
>> Mark Millard
>> markmi at dsl-only.net
>>
>> On 2016-Nov-19, at 7:00 PM, Mark Millard <markmi at dsl-only.net>  
>> wrote:
>>
>> It may take a little bit but I'll try the patch.
>>
>> It looks like sys/powerpc/include/pmap.h from -r176700 from 2088- 
>> Mar-3
>> is when the BOOKE/E500 split started with the preprocessor use of AIM
>> and #else . This predates PowerMac G5 support.
>>
>> This is definitely not new for the general structure on the powerpc
>> side of things. Any place that did not have the AIM vs. not status
>> available was subject to problems of possibly mismatched definitions.
>>
>> ===
>> Mark Millard
>> markmi at dsl-only.net
>>
>> On 2016-Nov-19, at 6:47 PM, Justin Hibbits <jhibbits at  
>> freebsd.org> wrote:
>>
>> On Sat, 19 Nov 2016 18:36:39 -0800
>> Mark Millard <markmi at dsl-only.net> wrote:
>>
>>> [Quick top post I'm afraid.]
>>>
>>> I think that I figured out why there is a problem even earlier
>>> --that just did not stop the compiles.
>>>
>>> lib/libutil/kinfo_getallproc.c is built here as part of buildworld
>>> (stage 4.2 "building libraries" instead of buildkernel. It does not
>>> have the KERNCONF's AIM vs. BOOKE vs. . . . definitions vs. lack of
>>> them).
>>>
>>> So if it includes machine/pmap.h that binds to
>>> sys/powerpc/include/pmap.h which has the structure. . .
>>>
>>> . . .
>>> #if defined(AIM)
>>> . . . (definitions here)
>>> #elif defined(BOOKE)
>>> . . . (definitions here)
>>> #endif
>>> . . .
>>>
>>> it gets no definition now.
>>>
>>> With the older:
>>>
>>> . . .
>>> #if defined(AIM)
>>> . . . (definitions here)
>>> #else
>>> . . . (definitions here)
>>> #endif
>>> . . .
>>>
>>> It got a definition, just not necessarily the right one.
>>>
>>>
>>> ===
>>> Mark Millard
>>> markmi at dsl-only.net
>>
>> Can you try the attached patch?  There was a subtle ABI issue that
>> r308817 exposed, which is that the pmap structs aren't identical such
>> that the pm_stats are at different locations, and libkvm ends up
>> reading with the Book-E pmap, getting different stats than expected  
>> for
>> AIM.  This patch fixes that, bumping version to account for this ABI
>> change.
>>
>> - Justin<fix_pmap.diff>
>
>
>