Re: [PATCH 1 of 4] p2m: Keep statistics on order of p2m entries

[George Dunlap <george.dunlap@citrix.com>]

On Fri, 2011-05-06 at 15:23 +0100, Tim Deegan wrote:
> Hi, 
> 
> Can you please add this:
> 
>  [diff]
>  showfunc = True

Ah, very helpful.

> There are two places where this patch changes the control flow of p2m
> operations: 

Oops -- yeah, those definitely need some comments.

The "*_free_entry()" functions are used when replacing smaller-order
pages with a larger-order page.  Most commonly this is replacing a bunch
of 4k pages with a 2MiB page, but in theory you could be replacing a
tree of mixed 4k and 2MiB pages with a 1G page as well.

When that happens, we also need to adjust the p2m order stats by:
* Decrementing stats on all the leaf entries being replaced
* Incrementing the stat for the new leaf entry.

Incrementing the stat for the new entry is done when the new entry is
assigned.  Decrementing stats requires walking the old p2m tables and
finding valid leaf entries.  Since *_free_entry() already do that, we
just piggyback.

p2m entries are already checked at the entrance to the function to see
if we're at a leaf. The patch adds appropriate reference counting if it
is.

The other change is how far down the tree *_free_entry() functions go.
Since their original purpose was only freeing intermediate tables, they
don't bother to walk all the way down to l0.  Since we want to do
reference counting of the l0s as well, we just make it go all the way
down.  

This does mean making a function call for all 512 entries of the L1
table, but it should be a pretty rare operation.  I could refactor the
function to do the check before making the call instead, if desired.

Let me know if you want me to include this description in a new patch
series.

 -George

> 
> > @@ -167,11 +169,14 @@
> >  void ept_free_entry(struct p2m_domain *p2m, ept_entry_t *ept_entry, int level)
> >  {
> >      /* End if the entry is a leaf entry. */
> > -    if ( level == 0 || !is_epte_present(ept_entry) ||
> > -         is_epte_superpage(ept_entry) )
> > +    if ( level == 0 || !is_epte_present(ept_entry) || is_epte_superpage(ept_entry) )
> > +    {
> > +        if ( is_epte_countable(ept_entry) )
> > +            p2m->stats.entries[level]--;
> >          return;
> > +    }
> >  
> > -    if ( level > 1 )
> > +    if ( level > 0 )
> >      {
> 
> and similarly: 
> 
> > @@ -184,11 +184,15 @@
> >  {
> >      /* End if the entry is a leaf entry. */
> >      if ( page_order == 0
> > -         || !(l1e_get_flags(*p2m_entry) & _PAGE_PRESENT)
> > +         || !(l1e_get_flags(*p2m_entry) & _PAGE_PRESENT) 
> >           || (l1e_get_flags(*p2m_entry) & _PAGE_PSE) )
> > +    {
> > +        if ( l1e_get_flags(*p2m_entry) )
> > +            p2m->stats.entries[page_order/9]--;
> >          return;
> > -
> > -    if ( page_order > 9 )
> > +    }
> > +
> > +    if ( page_order )
> 
> here.  Can you explain those in a bit more detail?
> 
> Cheers,
> 
> Tim.
>