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

[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
>> _at__at_ -179,13 +180,13 _at__at_
>> 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>