Subject: Re: [RFC PATCH v2 3/4] powerpc: allow ioremap within reserved memory regions

From: Albert Herranz <[email-masked]>
Date: Sat, 12 Dec 2009 01:33:15 +0100

Benjamin Herrenschmidt wrote:
> On Tue, 2009-12-08 at 19:43 +0100, Albert Herranz wrote:
>> Signed-off-by: Albert Herranz <[email-masked]>
> 
> Acked-by: Benjamin Herrenschmidt <[email-masked]>
> 
>> ---
>> v1 -> v2
>> - use a run-time flag to allow/disallow remapping reserved regions
>> - use lmbs to determine reserved regions
> 
> We won't need that once we fix proper discontig mem.
> 
> BTW. Question: Why do we need that fixup of yours to fold the 2 LMBs
> into one ?
> 
> Wouldn't it be easier just to keep the 2 LMBs ? You already fix the
> mapin_ram thingy, so you could easily fix it up to just iterate over the
> LMBs instead no ? For now, it could only BAT map the first LMB to
> simplify things and we can fix the BAT mapping for the second one in a
> second step too.
> 
> Wouldn't that work with simpler code ? An in the case of ioremap, the
> test becomes simply to check if it's RAM by checking if it's in the LMB
> rather than if it's reserved, which should be easier and wouldn't
> require your flag to "enable" the tweak since it could perfectly be kept
> as standard behaviour
> 
> Also the code in arch/powerpc/mm/mem.c will already deal with holes
> just fine and will pass the hole size information to the VM which should
> make it behave properly.
> 
> Thus I have the feeling that keeping the 2 LMBs rather than coalescing
> would simplify the code basically by only requiring a small fixup of the
> maping RAM bit.
> 
> I'm acking the patches for now, so you can always come up with a fixup
> on top of them and we can merge the current ones.
> 

I'll look into this.
I used a single lmb range just as the current code does to avoid any unwanted side effects as I didn't audit all the mm code.

Thanks,
Albert