On Wed, Aug 28, 2002 at 10:50:51AM -0700, Jason R Thorpe wrote:
> On Wed, Aug 28, 2002 at 07:41:29PM +0200, Jaromir Dolecek wrote:
> 
>  > I wonder if there is need for this. Perhaps this could be made
>  > more universal, and pmap internal structures synced on pmap_update()? 
>  > This might even some other operations using many pmap_remove()s faster.
> 
> Why not just make a pmap_remove_all() to just nuke all user mappings from
> a pmap, and allow it to use the shortcut?  Then exec/exit can use it before
> they do the normal remove steps, so that all the pmap_remove() steps done
> during the uvm_unmap's will be noops?



ok, after some more thought, I think that you're right that a "remove all"
operation is more what's wanted here than something to do with destroying
a pmap.  in the exec case, there's no need to actually destroy the pmap,
and destroying it and allocating a new one might be expensive.

so I'll change the proposal to be a pmap_remove_all() like you suggest,
but defined this way:

   pmap_remove_all() is a hint to the pmap module that the pmap is about to
   have all of its mappings removed.  following this call, there will be
   pmap_remove() calls resulting in every mapping being removed, followed
   by either pmap_destroy() or pmap_update().  no other pmap interfaces
   which take the pmap as an argument will called during this process.
   other interfaces which might need to access this pmap (such as
   pmap_page_protect()) are permitted during this process.

   the pmap module is free to either remove all the pmap's mappings
   immediately in pmap_remove_all(), or to use the knowledge of the
   upcoming pmap_remove() calls to optimize the removals.

   (there should be something in here about the pmap not being active
   in the sense of pmap_activate() during the removal process as well,
   but I'm not sure how to phrase it.)


the reason that it might be preferable (and in my mind it would always
be preferable) to just skip cache flushes in pmap_remove() instead of
removing everything immediately in pmap_remove_all() is that the naive
implementation of pmap_remove_all() as "pmap_remove(pmap, minaddr, maxaddr)"
would waste a lot of time looking for entries in portions of the address
space that UVM knows are unused.  this would no doubt be unacceptably slow.
the more efficient way to do this would be to write new code to find the
mappings by traversing the page tables, but platforms that use a hash table
instead of page tables would be pretty much screwed, I think.  scanning
only the ranges of the address space where UVM tells the pmap to look
seems likely to be faster than even scanning page tables, too.

as for the other question of why do we would want a new API at all,
instead of just deferring the cache flushes until we see a pmap_update():
this design works fine for the exit path, but for the exec path it would
still result in a cache flush at the end, since we won't be destroying
the pmap.  yes, the pmap could keep track of how many mappings exist
and transform the cache flush into allocating a new context if that would
be more efficient, but it seems simpler to just tell the pmap what we're
about to do so it doesn't need to have heuristics for this or track mapping
counts, etc.  however, to allow someone to do this in a pmap if they want,
I'll change the exit path to skip the pmap_update() after everything is
removed (since it simplifies implementations of my new proposal as well).

to avoid conditionalizing things, I'll just add an empty inline definition
for pmap_remove_all() to all the other pmaps.  that should be as good
as #define'ing it away, but it won't cause goofiness with the prototype
like currently exists for pmap_update().  (in fact, I could convert the
existing empty definitions of pmap_update() into inline functions as well,
to remove that goofiness entirely.)

a diff implementing this new proposal is at

ftp://ftp.netbsd.org/pub/NetBSD/misc/chs/sparc64pmap/diff.20020907

-Chuck