Date: Sat, 19 Nov 2016 20:22:08 -0800 From: Mark Millard <markmi@dsl-only.net> To: Justin Hibbits <jhibbits@freebsd.org> Cc: svn-src-head@freebsd.org, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: svn commit: r308817 - head/sys/powerpc/include [Still have pmap_t and struct pmap ppowerpc64 problems as of -r308860] Message-ID: <9CDAF14D-2953-4914-BD3D-C91F591DE7DD@dsl-only.net> In-Reply-To: <D9F92D13-80EF-49C8-BA1B-9EC7C91D91E7@freebsd.org> References: <39962D4C-29BA-4AA4-B77D-2344A68FDB54@dsl-only.net> <53258F35-C86E-4DE0-BDF0-5C139E68356D@dsl-only.net> <20161119204715.79632a66@zhabar.knownspace> <E6398D77-9604-4DB8-8139-231D4A6118B0@dsl-only.net> <3D338DB4-9FAF-46A8-96FF-4F77B01871E2@dsl-only.net> <CAHSQbTARWYfg8OSX90WXR0%2BKs49brYm%2BjBXtHF%2BpLAX9QqubZQ@mail.gmail.com> <B1FB9B1B-2341-480F-9D25-683255B0197E@dsl-only.net> <1EC92166-7DF3-42DC-9AAD-AED1BA9D5CC1@dsl-only.net> <59613CEF-BA2D-4010-996C-6FB8F9D126C6@dsl-only.net> <D9F92D13-80EF-49C8-BA1B-9EC7C91D91E7@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2016-Nov-16, at 8:33 PM, Justin Hibbits <jhibbits@freebsd.org> wrote: > *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). >=20 > - Justin That will not be the only potential place: umastat.c in = tools/tools/umastat/ also has a include of vm/vm_page.h: > # find /usr/src/ -name .svn -prune -o -name sys -prune -o -name man = -prune -o -exec grep "vm_page[.]h" {} \; -print | more > #include <vm/vm_page.h> > /usr/src/lib/libmemstat/memstat_uma.c > #define LIBMEMSTAT /* Cause vm_page.h not to include opt_vmpage.h = */ > #include <vm/vm_page.h> > /usr/src/tools/tools/umastat/umastat.c =3D=3D=3D Mark Millard markmi at dsl-only.net On Nov 19, 2016, at 9:47 PM, Mark Millard wrote: > [Top post of bad news.] >=20 > With the patch I get a different incomplete type used in libmemstat: >=20 > struct md_page >=20 > --- 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 >=20 > make[5]: stopped in /usr/src/lib/libmemstat >=20 >=20 > =3D=3D=3D > Mark Millard > markmi at dsl-only.net >=20 > On 2016-Nov-19, at 7:42 PM, Mark Millard <markmi at dsl-only.net> = wrote: >=20 >> On 2016-Nov-19, at 7:36 PM, Mark Millard <markmi at dsl-only.net> = wrote: >>=20 >>> On 2016-Nov-19, at 7:32 PM, Justin Hibbits <jhibbits at freebsd.org> = wrote: >>>=20 >>>> 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 >>>=20 >>> Yep: I manually did the move of the pm_stats line and am building. >>=20 >> 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? >>=20 >> =3D=3D=3D >> Mark Millard >> markmi at dsl-only.net >>=20 > On Nov 19, 2016 21:27, "Mark Millard" <markmi at dsl-only.net> wrote: >> [Top post about patch issues.] >>=20 >> Looking at the patch it seems to be designed for when #else was in = use: >>=20 >>> -#else >>> +#elif defined(BOOKE) >>=20 >> but -r308817 already has the 2nd line (BOOKE). Your patch shows: >>=20 >>> Index: sys/powerpc/include/pmap.h >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> --- sys/powerpc/include/pmap.h (revision 308718) >>> +++ sys/powerpc/include/pmap.h (working copy) >>=20 >> So it looks like you started from before -r308817 . >>=20 >> Trying it (I'm at -r308860): >>=20 >>> 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 >>=20 >>> # 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 **); >>>=20 >>> -#else >>> +#elif defined(BOOKE) >>>=20 >>> 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 = */ >>>=20 >>> /* Page table directory, array of pointers to page tables. */ >>> pte_t *pm_pdir[PDIR_NENTRIES]; >>=20 >>=20 >> =3D=3D=3D >> Mark Millard >> markmi at dsl-only.net >>=20 >> On 2016-Nov-19, at 7:00 PM, Mark Millard <markmi at dsl-only.net> = wrote: >>=20 >> It may take a little bit but I'll try the patch. >>=20 >> 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. >>=20 >> 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. >>=20 >> =3D=3D=3D >> Mark Millard >> markmi at dsl-only.net >>=20 >> On 2016-Nov-19, at 6:47 PM, Justin Hibbits <jhibbits at freebsd.org> = wrote: >>=20 >> On Sat, 19 Nov 2016 18:36:39 -0800 >> Mark Millard <markmi at dsl-only.net> wrote: >>=20 >>> [Quick top post I'm afraid.] >>>=20 >>> I think that I figured out why there is a problem even earlier >>> --that just did not stop the compiles. >>>=20 >>> 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). >>>=20 >>> So if it includes machine/pmap.h that binds to >>> sys/powerpc/include/pmap.h which has the structure. . . >>>=20 >>> . . . >>> #if defined(AIM) >>> . . . (definitions here) >>> #elif defined(BOOKE) >>> . . . (definitions here) >>> #endif >>> . . . >>>=20 >>> it gets no definition now. >>>=20 >>> With the older: >>>=20 >>> . . . >>> #if defined(AIM) >>> . . . (definitions here) >>> #else >>> . . . (definitions here) >>> #endif >>> . . . >>>=20 >>> It got a definition, just not necessarily the right one. >>>=20 >>>=20 >>> =3D=3D=3D >>> Mark Millard >>> markmi at dsl-only.net >>=20 >> 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. >>=20 >> - Justin<fix_pmap.diff> >=20 >=20 >=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9CDAF14D-2953-4914-BD3D-C91F591DE7DD>