[RFC PATCH 05/17] ARM: kernel: save/restore kernel IF

[Lorenzo Pieralisi]

On Mon, Jul 11, 2011 at 03:31:30PM +0100, Frank Hofmann wrote:
> 
> 
> On Mon, 11 Jul 2011, Lorenzo Pieralisi wrote:
> 
> > On Fri, Jul 08, 2011 at 05:12:22PM +0100, Frank Hofmann wrote:
> >> Hi Lorenzo,
> >>
> >> only a few comments at this stage.
> >>

[...]

> >> How much memory do all the pagedirs require that are being kept around ?
> >> Why does each core need a separate one, what would happen to just use a
> >> single "identity table" for all ?
> >> I understand you can't use swapper_pg_dir for idle, so a separate one has
> >> to be allocated, yet the question remains why per-cpu required ?
> >>
> >>
> >
> > I just allocate a 1:1 mapping once cloned from init_mm, reused for all CPUs,
> > with an additional mapping.
> 
> Have started to wonder whether this facility (a 1:1-mapped pagedir for 
> kernel text/data, or maybe even "non-user text/data") could/should be made 
> available on a global scale; after all, both kexec, reset, hibernate, some 
> forms of idle/suspend do require "some sort of that" to go through MMU 
> reinitialization. I'm unfortunately not deep enough in the VM subsystem to 
> say how exactly this best would have to look like.
> 
> Merely mentioning this because it looks while everyone creates 1:1 
> mappings, there's ever so slight differences between how the 1:1 
> mapping(s) are created; we've seen:
> 
>  	* (re)using current->active_mm->pgd
>  	* (re)using swapper_pg_dir (different lengths for the 1:1 section)
>  	* using a separately-allocated/initialized pgdir
> 
> Such proliferation usually means there's a justified need to do that kind 
> of thing. Just the gritty details haven't been sorted how everyone with 
> that need could do _the same_ thing instead of reinventing various square 
> wheels.
> 
> The main reason why I've used swapper_pg_dir in the hibernation patch is 
> because it's the only static one in the system (the hibernation hooks are 
> in irqs-off codepaths and pgd_alloc isn't a good idea then), and happens 
> to have "clear" lower sections (in the user area) so that one's not 
> actually substituting anything when creating 1:1 mappings (and getting rid 
> of them restores the "pristine" state). But this assumption only holds as 
> long as swapper_pg_dir's use isn't changed. So a little creepy feeling 
> remains.
> 
> A generic "reset_mmu_pg_dir", wouldn't that be a good thing to have ?
>

It is a fair point, and certainly worth discussing. I will give it
some thought and get back to you.

> > The array of pointers is there to save pgdir on idle entry, one per-cpu.
> 
> If you're going through cpu_{do_}suspend/resume, the TTBRs are 
> saved/restored anyway, what do you need to keep the virtual addresses 
> around for ?
> 

Because I switch mm before calling suspend, which is called
with a cloned pgdir. I am not sure I can avoid that.

> 
> >
> >>
> >> I'm currently transitioning between jobs; will re-subscribe to arm-kernel
> >> under a different email address soon, this one is likely to stop working
> >> in August. Sorry the inconvenience and high-latency responses till then :(
> >>
> >> FrankH.
> >
> > May I thank you very much for the review in the interim,
> 
> You're welcome.
> FrankH.
> 

Thank you,
Lorenzo