A few ports fail to build due to missing pmap definitions, which are
specific per-pmap type. This tries to appease those ports, by merging all pmaps
together. It can probably be cleaned up a bit more, but this is a working first
pass.
Details
- Reviewers
luporl nwhitehorn - Commits
- rS347078: powerpc: Merge all pmap struct definitions
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 24027 Build 22913: arc lint + arc unit
Event Timeline
Overall this change looks good to me, but I'm worried with the union of structs parts (see comment below).
Do you have an idea of what is the size of pmap and md_page structs on BOOKE and on AIM?
Could this change incur a significant overhead on memory usage?
sys/powerpc/include/pmap.h | ||
---|---|---|
153 | I'm concerned about the parts of this change that use union of structs instead of ifdefs. For instance, at this point, even if I'm not on a BOOKE, my pmap objects must be big enough to hold BOOKE fields, right? Wouldn't we be wasting memory with this approach? Couldn't the union be replaced with an ifdef, inside a single pmap struct definition? |
The 64-bit Book-E pmap overhead is around 32kB (12 bits worth of PP2D_NENTRIES, so 4096 8-byte entries), and 4kB on 32-bit. This is not small by any stretch, and I do want to rework it, and plan to over the next couple months. For now I can #ifdef within the union (I do want to keep it a union, so that we can eventually have a GENERIC that boots on both AIM and Book-E).
sys/powerpc/include/pmap.h | ||
---|---|---|
153 | We could, but that almost defeats the purpose of this exercise, which is to let those ports that need access to kernel data structures to have access to those structures without having to specify AIM or BOOKE. On my list of things to do is also to rework the Book-E pmap layer to not have as much stuff inline for just this reason. |
Ok, I see your point, thanks for fetching the overhead data. Then it should ok to go ahead, with the #ifdef's for now, until the rework is done and it can be removed.