Merged pmap.c - Re: Support for 64bit userspace.

[Jayachandran C.]

On Sat, Jul 31, 2010 at 5:17 AM, Jayachandran C.
<c.jayachandran at gmail.com>wrote:

> On Fri, Jul 30, 2010 at 2:07 PM, Juli Mallett <jmallett at freebsd.org>wrote:
>
>> On Fri, Jul 30, 2010 at 01:15, Jayachandran C. <c.jayachandran at gmail.com>
>> wrote:
>> > I will look at merging them without too many ifdefs. Another
>> > possibility is to have a pmap.c, pmap32.c and pmap64.c, so that the
>> > common code is not duplicated, but that too (as far as I can see) will
>> > require a config option.
>>
>> I'm not really sure what the aversion to ifdefs is — they kind of come
>> with the territory and this is exactly the part of the system where
>> you expect to have them to conditionalize exactly this kind of
>> behavior.  If you were going to split out some subroutines for 32-bit
>> and 64-bit, there are two ways you can do it without a config option
>> (and I agree with you that avoiding one is very desirable — we already
>> use too many knobs to get the right ABI in kernel configs).  You could
>> do:
>>
>> #ifndef __mips_n64
>> #include "pmap32.c"
>> #else
>> #include "pmap64.c"
>> #endif
>>
>> Or you could wrap the whole 32/64 files (modulo a "struct __hack;" or
>> whatever to avoid an empty translation unit) with ifdefs.
>>
>> Personally, I find having two implementations of a function a lot
>> harder to read and keep track of than a single implementation with
>> ifdefs in the body — there's very few places in the kernel where we do
>> the former.  If you do split out functions into separate files I would
>> beg you to only split out the really big functions that are actually
>> doing things based on different levels of page tables and then it's
>> obvious why those things have two implementations and what needs to be
>> abstracted to merge them.  Having two copies of trivial functions or
>> non-trivial functions that are mostly identical is a bad thing.
>>
>> You're doing the work, so at some point it's kind of your call, but I
>> feel like you're making style changes to things that I cleaned up and
>> tried to unify in pmap.c and that the way that they're done is pretty
>> significant given how intimidating people find pmap.  If we were going
>> to support different kinds of MMUs (e.g. adding support for BAT-based
>> MMUs) I'd be in favor of splitting things up, but then I'd want to
>> follow PowerPC's lead.  For now I think the appropriate thing to do is
>> to follow the lead of my 64-bit work in the MIPS pmap and the PAE work
>> in the i386 pmap and to use ifdefs and to try to abstract out the
>> page-table differences.
>>
>
> I had not looked at merging the files so far (mostly because it was easier
> to do it this way), so I will see how best this can be done.
>
> I'm really thrilled with the work you're doing and can't wait for it
>> to be in head — I'm quite eager to move everything on Octeon to n64 :)
>>
>
> I have checked in everything except the page table implementation code.
>
> If you like to try the current code, the attached patch has the rest of the
> changes.  This has a few more issues fixed, n64 mostly works for me, except
> a csh crash I'm looking at.
>
> I'll have a look at merging pmap.c with pmap64.c, and have another look at
> the PAE.
>


Here's is the merged version of n64 changes. I had to change the existing
pmap.c style a bit, so that merging the 32 bit and 64 bit versions was
easier.

On the n64 compilation, I have done a few more fixes, and the current code
comes up multi-user and holds up pretty well on XLR.

I'm doing some stress testing on o32 to see that I have not broken anything
there during the changes.

Let me know your comments. Also note that PTEs are still 32 bit and KSEG0
addresses are still used for page table pages, but these can be added...

Thanks,
JC.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mips-3lvl-pt.patch
Type: application/octet-stream
Size: 29542 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-mips/attachments/20100803/3f70a31a/mips-3lvl-pt.obj