Date: Sat, 19 Nov 2016 19:36:25 -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: <B1FB9B1B-2341-480F-9D25-683255B0197E@dsl-only.net> In-Reply-To: <CAHSQbTARWYfg8OSX90WXR0%2BKs49brYm%2BjBXtHF%2BpLAX9QqubZQ@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2016-Nov-19, at 7:32 PM, Justin Hibbits <jhibbits@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.
=3D=3D=3D
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.]
>=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 **);
> >
> > -#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];
>=20
>=20
> =3D=3D=3D
> Mark Millard
> markmi at dsl-only.net
>=20
> On 2016-Nov-19, at 7:00 PM, Mark Millard <markmi@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.]
> >
> > 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.
> >
> >
> > =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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B1FB9B1B-2341-480F-9D25-683255B0197E>