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 <[email protected]> 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
