Re: [PAE support] Initial patch review

[Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>]

On Thu, 10 Jun 2010 01:45:38 +0100, Mindaugas Rasiukevicius
<rmind%netbsd.org@localhost> wrote:
>> It diverges quite a bit from the original patch from Jeremy. Reason is 
>> to avoid too many #ifdef's between Xen and non-Xen pmap <...>
> 
> Getting rid of #ifdefs, especially in pmap, is a right direction we
> should continue moving in time.

I took some time to avoid too many #ifdef's here; but I am still not
100% satisfied, my eyes burn each time I read through pmap_load().

>> XXX misses support for ephemeral mapping. I am not sure on how to 
>> implement it correctly, so pmap_load() uses tlbflush(). Same goes for 
>> port-xen. I can revisit it later, after merging this patch +
xen-suspend.
> 
> Which part of support?  It is same entering/removal of unmanaged
mappings,
> except without TLB flush (and IPIs).  These are something to track with
> generation numbers, normally in such way:
> 
>       u_int gen = uvm_emap_gen_return();
>       tlbflush();
>       uvm_emap_update(gen);

Cool then, I thought it would be trickier to use due to %cr3 handling of
PAE.
emap support is pretty much straightforward in that case.

>> Any comments on the modifications there? I would prefer to have all 
>> important changes "approved" before starting regression testing.
> 
> Do you have any data whether/how these changes affect performance?

Don't expect improvements with PAE; I expect it to be slower than ia32,
mainly because of paddr_t (64 bits handling in 32 bits), and probably
at the hardware level too.

Benchmarks will be part of the regression testing step (build.sh release
runs,
hackbench, sysbench, unless Andi suggests something else)

I am also curious to compare Xen non-PAE vs Xen PAE.

> I see some clean-ups and style changes in the patch - help (as well as
> reviews and testing) on syncing of uvmplock branch would be useful,
since
> some parts of pmap are changed/split there.
> 
> Thanks for working on this!
> 
> P.S. Could you please generate patches with diff -p option?

Yup, sorry; file updated [1].

[1] http://www.NetBSD.org/~jym/pae.diff

-- 
Jean-Yves Migeon
jeanyves.migeon%free.fr@localhost