Re: UVM patch for PR port-xen/45975

[Chuck Silvers <chuq%chuq.com@localhost>]

On Sat, Feb 18, 2012 at 10:25:08PM +0100, Manuel Bouyer wrote:
> On Sat, Feb 18, 2012 at 09:16:45PM +0000, Mindaugas Rasiukevicius wrote:
> > Manuel Bouyer <bouyer%antioche.eu.org@localhost> wrote:
> > > On Sat, Feb 18, 2012 at 07:49:13PM +0000, Mindaugas Rasiukevicius wrote:
> > > > > [...]
> > > > > I also implemented the idea of removing the mappings and freeing page
> > > > > in batch of 16 entries. Updated patch attached, I'm re-running the
> > > > > build.sh -jx release tests. This should also fix the problem in the
> > > > > native case as there's no #ifdef any more :)
> > > > 
> > > > This is unnecessary (due to deferred invalidations), please keep it
> > > > simple.
> > > 
> > > I guess it's the batch'ing of pmap_kremove that's unecessery.
> > > I'm not sure: some pmap_kremove() implementation do what looks like
> > > expensive calls (like PMAP_SYNC_ISTREAM_KERNEL on alpha,
> > > pmap_vac_me_harder() on arm, VAC flushing on m68k/motorola, and so on).
> > > Also, some platforms zero out pte using memset, and it's probably more
> > > efficient than zeroing one at a time
> > 
> > Yes.  Such architectures are free to follow performance improvements made
> > on x86 and MIPS.  It is the reason why we have pmap_update() in pmap(9) API.
> 
> If the expensive call has to be made before updating the mappings,
> pmap_update() is not of that much use (or you have to remember operations
> to be made, and apply them at pmap_update() time, which is not that
> easy).

since pmap_kremove() can defer its effects (though I guess it currently doesn't
for xen, since otherwise your latest patch wouldn't work?  not sure, I haven't
found the code yet), I would think we would need to call pmap_update() before
freeing the page in this code.

if it's difficult to avoid calling pmap_update() multiple times due to that,
note that calling pmap_update() extra times doesn't have to cost very much,
since it can easily return without doing anything if no deferred operations
are pending.  the x86 pmap_tlb_shootnow() already has a check for this,
though the x86 pmap_update() could be even cheaper in the case where it
really has nothing to do.

-Chuck